Attachment 356289 Details for Bug 191297 – Fixes the bug

[patch] Fixes the bug

bug-191297-20181130205510.patch (text/plain), 18.70 KB, created by Ryosuke Niwa on 2018-11-30 20:55:11 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2018-11-30 20:55:11 PST

Size: 18.70 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 238758)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,27 @@
>+2018-11-30  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        title attribute on style & link elements should be ignored inside a shadow tree
>+        https://bugs.webkit.org/show_bug.cgi?id=191297
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Fixed the by not setting the stylesheet's title even when the title content attribute is present
>+        or set on SVG/HTML style and link elements inside a shadow tree.
>+
>+        Test: fast/shadow-dom/stylesheet-title-in-shadow-tree.html
>+
>+        * dom/InlineStyleSheetOwner.cpp:
>+        (WebCore::InlineStyleSheetOwner::createSheet):
>+        * html/HTMLLinkElement.cpp:
>+        (WebCore::HTMLLinkElement::parseAttribute):
>+        (WebCore::HTMLLinkElement::initializeStyleSheet):
>+        * html/HTMLStyleElement.cpp:
>+        (WebCore::HTMLStyleElement::parseAttribute):
>+        * style/StyleScope.cpp:
>+        (WebCore::Style::Scope::collectActiveStyleSheets):
>+        * svg/SVGStyleElement.cpp:
>+        (WebCore::SVGStyleElement::parseAttribute):
>+
> 2018-11-30  Ryosuke Niwa  <rniwa@webkit.org>
> 
>         ShadowRoot should have styleSheets property
>Index: Source/WebCore/dom/InlineStyleSheetOwner.cpp
>===================================================================
>--- Source/WebCore/dom/InlineStyleSheetOwner.cpp	(revision 238758)
>+++ Source/WebCore/dom/InlineStyleSheetOwner.cpp	(working copy)
>@@ -189,7 +189,8 @@ void InlineStyleSheetOwner::createSheet(
>             ASSERT(cachedSheet->isCacheable());
>             m_sheet = CSSStyleSheet::createInline(*cachedSheet, element, m_startTextPosition);
>             m_sheet->setMediaQueries(mediaQueries.releaseNonNull());
>-            m_sheet->setTitle(element.title());
>+            if (!element.isInShadowTree())
>+                m_sheet->setTitle(element.title());
> 
>             sheetLoaded(element);
>             element.notifyLoadedSheetAndAllCriticalSubresources(false);
>@@ -203,7 +204,8 @@ void InlineStyleSheetOwner::createSheet(
> 
>     m_sheet = CSSStyleSheet::createInline(contents.get(), element, m_startTextPosition);
>     m_sheet->setMediaQueries(mediaQueries.releaseNonNull());
>-    m_sheet->setTitle(element.title());
>+    if (!element.isInShadowTree())
>+        m_sheet->setTitle(element.title());
> 
>     contents->parseString(text);
> 
>Index: Source/WebCore/html/HTMLLinkElement.cpp
>===================================================================
>--- Source/WebCore/html/HTMLLinkElement.cpp	(revision 238758)
>+++ Source/WebCore/html/HTMLLinkElement.cpp	(working copy)
>@@ -198,7 +198,7 @@ void HTMLLinkElement::parseAttribute(con
>         return;
>     }
>     if (name == titleAttr) {
>-        if (m_sheet)
>+        if (m_sheet && !isInShadowTree())
>             m_sheet->setTitle(value);
>         return;
>     }
>@@ -401,7 +401,8 @@ void HTMLLinkElement::initializeStyleShe
> 
>     m_sheet = CSSStyleSheet::create(WTFMove(styleSheet), *this, originClean);
>     m_sheet->setMediaQueries(MediaQuerySet::create(m_media, context));
>-    m_sheet->setTitle(title());
>+    if (!isInShadowTree())
>+        m_sheet->setTitle(title());
> 
>     if (!m_sheet->canAccessRules())
>         m_sheet->contents().setAsOpaque();
>Index: Source/WebCore/html/HTMLStyleElement.cpp
>===================================================================
>--- Source/WebCore/html/HTMLStyleElement.cpp	(revision 238758)
>+++ Source/WebCore/html/HTMLStyleElement.cpp	(working copy)
>@@ -78,7 +78,7 @@ Ref<HTMLStyleElement> HTMLStyleElement::
> 
> void HTMLStyleElement::parseAttribute(const QualifiedName& name, const AtomicString& value)
> {
>-    if (name == titleAttr && sheet())
>+    if (name == titleAttr && sheet() && !isInShadowTree())
>         sheet()->setTitle(value);
>     else if (name == mediaAttr) {
>         m_styleSheetOwner.setMedia(value);
>Index: Source/WebCore/style/StyleScope.cpp
>===================================================================
>--- Source/WebCore/style/StyleScope.cpp	(revision 238758)
>+++ Source/WebCore/style/StyleScope.cpp	(working copy)
>@@ -329,7 +329,7 @@ void Scope::collectActiveStyleSheets(Vec
>             sheet = downcast<ProcessingInstruction>(*node).sheet();
>         } else if (is<HTMLLinkElement>(*node) || is<HTMLStyleElement>(*node) || is<SVGStyleElement>(*node)) {
>             Element& element = downcast<Element>(*node);
>-            AtomicString title = element.attributeWithoutSynchronization(titleAttr);
>+            AtomicString title = element.isInShadowTree() ? nullAtom() : element.attributeWithoutSynchronization(titleAttr);
>             bool enabledViaScript = false;
>             if (is<HTMLLinkElement>(element)) {
>                 // <LINK> element
>Index: Source/WebCore/svg/SVGStyleElement.cpp
>===================================================================
>--- Source/WebCore/svg/SVGStyleElement.cpp	(revision 238758)
>+++ Source/WebCore/svg/SVGStyleElement.cpp	(working copy)
>@@ -94,7 +94,7 @@ String SVGStyleElement::title() const
> void SVGStyleElement::parseAttribute(const QualifiedName& name, const AtomicString& value)
> {
>     if (name == SVGNames::titleAttr) {
>-        if (sheet())
>+        if (sheet() && !isInShadowTree())
>             sheet()->setTitle(value);
>         return;
>     }
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 238758)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,19 @@
>+2018-11-30  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        title attribute on style & link elements should be ignored inside a shadow tree
>+        https://bugs.webkit.org/show_bug.cgi?id=191297
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Adde a W3C-style testharness.js test for a more comprehensive testing of the title content attribute
>+        on HTML link and style elements and SVG style element inside a shadow tree.
>+
>+        * TestExpectations:
>+        * fast/shadow-dom/resources/green-div.css: Added.
>+        * fast/shadow-dom/resources/red-div.css: Added.
>+        * fast/shadow-dom/stylesheet-title-in-shadow-tree-expected.txt: Added.
>+        * fast/shadow-dom/stylesheet-title-in-shadow-tree.html: Added.
>+
> 2018-11-30  Ryosuke Niwa  <rniwa@webkit.org>
> 
>         ShadowRoot should have styleSheets property
>Index: LayoutTests/TestExpectations
>===================================================================
>--- LayoutTests/TestExpectations	(revision 238758)
>+++ LayoutTests/TestExpectations	(working copy)
>@@ -2817,7 +2817,6 @@ imported/w3c/web-platform-tests/css/css-
> imported/w3c/web-platform-tests/css/css-text-decor/text-emphasis-position-above-left-001.xht [ ImageOnlyFailure ]
> imported/w3c/web-platform-tests/css/css-display/run-in/run-in-listitem-between-003.xht [ ImageOnlyFailure ]
> imported/w3c/web-platform-tests/css/css-display/run-in/run-in-basic-010.xht [ ImageOnlyFailure ]
>-imported/w3c/web-platform-tests/css/css-scoping/shadow-disabled-sheet-001.html [ ImageOnlyFailure ]
> imported/w3c/web-platform-tests/css/css-display/run-in/run-in-run-in-between-003.xht [ ImageOnlyFailure ]
> imported/w3c/web-platform-tests/css/css-multicol/multicol-breaking-003.html [ ImageOnlyFailure ]
> imported/w3c/web-platform-tests/css/css-display/run-in/run-in-contains-table-caption-001.xht [ ImageOnlyFailure ]
>Index: LayoutTests/fast/shadow-dom/stylesheet-title-in-shadow-tree-expected.txt
>===================================================================
>--- LayoutTests/fast/shadow-dom/stylesheet-title-in-shadow-tree-expected.txt	(nonexistent)
>+++ LayoutTests/fast/shadow-dom/stylesheet-title-in-shadow-tree-expected.txt	(working copy)
>@@ -0,0 +1,14 @@
>+
>+PASS title content attribute on a HTML style element should not set the title of a stylesheet inside a shadow tree of open mode 
>+PASS title content attribute on a HTML style element should not set the title of a stylesheet inside a shadow tree of closed mode 
>+PASS Setting title content attribute on a HTML style element should not set the title of a stylesheet inside a shadow tree of open mode 
>+PASS Setting title content attribute on a HTML style element should not set the title of a stylesheet inside a shadow tree of closed mode 
>+PASS title content attribute on a SVG style element should not set the title of a stylesheet inside a shadow tree of open mode 
>+PASS title content attribute on a SVG style element should not set the title of a stylesheet inside a shadow tree of closed mode 
>+PASS Setting title content attribute on a HTML style element should not set the title of a stylesheet inside a shadow tree of open mode 
>+PASS Setting title content attribute on a HTML style element should not set the title of a stylesheet inside a shadow tree of closed mode 
>+PASS title content attribute on a HTML style element should not set the title of a stylesheet inside a shadow tree of open mode 
>+PASS title content attribute on a HTML style element should not set the title of a stylesheet inside a shadow tree of closed mode 
>+PASS Setting title content attribute on a HTML style element should not set the title of a stylesheet inside a shadow tree of open mode 
>+PASS Setting title content attribute on a HTML style element should not set the title of a stylesheet inside a shadow tree of closed mode 
>+
>Index: LayoutTests/fast/shadow-dom/stylesheet-title-in-shadow-tree.html
>===================================================================
>--- LayoutTests/fast/shadow-dom/stylesheet-title-in-shadow-tree.html	(nonexistent)
>+++ LayoutTests/fast/shadow-dom/stylesheet-title-in-shadow-tree.html	(working copy)
>@@ -0,0 +1,136 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<meta name="author" title="Ryosuke Niwa" href="mailto:rniwa@webkit.org">
>+<meta name="assert" content="title content attribute should not affect the stylesheet's title inside a shadow tree">
>+<link rel="help" href="https://html.spec.whatwg.org/#attr-style-title">
>+<link rel="help" href="https://html.spec.whatwg.org/#attr-link-title">
>+<script src="../../resources/testharness.js"></script>
>+<script src="../../resources/testharnessreport.js"></script>
>+</head>
>+<body>
>+<div id="log"></div>
>+<script>
>+
>+function testStyleElementWithTitleInShadowTree(mode) {
>+    test(function () {
>+        const host = document.createElement('div');
>+        document.body.appendChild(host);
>+        const shadowRoot = host.attachShadow({'mode': mode});
>+        shadowRoot.innerHTML = '<style title="foo">div {width: 10px}</style><style title="bar">div {width: 20px}</style><div></div>';
>+        assert_equals(shadowRoot.styleSheets.length, 2);
>+        assert_equals(shadowRoot.styleSheets[0].title, null);
>+        assert_equals(shadowRoot.styleSheets[1].title, null);
>+        assert_equals(getComputedStyle(shadowRoot.querySelector('div')).width, '20px');
>+    }, `title content attribute on a HTML style element should not set the title of a stylesheet inside a shadow tree of ${mode} mode`);
>+}
>+
>+testStyleElementWithTitleInShadowTree('open');
>+testStyleElementWithTitleInShadowTree('closed');
>+
>+function testUpdatingTitleOnStyleElemenInShadowTree(mode) {
>+    test(function () {
>+        const host = document.createElement('div');
>+        document.body.appendChild(host);
>+        const shadowRoot = host.attachShadow({mode});
>+        shadowRoot.innerHTML = '<style>div {height: 10px}</style><style>div {height: 20px}</style><div></div>';
>+        assert_equals(shadowRoot.styleSheets.length, 2);
>+        assert_equals(shadowRoot.styleSheets[0].title, null);
>+        assert_equals(shadowRoot.styleSheets[1].title, null);
>+        shadowRoot.children[0].setAttribute('title', 'foo');
>+        shadowRoot.children[1].setAttribute('title', 'bar');
>+        assert_equals(shadowRoot.styleSheets.length, 2);
>+        assert_equals(shadowRoot.styleSheets[0].title, null);
>+        assert_equals(shadowRoot.styleSheets[1].title, null);
>+        assert_equals(getComputedStyle(shadowRoot.querySelector('div')).height, '20px');
>+    }, `Setting title content attribute on a HTML style element should not set the title of a stylesheet inside a shadow tree of ${mode} mode`);
>+}
>+
>+testUpdatingTitleOnStyleElemenInShadowTree('open');
>+testUpdatingTitleOnStyleElemenInShadowTree('closed');
>+
>+function testSVGStyleElementWithTitleInShadowTree(mode) {
>+    test(function () {
>+        const host = document.createElement('div');
>+        document.body.appendChild(host);
>+        const shadowRoot = host.attachShadow({'mode': mode});
>+        shadowRoot.innerHTML = '<svg><style title="foo">div {width: 10px}</style><style title="bar">div {width: 20px}</style></svg><div></div>';
>+        assert_equals(shadowRoot.styleSheets.length, 2);
>+        assert_equals(shadowRoot.styleSheets[0].title, null);
>+        assert_equals(shadowRoot.styleSheets[1].title, null);
>+        assert_equals(getComputedStyle(shadowRoot.querySelector('div')).width, '20px');
>+    }, `title content attribute on a SVG style element should not set the title of a stylesheet inside a shadow tree of ${mode} mode`);
>+}
>+
>+testSVGStyleElementWithTitleInShadowTree('open');
>+testSVGStyleElementWithTitleInShadowTree('closed');
>+
>+function testUpdatingTitleOnSVGStyleElemenInShadowTree(mode) {
>+    test(function () {
>+        const host = document.createElement('div');
>+        document.body.appendChild(host);
>+        const shadowRoot = host.attachShadow({mode});
>+        shadowRoot.innerHTML = '<svg><style>div {height: 10px}</style><style>div {height: 20px}</style></svg><div></div>';
>+        assert_equals(shadowRoot.styleSheets.length, 2);
>+        assert_equals(shadowRoot.styleSheets[0].title, null);
>+        assert_equals(shadowRoot.styleSheets[1].title, null);
>+        shadowRoot.firstChild.children[0].setAttribute('title', 'foo');
>+        shadowRoot.firstChild.children[1].setAttribute('title', 'bar');
>+        assert_equals(shadowRoot.styleSheets.length, 2);
>+        assert_equals(shadowRoot.styleSheets[0].title, null);
>+        assert_equals(shadowRoot.styleSheets[1].title, null);
>+        assert_equals(getComputedStyle(shadowRoot.querySelector('div')).height, '20px');
>+    }, `Setting title content attribute on a HTML style element should not set the title of a stylesheet inside a shadow tree of ${mode} mode`);
>+}
>+
>+testUpdatingTitleOnSVGStyleElemenInShadowTree('open');
>+testUpdatingTitleOnSVGStyleElemenInShadowTree('closed');
>+
>+function testLinkElementWithTitleInShadowTree(mode) {
>+    promise_test(async function () {
>+        const host = document.createElement('div');
>+        const shadowRoot = host.attachShadow({'mode': mode});
>+        shadowRoot.innerHTML = '<link rel="stylesheet" href="resources/green-div.css" title="foo"><link rel="stylesheet" href="resources/green-div.css" title="bar"><div></div>';
>+        const promises = Promise.all(Array.from(shadowRoot.querySelectorAll('link')).map((link) => {
>+            return new Promise((resolve) => link.addEventListener('load', resolve));
>+        }));
>+        document.body.appendChild(host);
>+        await promises;
>+        assert_equals(shadowRoot.styleSheets.length, 2);
>+        assert_equals(shadowRoot.styleSheets[0].title, null);
>+        assert_equals(shadowRoot.styleSheets[1].title, null);
>+        assert_equals(getComputedStyle(shadowRoot.querySelector('div')).backgroundColor, 'rgb(0, 128, 0)');
>+    }, `title content attribute on a HTML style element should not set the title of a stylesheet inside a shadow tree of ${mode} mode`);
>+}
>+
>+testLinkElementWithTitleInShadowTree('open');
>+testLinkElementWithTitleInShadowTree('closed');
>+
>+function testUpdatingTitleOnLinkElementInShadowTree(mode) {
>+    promise_test(async function () {
>+        const host = document.createElement('div');
>+        const shadowRoot = host.attachShadow({'mode': mode});
>+        shadowRoot.innerHTML = '<link rel="stylesheet" href="resources/green-div.css" title="foo"><link rel="stylesheet" href="resources/green-div.css" title="bar"><div></div>';
>+        const promises = Promise.all(Array.from(shadowRoot.querySelectorAll('link')).map((link) => {
>+            return new Promise((resolve) => link.addEventListener('load', resolve));
>+        }));
>+        document.body.appendChild(host);
>+        await promises;
>+        assert_equals(shadowRoot.styleSheets.length, 2);
>+        assert_equals(shadowRoot.styleSheets[0].title, null);
>+        assert_equals(shadowRoot.styleSheets[1].title, null);
>+        shadowRoot.children[0].setAttribute('title', 'foo');
>+        shadowRoot.children[1].setAttribute('title', 'bar');
>+        assert_equals(shadowRoot.styleSheets.length, 2);
>+        assert_equals(shadowRoot.styleSheets[0].title, null);
>+        assert_equals(shadowRoot.styleSheets[1].title, null);
>+        assert_equals(getComputedStyle(shadowRoot.querySelector('div')).backgroundColor, 'rgb(0, 128, 0)');
>+    }, `Setting title content attribute on a HTML style element should not set the title of a stylesheet inside a shadow tree of ${mode} mode`);
>+}
>+
>+testUpdatingTitleOnLinkElementInShadowTree('open');
>+testUpdatingTitleOnLinkElementInShadowTree('closed');
>+
>+</script>
>+</body>
>+</html>
>Index: LayoutTests/fast/shadow-dom/resources/green-div.css
>===================================================================
>--- LayoutTests/fast/shadow-dom/resources/green-div.css	(nonexistent)
>+++ LayoutTests/fast/shadow-dom/resources/green-div.css	(working copy)
>@@ -0,0 +1 @@
>+div { background-color: green; }
>Index: LayoutTests/fast/shadow-dom/resources/red-div.css
>===================================================================
>--- LayoutTests/fast/shadow-dom/resources/red-div.css	(nonexistent)
>+++ LayoutTests/fast/shadow-dom/resources/red-div.css	(working copy)
>@@ -0,0 +1 @@
>+div { background-color: red; }
>Index: LayoutTests/imported/w3c/ChangeLog
>===================================================================
>--- LayoutTests/imported/w3c/ChangeLog	(revision 238774)
>+++ LayoutTests/imported/w3c/ChangeLog	(working copy)
>@@ -1,3 +1,14 @@
>+2018-11-30  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        title attribute on style & link elements should be ignored inside a shadow tree
>+        https://bugs.webkit.org/show_bug.cgi?id=191297
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Rebaseline the test case that's now passing.
>+
>+        * web-platform-tests/css/css-scoping/stylesheet-title-002-expected.txt:
>+
> 2018-11-30  Ryosuke Niwa  <rniwa@webkit.org>
> 
>         ShadowRoot should have styleSheets property
>Index: LayoutTests/imported/w3c/web-platform-tests/css/css-scoping/stylesheet-title-002-expected.txt
>===================================================================
>--- LayoutTests/imported/w3c/web-platform-tests/css/css-scoping/stylesheet-title-002-expected.txt	(revision 238758)
>+++ LayoutTests/imported/w3c/web-platform-tests/css/css-scoping/stylesheet-title-002-expected.txt	(working copy)
>@@ -1,3 +1,3 @@
> 
>-FAIL Title attribute in stylesheets not in the document tree is ignored assert_equals: expected 3 but got 2
>+PASS Title attribute in stylesheets not in the document tree is ignored 
>

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 191297: 356289