Bug 42950 – fast/parser/residual-style-hang.html hangs

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Adam Barth]

fast/parser/residual-style-hang.html hangs

[Adam Barth]

Created attachment 62530 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 62530 [details]
Patch

OK, but it would be better to have a test which explicitly showed this limit.  I.e. what is the resulting DOM from such a limit?

I believe you could produce this with:

<a>
<b><i>
<b><i>
<b><i>
<b><i>
<b><i>
<p>
</a>

Would be best to document.write() those and give them all unique ids of course.  Would end up being a "big" DOM, but I think seeing it in a test would help any future tweaks to this be better understood.  A test which simply hangs isn't super helpful.

[Adam Barth]

We could.  I think the desired behavior is to pass all the other tests and to not hang that test, which is what we're doing now.

[Eric Seidel (no email)]

Sure, but there are multiple ways to pass that test.  One of them would be to produce a full DOM, slowly.  Or to ignore the middle tags instead of the end tags.  I'm not sure that it matters, but the current testing is a pretty coarse filter for determining desired behavior.

[Adam Barth]

We should see what James things, but I think either of those would be fine ways to pass the test.

[Eric Seidel (no email)]

Did you mean html5lib james? or chromium/rendering james?

[James Robinson]

I think it'd be better to have a test that checked that the output DOM is correct and requires 10 iterations - or at least 7, since we've seen pages in the wild that require 7 iterations of the adoption agency to render correctly https://bugs.webkit.org/show_bug.cgi?id=41181.

residual-style-hang is at test that we don't hang forever, not for any particular behavior beyond that.

[Adam Barth]

I meant jamesr.  He's the one who worked on this in the old parser.

[James Robinson]

Do we want a cap on the outter and inner loops? The legacy tree builder only caps the equivalent of the outter loop, I believe.  I don't fully grok the fixup algorithm enough to know when this will produce different behavior but I think we should still do 10 rounds of fixup even if the misnested DOM is very deep.

[Adam Barth]

Just capping the outer loop wasn't sufficient to prevent the hang.  I'm not sure exactly how the newer and older algorithms relate.

[Eric Seidel (no email)]

Comment on attachment 62530 [details]
Patch

Needs a layout test which shows behavior.

[Adam Barth]

Created attachment 63522 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 63522 [details]
Patch

Thanks.

[Adam Barth]

Comment on attachment 63522 [details]
Patch

Clearing flags on attachment: 63522

Committed r64702: <http://trac.webkit.org/changeset/64702>

[Adam Barth]

All reviewed patches have been landed.  Closing bug.