Summary: | Factor HTMLConstructionSite out of HTMLTreeBuilder | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adam Barth <abarth> | ||||
Component: | New Bugs | Assignee: | Adam Barth <abarth> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | eric | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Other | ||||||
OS: | OS X 10.5 | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 41123 | ||||||
Attachments: |
|
Description
Adam Barth
2010-07-06 15:02:11 PDT
Created attachment 60655 [details]
Patch
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.
Committed r62599: <http://trac.webkit.org/changeset/62599> |