Bug 13603 – style leaks in washingtonpost.com

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Antti Koivisto]

Open the attached webarchive, mouse over the main story links. Red underlining spreads to other stories too. The problems does not happen on this page normally but I have seen daily versions where it does occur before too, so it is not a one time thing. Works fine in ie/ffx.

Not a regression but marking P1 since it is a major site and the problem is highly visible when it happens.

[Antti Koivisto]

Created attachment 14374 [details]
webarchive where the problem is visible

[Alexey Proskuryakov]

Created attachment 14376 [details]
reduced test case

For this test case, html5lib produces the same tree as Firefox.

Is Layout and Rendering the correct component? Is this not a duplicate of bug 12808 (or bug 12861)?

[Alexey Proskuryakov]

...or even bug 8750?

[Eric Seidel (no email)]

Surprised this is a P1, certainly not a regression.  I guess just that it's wpost.com

[Antti Koivisto]

Seemed appropriate since it is wp and the problem is very ugly. Also there seems to be tons of duplicates.

Created attachment 14520 [details]
[WIP] First cut at a fix

Not even tested thoroughly, this patch tries to fix the "one block" limitation of handleResidualStyleCloseTagAcrossBlocks() mentioned in bug 8750 comment #2. It appears to fix the reductions in this bug, in bug 8750, in bug 12808 and in bug 12861. Not sure that it works in some more complicated cases.

Comment on attachment 14520 [details]
[WIP] First cut at a fix

This is leaking newNode. I think

+            prevMaxElem->didRefNode = false;

should say true.

Created attachment 14534 [details]
[WIP] non-leaking version

Created attachment 14539 [details]
[WIP] updated comments and fixed behavior for "unaffected" tags

Created attachment 14540 [details]
Attachment 14539 [details] without whitespace-only changes

This is a more readable version of attachment 14539 [details].

Created attachment 14541 [details]
Handle closing of residual style across multiple blocks

Code changes are the same as in attachment 14539 [details]. No test regressions.

Includes a change log and a test. The expected results in the test are identical to the results produced by html5lib-0.9. They differ from the results given by Firefox in some cases, mostly because Firefox does not insert an empty inline between two consecutive block tags, e.g. <i><div><pre>foo</i> turns into <i></i><div><pre><i>foo</i> in Firefox, but html5lib and this patch parse it as <i></i><div><i></i><pre><i>foo</i>.

[Dave Hyatt]

I would prefer it if we could avoid the empty inlines.  As a general rule we should be shooting for the most minimal set of tags possible to ensure the correct rendering.  The empty inlines are obviously useless, so it would be good if we could detect this scenario and avoid creating them.

We can make sure that HTML5 is updated to reflect this too, since I'm sure Hixie would agree that the algorithm should not produce useless extra tag sequences.

[Dave Hyatt]

Comment on attachment 14541 [details]
Handle closing of residual style across multiple blocks

Patch looks good, but I'd like to avoid generating inlines when it isn't necessary.

[Dave Hyatt]

Comment on attachment 14541 [details]
Handle closing of residual style across multiple blocks

I'm going to r+ this.

We should maybe file a followup bug about stripping empty inlines whereever we think we can get away with it.

[Antti Koivisto]

<rdar://problem/5200418>

(In reply to comment #15)
> We should maybe file a followup bug about stripping empty inlines whereever we
> think we can get away with it.
 
Bug 13712.

[Mark Rowe (bdash)]

Landed in r21472.