Summary: | Disable adding a shadow root to elements having a dynamic built-in shadow root. | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Shinya Kawanaka <shinyak> | ||||||||||
Component: | DOM | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | dglazkov, dominicc, eric, tkent, tsepez, webkit.review.bot | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 77503, 77931 | ||||||||||||
Attachments: |
|
Description
Shinya Kawanaka
2012-02-06 22:17:15 PST
Created attachment 125779 [details]
Patch
Comment on attachment 125779 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=125779&action=review Temporarily turning off author shadow roots for elements that use UA shadow roots sounds like a good idea. > Source/WebCore/dom/ShadowRoot.h:49 > + enum CheckUserShadowRootAllowance { It might be better to rename this to reflect the caller intent, eg enum (mumble) { CreatingUserAgentShadowRoot, CreatingAuthorShadowRoot } ? Related Chromium bug: <http://code.google.com/p/chromium/issues/detail?id=113168> Comment on attachment 125779 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=125779&action=review >> Source/WebCore/dom/ShadowRoot.h:49 >> + enum CheckUserShadowRootAllowance { > > It might be better to rename this to reflect the caller intent, eg > > enum (mumble) { > CreatingUserAgentShadowRoot, > CreatingAuthorShadowRoot > } > > ? I like Dominic's idea here. Other than that, the patch looks fine. Related to <http://code.google.com/p/chromium/issues/detail?id=113168> and <http://code.google.com/p/chromium/issues/detail?id=113174> downstream in Chromium. Since I want to add new flag which will introduced in Bug 77929, I'll update this after Bug 77929 is landed. Created attachment 126242 [details]
Patch
Comment on attachment 126242 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=126242&action=review > Source/WebCore/dom/ShadowRoot.cpp:85 > + return false; > + maybe TEXTAREA, too. > Source/WebCore/dom/ShadowRoot.h:53 > + static PassRefPtr<ShadowRoot> create(Element*, ShadowRootCreationPurpose, ExceptionCode&); We had better add a default argument to ExceptionCode&. ..., ExceptionCode& = ASSERT_NO_EXCEPTION); Created attachment 126245 [details]
Patch
Comment on attachment 126245 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=126245&action=review > Source/WebCore/dom/Element.cpp:1202 > + return ShadowRoot::create(this, ShadowRoot::CreatingUserAgentShadowRoot, ASSERT_NO_EXCEPTION).get(); You can remove ASSERT_NO_EXCEPTION argument. > Source/WebCore/html/HTMLDetailsElement.cpp:113 > + RefPtr<ShadowRoot> root = ShadowRoot::create(this, ShadowRoot::CreatingUserAgentShadowRoot, ASSERT_NO_EXCEPTION); ditto. > Source/WebCore/html/HTMLKeygenElement.cpp:89 > + RefPtr<ShadowRoot> root = ShadowRoot::create(this, ShadowRoot::CreatingUserAgentShadowRoot, ASSERT_NO_EXCEPTION); ditto. > Source/WebCore/html/HTMLMeterElement.cpp:244 > + RefPtr<ShadowRoot> root = ShadowRoot::create(this, ShadowRoot::CreatingUserAgentShadowRoot, ASSERT_NO_EXCEPTION); ditto. > Source/WebCore/html/HTMLSummaryElement.cpp:77 > + RefPtr<ShadowRoot> root = ShadowRoot::create(this, ShadowRoot::CreatingUserAgentShadowRoot, ASSERT_NO_EXCEPTION); ditto. > Source/WebCore/html/HTMLTextAreaElement.cpp:88 > + RefPtr<ShadowRoot> root = ShadowRoot::create(this, ShadowRoot::CreatingUserAgentShadowRoot, ASSERT_NO_EXCEPTION); ditto. Created attachment 126257 [details]
Patch for landing
(In reply to comment #10) > (From update of attachment 126245 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=126245&action=review > > > Source/WebCore/dom/Element.cpp:1202 > > + return ShadowRoot::create(this, ShadowRoot::CreatingUserAgentShadowRoot, ASSERT_NO_EXCEPTION).get(); > > You can remove ASSERT_NO_EXCEPTION argument. > > > Source/WebCore/html/HTMLDetailsElement.cpp:113 > > + RefPtr<ShadowRoot> root = ShadowRoot::create(this, ShadowRoot::CreatingUserAgentShadowRoot, ASSERT_NO_EXCEPTION); > > ditto. > > > Source/WebCore/html/HTMLKeygenElement.cpp:89 > > + RefPtr<ShadowRoot> root = ShadowRoot::create(this, ShadowRoot::CreatingUserAgentShadowRoot, ASSERT_NO_EXCEPTION); > > ditto. > > > Source/WebCore/html/HTMLMeterElement.cpp:244 > > + RefPtr<ShadowRoot> root = ShadowRoot::create(this, ShadowRoot::CreatingUserAgentShadowRoot, ASSERT_NO_EXCEPTION); > > ditto. > > > Source/WebCore/html/HTMLSummaryElement.cpp:77 > > + RefPtr<ShadowRoot> root = ShadowRoot::create(this, ShadowRoot::CreatingUserAgentShadowRoot, ASSERT_NO_EXCEPTION); > > ditto. > > > Source/WebCore/html/HTMLTextAreaElement.cpp:88 > > + RefPtr<ShadowRoot> root = ShadowRoot::create(this, ShadowRoot::CreatingUserAgentShadowRoot, ASSERT_NO_EXCEPTION); > > ditto. Done. Comment on attachment 126257 [details] Patch for landing Clearing flags on attachment: 126257 Committed r107202: <http://trac.webkit.org/changeset/107202> All reviewed patches have been landed. Closing bug. *** Bug 78138 has been marked as a duplicate of this bug. *** |