Bug 41181 - REGRESSION(53790): Neopets page with mismatched elements misrenders
Summary: REGRESSION(53790): Neopets page with mismatched elements misrenders
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL: http://www.neopets.com/games/arcade.p...
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-24 16:09 PDT by James Robinson
Modified: 2010-06-28 13:08 PDT (History)
3 users (show)

See Also:


Attachments
Patch (1.68 KB, patch)
2010-06-24 17:01 PDT, James Robinson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description James Robinson 2010-06-24 16:09:10 PDT
The limit of 5 iterations in http://trac.webkit.org/changeset/53790 appears to be too low for the neopets page in the bug URL.  Bumping the limit to 7 is sufficient to render this page correctly.
Comment 1 James Robinson 2010-06-24 16:11:30 PDT
Here's the time spend parsing fast/parser/residual-style-hang.html with different limits on my Mac Pro (intel xeon @ 2.26 GHz):

limit / time

 5 /  379ms
 6 /  494ms
 7 /  630ms
 8 /  770ms
 9 /  930ms
10 / 1073ms

This looks like a pretty much worst case scenario so I'd suggest raising the limit to 10 iterations just in case there are other sites out there that are broken.
Comment 2 James Robinson 2010-06-24 17:00:00 PDT
Gecko manages to render this page correctly with and without their HTML5 parser enabled.
Comment 3 James Robinson 2010-06-24 17:01:46 PDT
Created attachment 59710 [details]
Patch
Comment 4 mitz 2010-06-24 17:06:12 PDT
I don’t remember how the constant 5 was chosen. Will 10 re-introduce the hang seen in bug 34059?
Comment 5 James Robinson 2010-06-24 17:11:38 PDT
See comment #1 for timing data from the hanging test case.  A limit of 10 is slower than a limit of 5 but not by too much (it is still too low to be likely to see a beachball).
Comment 6 mitz 2010-06-24 17:15:23 PDT
The bug wasn’t about the test though, it was about a hang with some real-world content.
Comment 7 James Robinson 2010-06-24 17:27:13 PDT
We (Maciej, Hixie and I) chatted about this a bit in IRC.  We don't have any real-world websites that were seeing the hang (although we're pretty sure some did).  I believe that the test case Maciej created is or is close to the worst case for the fixup algorithm and don't expect that sites in the wild will be slower than it.  I hope so, anyway :)

I don't think we have too much choice, fundamentally we have to make the limit large enough to handle any websites that people care about.

The most recent Minefield nightly with the HTML5 parser enabled handles up to 100 nested "<b><div>"  instances before it starts dropping content.
Comment 8 James Robinson 2010-06-28 13:08:17 PDT
Comment on attachment 59710 [details]
Patch

Clearing flags on attachment: 59710

Committed r62029: <http://trac.webkit.org/changeset/62029>
Comment 9 James Robinson 2010-06-28 13:08:21 PDT
All reviewed patches have been landed.  Closing bug.