[webkit-reviews] review granted: [Bug 41319] Implement HTMLTreeBuilder::reconstructTheActiveFormattingElements : [Attachment 59960] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Eric Seidel <eric at webkit.org>'s
request for review:
Bug 41319: Implement HTMLTreeBuilder::reconstructTheActiveFormattingElements
https://bugs.webkit.org/show_bug.cgi?id=41319

Attachment 59960: Patch
https://bugs.webkit.org/attachment.cgi?id=59960&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
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.