| Summary: | fast/parser/residual-style-hang.html hangs | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Adam Barth <abarth> | ||||||
| Component: | New Bugs | Assignee: | Adam Barth <abarth> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | eric, jamesr | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Other | ||||||||
| OS: | OS X 10.5 | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 41123 | ||||||||
| Attachments: |
|
||||||||
|
Description
Adam Barth
2010-07-25 13:45:34 PDT
Created attachment 62530 [details]
Patch
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.
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. 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. We should see what James things, but I think either of those would be fine ways to pass the test. Did you mean html5lib james? or chromium/rendering james? 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. I meant jamesr. He's the one who worked on this in the old parser. 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. Just capping the outer loop wasn't sufficient to prevent the hang. I'm not sure exactly how the newer and older algorithms relate. Comment on attachment 62530 [details]
Patch
Needs a layout test which shows behavior.
Created attachment 63522 [details]
Patch
Comment on attachment 63522 [details]
Patch
Thanks.
Comment on attachment 63522 [details] Patch Clearing flags on attachment: 63522 Committed r64702: <http://trac.webkit.org/changeset/64702> All reviewed patches have been landed. Closing bug. |