Edgewall Software
Modify

Ticket #3262 (closed defect: wontfix)

Opened 6 years ago

Last modified 5 years 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:
Release Notes:
API Changes:

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

Download all attachments as: .zip

Change History

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

Added test for api.py

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

First attempt at fix, diff against trunk@3423

comment:1 Changed 6 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 5 years ago by Tim Hatch <trac@…>

Proof of bug as exhibited on t.e.o

comment:2 Changed 5 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.

comment:3 Changed 5 years ago by cboos

  • Milestone set to 0.10.3

comment:4 Changed 5 years ago by Tim Hatch <trac@…>

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

comment:5 Changed 5 years ago by Tim Hatch <trac@…>

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

comment:6 Changed 5 years ago by sid

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

comment:7 Changed 5 years ago by cboos

  • Keywords mimeviewer highlighting added
  • Resolution set to wontfix
  • Status changed from new to closed

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).

comment:8 Changed 5 years ago by cboos

  • Milestone 0.10.3 deleted
View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
to The owner will be changed from jonas. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.