Edgewall Software

Ticket #3262 (closed defect: wontfix)

Opened 2 years ago

Last modified 19 months ago

_html_splitlines reopens wrong tag in certain cases

Reported by: Tim Hatch <trac@…> Owned by: jonas
Priority: normal Milestone:
Component: version control/browser Version: 0.10
Severity: normal Keywords: mimeviewer highlighting
Cc:

Description

When run on the output of the PHP highlighter (php -sn), _html_splitlines chokes. This is due to having two different passes for opening and closing passes. It pretty much comes down to an incorrect interpretation of

<span class="x">
</span><span class="y">

The span open on the next line should be y, not x. This is solved by using a single regexp to walk the source, and keeping a more rigid stack of tags that are open. This is something that I've done before and am willing to code up a fix in a couple of days.

I'm attaching a patch to add a testcase in trac/mimeview/tests/api.py for this issue. This is actually a real-world issue for the following sample, when using the php-specific highlighter:

<?php
/*
comment
*/
?>

Attachments

mimeview-html-splitlines-close-order.diff (1.2 KB) - added by Tim Hatch <trac@…> 2 years ago.
Added test for api.py
mimeview-html-splitlines-close-order-fix.diff (2.0 KB) - added by Tim Hatch <trac@…> 2 years ago.
First attempt at fix, diff against trunk@3423
mimeview-html-splitlines-bugtest.diff (1.7 KB) - added by Tim Hatch <trac@…> 2 years ago.
Proof of bug as exhibited on t.e.o

Change History

Changed 2 years ago by Tim Hatch <trac@…>

Added test for api.py

Changed 2 years ago by Tim Hatch <trac@…>

First attempt at fix, diff against trunk@3423

Changed 2 years ago by Tim Hatch <trac@…>

My first attempt passes the tests in tests/api.py along with a couple others I added (the more important one is in mimeview-html-splitlines-close-order.diff above) which were previously failing.

I am using this on my copy of Trac locally and have not noticed any regressions in the display of rendered files.

I'm open to comments -- it's not as pretty as it could be (especially wrt reversing the open_tags list twice).

Changed 2 years ago by Tim Hatch <trac@…>

Proof of bug as exhibited on t.e.o

Changed 2 years ago by Tim Hatch <trac@…>

  • version changed from devel to 0.10

Bump.

I just noticed this bug affects t.e.o with the Enscript highlighter for Python. See trunk/trac/__init__.py -- the whole license should be highlighted as a string, but reverts to p_identifier instead of p_tripledouble. The SilverCity highlighter doesn't trigger this bug on this specific file, but Enscript does. Here's a sanity test and one for the specific issue raised on t.e.o: attachment:mimeview-html-splitlines-bugtest.diff

This is fixed by attachment:mimeview-html-splitlines-close-order-fix.diff above. Also, it's been long enough that 'devel' now refers to 0.11-dev, so it should probably retargeted at 0.10 since it shows up on t.e.o.

Changed 2 years ago by cboos

  • milestone set to 0.10.3

Changed 2 years ago by Tim Hatch <trac@…>

Turns out I got highlighter names backwards. Triggered with SilverCity, not Enscript.

Changed 2 years ago by Tim Hatch <trac@…>

As of r4399, this ticket should be no longer relevant. Is this 'invalid' or 'wontfix' at this point?

Changed 2 years ago by sid

Not completely intuitive yet, but this would probably be a worksforme based on TracTicketTriage guidelines.

Changed 2 years ago by cboos

  • keywords mimeviewer highlighting added
  • status changed from new to closed
  • resolution set to wontfix

Well, taking all the versions into account (as discussed in #4298) we have for this ticket:

Affects Version: 0.10.3 Fixed in Version: -- (wontfix) (1)
Affects Version: 0.11 Fixed in Version: 0.11 (fixed) (2)
  1. because it's not worth the trouble, I think
  2. fixed by being replaced by the new code (r4399)

As the reported version here is 0.10.3, I think wontfix is appropriate (well, unless someone really insists this fix goes into stable).

Changed 19 months ago by cboos

  • milestone 0.10.3 deleted

Add/Change #3262 (_html_splitlines reopens wrong tag in certain cases)

Author



Change Properties
<Author field>
Action
as closed
Next status will be 'reopened'
to The owner will change from jonas. Next status will be 'closed'
 
Note: See TracTickets for help on using tickets.