WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
121018
Move <style scoped> code behind ENABLE(STYLE_SCOPED)
https://bugs.webkit.org/show_bug.cgi?id=121018
Summary
Move <style scoped> code behind ENABLE(STYLE_SCOPED)
Andreas Kling
Reported
2013-09-08 21:21:55 PDT
Move <style scoped> code behind ENABLE(STYLE_SCOPED)
Attachments
Patch
(11.65 KB, patch)
2013-09-08 21:25 PDT
,
Andreas Kling
no flags
Details
Formatted Diff
Diff
Patch
(12.99 KB, patch)
2013-09-08 21:28 PDT
,
Andreas Kling
darin
: review+
buildbot
: commit-queue-
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Andreas Kling
Comment 1
2013-09-08 21:25:21 PDT
Created
attachment 211006
[details]
Patch
Andreas Kling
Comment 2
2013-09-08 21:28:46 PDT
Created
attachment 211007
[details]
Patch
Darin Adler
Comment 3
2013-09-08 21:38:05 PDT
Comment on
attachment 211007
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=211007&action=review
r=me as long as it doesn’t break the build
> Source/WebCore/css/StyleResolver.cpp:469 > +#if ENABLE(STYLE_SCOPED) > if (parent->hasScopedHTMLStyleChild()) > return 0; > +#endif
The way I’d prefer to do this is to have hasScopedHTMLStyleChild be defined but be an inline function that returns false.
> Source/WebCore/css/StyleResolver.cpp:662 > +#if ENABLE(STYLE_SCOPED) > if (element->hasScopedHTMLStyleChild()) > return false; > +#endif
Ditto.
> Source/WebCore/css/StyleResolver.cpp:749 > +#if ENABLE(STYLE_SCOPED) > if (state.styledElement()->hasScopedHTMLStyleChild()) > return 0; > +#endif
Ditto.
> Source/WebCore/dom/Node.h:591 > +#if ENABLE(STYLE_SCOPED) > virtual void registerScopedHTMLStyleChild(); > virtual void unregisterScopedHTMLStyleChild(); > size_t numberOfScopedHTMLStyleChildren() const; > +#endif
Bizarre to have this all in Node!
> Source/WebCore/dom/Node.h:637 > +#if ENABLE(STYLE_SCOPED) > HasScopedHTMLStyleChildFlag = 1 << 22, > +#endif
Not sure this is needed.
> Source/WebCore/dom/ShadowRoot.h:75 > +#if ENABLE(STYLE_SCOPED) > virtual void registerScopedHTMLStyleChild() OVERRIDE; > virtual void unregisterScopedHTMLStyleChild() OVERRIDE; > +#endif
At least mark these final?
> Source/WebCore/html/HTMLStyleElement.cpp:82 > +#if ENABLE(STYLE_SCOPED) > else if (name == scopedAttr && ContextFeatures::styleScopedEnabled(&document())) > scopedAttributeChanged(!value.isNull()); > +#endif
Are we keeping this ContextFeatures thing at all? Who is using it in WebKit?
> Source/WebCore/html/HTMLStyleElement.cpp:187 > +bool HTMLStyleElement::scoped() const > +{ > + return fastHasAttribute(scopedAttr) && ContextFeatures::styleScopedEnabled(&document()); > +} > + > +void HTMLStyleElement::setScoped(bool scopedValue) > +{ > + setBooleanAttribute(scopedAttr, scopedValue); > +}
These should be generated by the bindings rather than writing explicit C++ code.
> Source/WebCore/html/HTMLStyleElement.cpp:213 > +#if ENABLE(STYLE_SCOPED) > if (m_scopedStyleRegistrationState == NotRegistered && (scoped() || isInShadowTree())) > registerWithScopingNode(scoped()); > +#endif
Lame how this calls scoped() twice. That’s not necessarily a fast function.
Build Bot
Comment 4
2013-09-08 22:03:12 PDT
Comment on
attachment 211007
[details]
Patch
Attachment 211007
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/1728263
Build Bot
Comment 5
2013-09-08 22:23:28 PDT
Comment on
attachment 211007
[details]
Patch
Attachment 211007
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.appspot.com/results/1735162
Build Bot
Comment 6
2013-09-12 17:30:18 PDT
Comment on
attachment 211007
[details]
Patch
Attachment 211007
[details]
did not pass win-ews (win): Output:
http://webkit-queues.appspot.com/results/1846072
Andreas Kling
Comment 7
2013-09-17 01:12:34 PDT
Committed
r155931
: <
http://trac.webkit.org/changeset/155931
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug