Bug 41716 - Factor HTMLConstructionSite out of HTMLTreeBuilder
Summary: Factor HTMLConstructionSite out of HTMLTreeBuilder
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: Adam Barth
URL:
Keywords:
Depends on:
Blocks: 41123
  Show dependency treegraph
 
Reported: 2010-07-06 15:02 PDT by Adam Barth
Modified: 2010-07-06 15:51 PDT (History)
1 user (show)

See Also:


Attachments
Patch (80.10 KB, patch)
2010-07-06 15:03 PDT, Adam Barth
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2010-07-06 15:02:11 PDT
Factor HTMLConstructionSite out of HTMLTreeBuilder
Comment 1 Adam Barth 2010-07-06 15:03:59 PDT
Created attachment 60655 [details]
Patch
Comment 2 Eric Seidel (no email) 2010-07-06 15:34:50 PDT
Comment on attachment 60655 [details]
Patch

WebCore/html/HTMLTreeBuilder.h: 
 +      // FIXME: FragmentScriptingPermission is a HACK for platform/Pasteboard.
I recommend leaving this code and FIXME in  the TreeBuilder.  It exists to support the legacy treebuilder usage and can be removed from the base class when we either factor our the legacy shim, or remove the legacy shim entirely.

WebCore/html/HTMLTreeBuilder.h:113
 +      // FIXME: FragmentScriptingPermission is a HACK for platform/Pasteboard.
This FIXME doesn't apply to this usage, only to the old usage for the legacy shim, since the legacy shim was emulating hackish behavior of the old tokenizer.

WebCore/html/HTMLTreeBuilder.h:110
 +      mutable HTMLElementStack m_openElements;
Confused why these are mutable?

WebCore/html/HTMLTreeBuilder.h:91
 +      PassRefPtr<ChildType> attach(Node* parent, PassRefPtr<ChildType> prpChild)
This doesn't need to be in the header since its only used from this class.

WebCore/html/HTMLTreeBuilder.h:86
 +      // FIXME: Remove this method.  It exists only to service the legacy tree builder.
I think we should leave this hack on the TreeBuilder instead of moving it here.  HTMLContructionSite has valid reasons to know the m_fragmentScriptingPermission.  The FIXME from before was about the shim needing to do hacky things.

WebCore/html/HTMLTreeBuilder.h:84
 +      void setForm(Element* form) { m_form = form; }
PassRefPtr<Element>?

WebCore/html/HTMLTreeBuilder.h:82
 +      PassRefPtr<Element> releaseForm() { return m_form.release(); }
We're inconsistent about take vs. release.

WebCore/html/HTMLTreeBuilder.cpp:2313
 +  void HTMLTreeBuilder::generateImpliedEndTags()
Seems this should move onto the site, no?

WebCore/html/HTMLTreeBuilder.cpp:2307
 +  void HTMLTreeBuilder::generateImpliedEndTagsWithExclusion(const AtomicString& tagName)
Seems this should move onto site, no?

WebCore/html/HTMLTreeBuilder.cpp:@
 +  void HTMLTreeBuilder::reconstructTheActiveFormattingElements()
Seems this belongs on the site as well.

WebCore/html/HTMLTreeBuilder.cpp:2257
 +  bool HTMLTreeBuilder::indexOfFirstUnopenFormattingElement(unsigned& firstUnopenElementIndex) const
And this...

WebCore/html/HTMLTreeBuilder.cpp:1784
 +              ASSERT(m_site.currentElement()->tagQName() == noscriptTag);
These shoudl be hasTagName(...)

Please consider the above comments.
Comment 3 Adam Barth 2010-07-06 15:51:29 PDT
Committed r62599: <http://trac.webkit.org/changeset/62599>