Bug 41319

Summary: Implement HTMLTreeBuilder::reconstructTheActiveFormattingElements
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: New BugsAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, commit-queue, dbates, eric, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 41123    
Attachments:
Description Flags
Patch
none
Patch
none
Patch abarth: review+

Description Eric Seidel (no email) 2010-06-28 17:05:53 PDT
Implement HTMLTreeBuilder::reconstructTheActiveFormattingElements
Comment 1 Eric Seidel (no email) 2010-06-28 17:08:35 PDT
Created attachment 59960 [details]
Patch
Comment 2 Adam Barth 2010-06-28 17:20:18 PDT
Comment on attachment 59960 [details]
Patch

I'd rather this patch had tests.  If you wait 15 minutes, we'll have the harness landed...

WebCore/html/HTMLTreeBuilder.cpp:870
 +      // Spec: Possible active formatting elements include:
I'd remove the word "spec" here.

WebCore/html/HTMLTreeBuilder.cpp:926
 +      for (; previousEntryIndex > 0; previousEntryIndex--) {
Can we make this a while or a function that returns the answer?  Also, predecrement!

WebCore/html/HTMLTreeBuilder.cpp:932
 +      for (unsigned unopenEntryIndex = previousEntryIndex + 1; unopenEntryIndex < m_activeFormattingElements.size(); ++unopenEntryIndex) {
Can we assert that previousEntryIndex is not the last guy in the list?

WebCore/html/HTMLTreeBuilder.h:155
 +          bool contains(Element* element)
We're going to need a cache here eventually.

WebCore/html/HTMLTreeBuilder.h:257
 +      };
Blank line below this line.
Comment 3 Eric Seidel (no email) 2010-06-28 18:11:11 PDT
Created attachment 59967 [details]
Patch
Comment 4 Adam Barth 2010-06-28 18:22:23 PDT
Comment on attachment 59967 [details]
Patch

WebCore/html/HTMLTreeBuilder.h:217
 +      PassRefPtr<Element> createElement(AtomicHTMLToken&);
I would have put a blank line here to paragraph these.
Comment 5 WebKit Commit Bot 2010-06-28 19:52:44 PDT
Comment on attachment 59967 [details]
Patch

Clearing flags on attachment: 59967

Committed r62077: <http://trac.webkit.org/changeset/62077>
Comment 6 WebKit Commit Bot 2010-06-28 19:52:48 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Daniel Bates 2010-06-28 20:23:11 PDT
This broke the Windows Release and Debug build bots.

For completeness:

Windows Release Stdio: <http://build.webkit.org/builders/Windows%20Release%20%28Build%29/builds/1051/steps/compile-webkit/logs/stdio>

Windows Debug Stdio: <http://build.webkit.org/builders/Windows%20Debug%20%28Build%29/builds/18218/steps/compile-webkit/logs/stdio>
Comment 8 Daniel Bates 2010-06-28 20:34:48 PDT
Landed attempt to fix the build in change-set 62080 <https://trac.webkit.org/changeset/62080>.
Comment 9 WebKit Review Bot 2010-06-28 21:13:15 PDT
http://trac.webkit.org/changeset/62080 might have broken Chromium Mac Release
Comment 10 Eric Seidel (no email) 2010-06-28 21:26:34 PDT
Created attachment 59981 [details]
Patch
Comment 11 Adam Barth 2010-06-28 21:30:15 PDT
Comment on attachment 59981 [details]
Patch

I don't love it.  I'd rather have iterators, but ok.
Comment 12 Eric Seidel (no email) 2010-06-28 21:36:19 PDT
Committed r62082: <http://trac.webkit.org/changeset/62082>