Bug 96385 - HTML parser fails to propertly close 4 identical nested formatting elements
Summary: HTML parser fails to propertly close 4 identical nested formatting elements
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Seidel (no email)
URL: http://www.thezorklibrary.com/history...
Keywords:
: 91509 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-09-11 07:05 PDT by Simon Pieters
Modified: 2012-09-12 17:34 PDT (History)
5 users (show)

See Also:


Attachments
Patch (7.33 KB, patch)
2012-09-12 14:38 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch (8.40 KB, patch)
2012-09-12 14:40 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch for landing (8.40 KB, patch)
2012-09-12 14:57 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Pieters 2012-09-11 07:05:26 PDT
URL has markup like this: 

<big><big><big><big>a</big></big></big></big>b 

See http://software.hixie.ch/utilities/js/live-dom-viewer/saved/1757

In Firefox the "b" is a child of body, in Opera/WebKit it's a child of the first <big> element (so the text gets bigger and bigger for each section on the page). 

In Opera we concluded that this was a spec violation and we have fixed it.
Comment 1 Adam Barth 2012-09-11 09:12:45 PDT
Isn't this just the noah's ark condition in action?
Comment 2 Ian 'Hixie' Hickson 2012-09-11 09:46:16 PDT
I think a misimplementation of Noah is the cause here. When you see the fourth <big>, you add it to the stack but not the list, and when you see the first </big> you end up popping both of the last two <big>s. The third and fourth </big>s get rid of the last two, and the last </big> ends up ignored. No?

Maybe we should increase the count to four per family...
Comment 3 Adam Barth 2012-09-11 10:43:29 PDT
Oh, did we goof up implementing the Noah's ark condition?  If so, we're happy to fix it.
Comment 4 Simon Pieters 2012-09-11 23:25:04 PDT
In Opera's case, we had missed to implement this part of AAA:

"If there is no such node, then abort these steps and instead act as described in the "any other end tag" entry below."

Hixie's comment doesn't quite match my understanding of the spec, but then again I don't quite follow AAA.

My understanding is that when seeing the fourth <big>, it gets added to the stack and the list but the oldest <big> gets dropped off the list (which I guess is equivalent to not adding the new one to the list). The first three </big>s run the AAA as normal, and the fourth hits the clause quoted above and gets treated as "any other end tag".

Since this page doesn't cause any elements to be reconstructed, what the limit is isn't supposed to make any difference here.
Comment 5 Ian 'Hixie' Hickson 2012-09-12 00:22:47 PDT
Uh, right, zcorpan is right. I forgot the order in which Noah lopped things off the list. Ignore comment 2.
Comment 6 Simon Pieters 2012-09-12 06:53:30 PDT
I added a few tests for the various limits to html5lib: https://code.google.com/p/html5lib/source/detail?r=5e044c0cfc8334f866d7a00b2cf90a935bd9a906
Comment 7 Adam Barth 2012-09-12 13:11:55 PDT
I think Eric is going to take a look at this bug.
Comment 8 Eric Seidel (no email) 2012-09-12 14:38:01 PDT
Created attachment 163694 [details]
Patch
Comment 9 Eric Seidel (no email) 2012-09-12 14:38:26 PDT
This only fixes one of our two AA bugs.  I'm looking at the second one now.
Comment 10 Eric Seidel (no email) 2012-09-12 14:40:18 PDT
Created attachment 163697 [details]
Patch
Comment 11 Adam Barth 2012-09-12 14:54:22 PDT
Comment on attachment 163697 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=163697&action=review

> Source/WebCore/html/parser/HTMLTreeBuilder.cpp:1429
> +                processAnyOtherEndTagForInBody(token);

Bad indent
Comment 12 Eric Seidel (no email) 2012-09-12 14:57:52 PDT
Created attachment 163705 [details]
Patch for landing
Comment 13 WebKit Review Bot 2012-09-12 15:39:53 PDT
Comment on attachment 163705 [details]
Patch for landing

Clearing flags on attachment: 163705

Committed r128373: <http://trac.webkit.org/changeset/128373>
Comment 14 WebKit Review Bot 2012-09-12 15:39:57 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 Eric Seidel (no email) 2012-09-12 17:34:48 PDT
*** Bug 91509 has been marked as a duplicate of this bug. ***