Bug 41319 - Implement HTMLTreeBuilder::reconstructTheActiveFormattingElements
Summary: Implement HTMLTreeBuilder::reconstructTheActiveFormattingElements
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Eric Seidel (no email)
URL:
Keywords:
Depends on:
Blocks: 41123
  Show dependency treegraph
 
Reported: 2010-06-28 17:05 PDT by Eric Seidel (no email)
Modified: 2010-06-28 21:36 PDT (History)
5 users (show)

See Also:


Attachments
Patch (4.98 KB, patch)
2010-06-28 17:08 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch (7.90 KB, patch)
2010-06-28 18:11 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch (4.04 KB, patch)
2010-06-28 21:26 PDT, Eric Seidel (no email)
abarth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>