diff --git a/third_party/blink/renderer/core/css/css_style_sheet.cc b/third_party/blink/renderer/core/css/css_style_sheet.cc index 096dc2743b74b9..edb16d23f1cc49 100644 --- a/third_party/blink/renderer/core/css/css_style_sheet.cc +++ b/third_party/blink/renderer/core/css/css_style_sheet.cc @@ -40,6 +40,7 @@ #include "third_party/blink/renderer/core/html/html_link_element.h" #include "third_party/blink/renderer/core/html/html_style_element.h" #include "third_party/blink/renderer/core/html_names.h" +#include "third_party/blink/renderer/core/inspector/console_message.h" #include "third_party/blink/renderer/core/probe/core_probes.h" #include "third_party/blink/renderer/core/svg/svg_style_element.h" #include "third_party/blink/renderer/platform/bindings/exception_state.h" @@ -358,8 +359,8 @@ unsigned CSSStyleSheet::insertRule(const String& rule_string, RuleMutationScope mutation_scope(this); if (rule->IsImportRule() && is_constructed_) { exception_state.ThrowDOMException( - DOMExceptionCode::kNotAllowedError, - "Can't insert @import rules to a constructed stylesheet."); + DOMExceptionCode::kSyntaxError, + "Can't insert @import rules into a constructed stylesheet."); return 0; } bool success = contents_->WrapperInsertRule(rule, index); @@ -460,15 +461,11 @@ ScriptPromise CSSStyleSheet::replace(ScriptState* script_state, DOMExceptionCode::kNotAllowedError, "Can't call replace on non-constructed CSSStyleSheets.")); } - // Parses the text synchronously, loads import rules asynchronously. - SetText(text, true /* allow_import_rules */, nullptr); - if (!IsLoading()) - return ScriptPromise::Cast(script_state, ToV8(this, script_state)); - // We're loading a stylesheet that contains @import rules. This is deprecated. - Deprecation::CountDeprecation(OwnerDocument(), - WebFeature::kCssStyleSheetReplaceWithImport); - resolver_ = MakeGarbageCollected(script_state); - return resolver_->Promise(); + SetText(text, CSSImportRules::kIgnoreWithWarning); + // We currently parse synchronously, and since @import support was removed, + // nothing else happens asynchronously. This API is left as-is, so that future + // async parsing can still be supported here. + return ScriptPromise::Cast(script_state, ToV8(this, script_state)); } void CSSStyleSheet::replaceSync(const String& text, @@ -478,7 +475,7 @@ void CSSStyleSheet::replaceSync(const String& text, DOMExceptionCode::kNotAllowedError, "Can't call replaceSync on non-constructed CSSStyleSheets."); } - SetText(text, false /* allow_import_rules */, &exception_state); + SetText(text, CSSImportRules::kIgnoreWithWarning); } void CSSStyleSheet::ResolveReplacePromiseIfNeeded(bool load_error_occured) { @@ -569,19 +566,21 @@ void CSSStyleSheet::SetLoadCompleted(bool completed) { contents_->ClientLoadStarted(this); } -void CSSStyleSheet::SetText(const String& text, - bool allow_import_rules, - ExceptionState* exception_state) { +void CSSStyleSheet::SetText(const String& text, CSSImportRules import_rules) { child_rule_cssom_wrappers_.clear(); CSSStyleSheet::RuleMutationScope mutation_scope(this); contents_->ClearRules(); - if (contents_->ParseString(text, allow_import_rules) == - ParseSheetResult::kHasUnallowedImportRule) { - DCHECK(exception_state); - exception_state->ThrowDOMException(DOMExceptionCode::kNotAllowedError, - "@import rules are not allowed when " - "creating stylesheet synchronously."); + bool allow_imports = import_rules == CSSImportRules::kAllow; + if (contents_->ParseString(text, allow_imports) == + ParseSheetResult::kHasUnallowedImportRule && + import_rules == CSSImportRules::kIgnoreWithWarning) { + OwnerDocument()->AddConsoleMessage(MakeGarbageCollected( + mojom::blink::ConsoleMessageSource::kJavaScript, + mojom::blink::ConsoleMessageLevel::kWarning, + "@import rules are not allowed here. See " + "https://github.com/WICG/construct-stylesheets/issues/" + "119#issuecomment-588352418.")); } } diff --git a/third_party/blink/renderer/core/css/css_style_sheet.h b/third_party/blink/renderer/core/css/css_style_sheet.h index 84d53558a22d83..238b5d2ddcfcab 100644 --- a/third_party/blink/renderer/core/css/css_style_sheet.h +++ b/third_party/blink/renderer/core/css/css_style_sheet.h @@ -48,6 +48,11 @@ class ScriptPromiseResolver; class ScriptState; class StyleSheetContents; +enum class CSSImportRules { + kAllow, + kIgnoreWithWarning, +}; + class CORE_EXPORT CSSStyleSheet final : public StyleSheet { DEFINE_WRAPPERTYPEINFO(); @@ -189,9 +194,7 @@ class CORE_EXPORT CSSStyleSheet final : public StyleSheet { bool SheetLoaded(); bool LoadCompleted() const { return load_completed_; } void StartLoadingDynamicSheet(); - // If `allow_import_rules` is false, an ExceptionState pointer must be - // provided, otherwise it can be null. - void SetText(const String&, bool allow_import_rules, ExceptionState*); + void SetText(const String&, CSSImportRules); void SetMedia(MediaList*); void SetAlternateFromConstructor(bool); bool CanBeActivated(const String& current_preferrable_name) const; diff --git a/third_party/blink/renderer/core/inspector/inspector_style_sheet.cc b/third_party/blink/renderer/core/inspector/inspector_style_sheet.cc index 7769a1aa17fa7d..ce12579c49de53 100644 --- a/third_party/blink/renderer/core/inspector/inspector_style_sheet.cc +++ b/third_party/blink/renderer/core/inspector/inspector_style_sheet.cc @@ -997,7 +997,7 @@ String InspectorStyleSheet::FinalURL() { bool InspectorStyleSheet::SetText(const String& text, ExceptionState& exception_state) { InnerSetText(text, true); - page_style_sheet_->SetText(text, true /* allow_import_rules */, nullptr); + page_style_sheet_->SetText(text, CSSImportRules::kAllow); OnStyleSheetTextChanged(); return true; } diff --git a/third_party/blink/web_tests/cssom/deprecated-stylesheet-replace-import-expected.txt b/third_party/blink/web_tests/cssom/deprecated-stylesheet-replace-import-expected.txt deleted file mode 100644 index 759d43db85d524..00000000000000 --- a/third_party/blink/web_tests/cssom/deprecated-stylesheet-replace-import-expected.txt +++ /dev/null @@ -1,5 +0,0 @@ -CONSOLE MESSAGE: line 6: Start script - there should be a deprecation message below about replace() and @import. -CONSOLE WARNING: line 9: Support for calls to CSSStyleSheet.replace() with stylesheet text that includes @import has been deprecated, and will be removed in M84, around June 2020. See https://chromestatus.com/feature/4735925877735424 for more details. -CONSOLE MESSAGE: line 15: Finish script -CONSOLE MESSAGE: line 10: Replace resolved - diff --git a/third_party/blink/web_tests/cssom/deprecated-stylesheet-replace-import.html b/third_party/blink/web_tests/cssom/deprecated-stylesheet-replace-import.html deleted file mode 100644 index 4c8adab5db39a6..00000000000000 --- a/third_party/blink/web_tests/cssom/deprecated-stylesheet-replace-import.html +++ /dev/null @@ -1,16 +0,0 @@ - - - diff --git a/third_party/blink/web_tests/external/wpt/css/cssom/CSSStyleSheet-constructable-expected.txt b/third_party/blink/web_tests/external/wpt/css/cssom/CSSStyleSheet-constructable-expected.txt index 160bd9243bad6a..2f84a0d554569c 100644 --- a/third_party/blink/web_tests/external/wpt/css/cssom/CSSStyleSheet-constructable-expected.txt +++ b/third_party/blink/web_tests/external/wpt/css/cssom/CSSStyleSheet-constructable-expected.txt @@ -1,8 +1,8 @@ This is a testharness.js-based test. PASS document.adoptedStyleSheets should initially have length 0. -FAIL new CSSStyleSheet produces empty CSSStyleSheet assert_equals: expected (object) null but got (string) "" -FAIL title cannot be set in the CSSStyleSheet constructor assert_equals: expected (object) null but got (string) "something" -FAIL CSSStyleSheet.replace produces Promise assert_equals: expected (object) null but got (string) "" +FAIL new CSSStyleSheet produces empty CSSStyleSheet assert_equals: The title attribute must return the title or null if title is the empty string expected (object) null but got (string) "" +FAIL title can be set in the CSSStyleSheet constructor assert_equals: title and alternate are not supported by the constructor. https://github.com/WICG/construct-stylesheets/issues/105 expected (object) null but got (string) "something" +FAIL CSSStyleSheet.replace produces Promise assert_equals: The title attribute must return the title or null if title is the empty string expected (object) null but got (string) "" PASS Constructed style sheets can be applied on document PASS Constructed style sheets can be applied on shadow root PASS Re-attaching shadow host with adopted stylesheets work @@ -16,13 +16,14 @@ PASS Adding non-constructed stylesheet to AdoptedStyleSheets is not allowed when PASS Adding non-constructed stylesheet to AdoptedStyleSheets is not allowed when the owner document of the stylesheet and the AdoptedStyleSheets are in different document trees PASS CSSStyleSheet.replaceSync replaces stylesheet text synchronously PASS CSSStyleSheet.replaceSync correctly updates the style of its adopters synchronously -FAIL Adopted sheets are ordered after non-adopted sheets in the shadow root assert_equals: expected "rgb(255, 255, 255)" but got "rgb(255, 0, 0)" -FAIL Adopted sheets are ordered after non-adopted sheets in the document assert_equals: expected "rgb(255, 255, 255)" but got "rgb(255, 0, 0)" -PASS CSSStyleSheet.replaceSync throws exception when there is import rule inside +FAIL Adopted sheets are ordered after non-adopted sheets in the shadow root assert_equals: with the adopted sheet disabled, the non-adopted style should take effect expected "rgb(255, 255, 255)" but got "rgb(255, 0, 0)" +FAIL Adopted sheets are ordered after non-adopted sheets in the document assert_equals: with the adopted sheet disabled, the non-adopted style should take effect expected "rgb(255, 255, 255)" but got "rgb(255, 0, 0)" PASS Inserting an @import rule through insertRule on a constructed stylesheet throws an exception PASS CSSStyleSheet.replaceSync should not trigger any loads from @import rules -PASS CSSStyleSheet.replace allows import rule inside -FAIL CSSStyleSheet.replace returns rejected promise on failed imports assert_equals: expected "NetworkError" but got "NotAllowedError" +PASS CSSStyleSheet.replace allows, but ignores, import rule inside +PASS CSSStyleSheet.replace ignores @import rule but still loads other rules +PASS CSSStyleSheet.replaceSync allows, but ignores, import rule inside +PASS CSSStyleSheet.replace does not reject on failed imports PASS Cloning a shadow host will not clone shadow root, and also adoptedStyleSheets PASS Importing a shadow host will not copy shadow root, and also adoptedStyleSheets PASS Adopting a shadow host will empty adoptedStyleSheets if adopting to a different document diff --git a/third_party/blink/web_tests/external/wpt/css/cssom/CSSStyleSheet-constructable.html b/third_party/blink/web_tests/external/wpt/css/cssom/CSSStyleSheet-constructable.html index fbee4298c14119..e6293909c21789 100644 --- a/third_party/blink/web_tests/external/wpt/css/cssom/CSSStyleSheet-constructable.html +++ b/third_party/blink/web_tests/external/wpt/css/cssom/CSSStyleSheet-constructable.html @@ -48,7 +48,7 @@ test(() => { const sheet = new CSSStyleSheet({disabled: true, media: "screen, print"}); - assert_equals(sheet.title, null); + assert_equals(sheet.title, null, "The title attribute must return the title or null if title is the empty string"); assert_equals(sheet.ownerNode, null); assert_equals(sheet.ownerRule, null); assert_equals(sheet.media.length, 2); @@ -66,7 +66,7 @@ assert_equals(sheet.cssRules[0].cssText, redStyleTexts[1]); const sheet2 = new CSSStyleSheet({}); - assert_equals(sheet2.title, null); + assert_equals(sheet2.title, null, "The title attribute must return the title or null if title is the empty string"); assert_equals(sheet2.ownerNode, null); assert_equals(sheet2.ownerRule, null); assert_equals(sheet2.media.length, 0); @@ -81,7 +81,7 @@ assert_equals(sheet2.cssRules.length, 0); const sheet3 = new CSSStyleSheet(); - assert_equals(sheet3.title, null); + assert_equals(sheet3.title, null, "The title attribute must return the title or null if title is the empty string"); assert_equals(sheet3.ownerNode, null); assert_equals(sheet3.ownerRule, null); assert_equals(sheet3.media.length, 0); @@ -98,14 +98,14 @@ test(() => { const sheet = new CSSStyleSheet({title: "something"}); - assert_equals(sheet.title, null); -}, "title cannot be set in the CSSStyleSheet constructor"); + assert_equals(sheet.title, null, "title and alternate are not supported by the constructor. https://github.com/WICG/construct-stylesheets/issues/105"); +}, "title can be set in the CSSStyleSheet constructor"); promise_test(() => { const sheet = new CSSStyleSheet({disabled: true, media: "screen, print"}); const promise_sheet = sheet.replace(redStyleTexts[0]); return promise_sheet.then(function(sheet) { - assert_equals(sheet.title, null); + assert_equals(sheet.title, null, "The title attribute must return the title or null if title is the empty string"); assert_equals(sheet.ownerNode, null); assert_equals(sheet.ownerRule, null); assert_equals(sheet.media.length, 2); @@ -535,24 +535,19 @@ const style = document.createElement("style"); style.textContent = ".target { color: white; }"; span.shadowRoot.appendChild(style) - // non-adopted styles should be ordered before adopted styles - assert_equals(getComputedStyle(shadowDiv).color, "rgb(255, 0, 0)"); + assert_equals(getComputedStyle(shadowDiv).color, "rgb(255, 0, 0)", "non-adopted styles should be ordered before adopted styles"); span.shadowRoot.adoptedStyleSheets = []; - // with no adopted styles in conflict, the non-adopted style should take effect - assert_equals(getComputedStyle(shadowDiv).color, "rgb(255, 255, 255)"); + assert_equals(getComputedStyle(shadowDiv).color, "rgb(255, 255, 255)", "with no adopted styles in conflict, the non-adopted style should take effect"); span.shadowRoot.adoptedStyleSheets = [sheet]; - // the adopted style should be ordered after the non-adopted style - assert_equals(getComputedStyle(shadowDiv).color, "rgb(255, 0, 0)"); + assert_equals(getComputedStyle(shadowDiv).color, "rgb(255, 0, 0)", "the adopted style should be ordered after the non-adopted style"); sheet.disabled = true; - // with the adopted sheet disabled, the non-adopted style should take effect - assert_equals(getComputedStyle(shadowDiv).color, "rgb(255, 255, 255)"); + assert_equals(getComputedStyle(shadowDiv).color, "rgb(255, 255, 255)", "with the adopted sheet disabled, the non-adopted style should take effect"); sheet.disabled = false; - // the adopted sheet re-enabled, it should take effect again. - assert_equals(getComputedStyle(shadowDiv).color, "rgb(255, 0, 0)"); + assert_equals(getComputedStyle(shadowDiv).color, "rgb(255, 0, 0)", "the adopted sheet re-enabled, it should take effect again"); }, 'Adopted sheets are ordered after non-adopted sheets in the shadow root') test(() => { @@ -574,41 +569,32 @@ const style = document.createElement("style"); style.textContent = ".target { color: white; }"; span.appendChild(style) - // non-adopted styles should be ordered before adopted styles - assert_equals(getComputedStyle(span).color, "rgb(255, 0, 0)"); + assert_equals(getComputedStyle(span).color, "rgb(255, 0, 0)", "non-adopted styles should be ordered before adopted styles"); document.adoptedStyleSheets = []; - // with no adopted styles in conflict, the non-adopted style should take effect - assert_equals(getComputedStyle(span).color, "rgb(255, 255, 255)"); + assert_equals(getComputedStyle(span).color, "rgb(255, 255, 255)", "with no adopted styles in conflict, the non-adopted style should take effect"); document.adoptedStyleSheets = [sheet]; - // the adopted style should be ordered after the non-adopted style - assert_equals(getComputedStyle(span).color, "rgb(255, 0, 0)"); + assert_equals(getComputedStyle(span).color, "rgb(255, 0, 0)", "the adopted style should be ordered after the non-adopted style"); sheet.disabled = true; - // with the adopted sheet disabled, the non-adopted style should take effect - assert_equals(getComputedStyle(span).color, "rgb(255, 255, 255)"); + assert_equals(getComputedStyle(span).color, "rgb(255, 255, 255)", "with the adopted sheet disabled, the non-adopted style should take effect"); sheet.disabled = false; - // the adopted sheet re-enabled, it should take effect again. - assert_equals(getComputedStyle(span).color, "rgb(255, 0, 0)") + assert_equals(getComputedStyle(span).color, "rgb(255, 0, 0)", "the adopted sheet re-enabled, it should take effect again") }, 'Adopted sheets are ordered after non-adopted sheets in the document') const import_text = '@import url("support/constructable-import.css");'; test(() => { - assert_throws_dom("NotAllowedError", () => { (new CSSStyleSheet).replaceSync(import_text) }); -}, 'CSSStyleSheet.replaceSync throws exception when there is import rule inside'); - -test(() => { - assert_throws_dom("NotAllowedError", () => { (new CSSStyleSheet).insertRule(import_text) }); + assert_throws_dom("SyntaxError", () => { (new CSSStyleSheet).insertRule(import_text) }); }, 'Inserting an @import rule through insertRule on a constructed stylesheet throws an exception'); async_test(t => { const importUrl = "support/constructable-import.css"; const sheet = new CSSStyleSheet(); - assert_throws_dom("NotAllowedError", () => { sheet.replaceSync(`@import url("${importUrl}");`) }); + sheet.replaceSync(`@import url("${importUrl}");`); const timeAfterReplaceSync = performance.now(); let link = document.createElement("link"); @@ -632,27 +618,68 @@ const sheet = new CSSStyleSheet(); span.shadowRoot.adoptedStyleSheets = [sheet]; assert_equals(getComputedStyle(shadowDiv).color, "rgb(0, 0, 0)"); - // Replace and assert that the imported rule is applied. + // Replace and assert that the imported rule is NOT applied. const sheet_promise = sheet.replace(import_text); + return sheet_promise.then((sheet) => { + // replace() ignores @import rules: + assert_equals(sheet.cssRules.length, 0); + assert_equals(getComputedStyle(shadowDiv).color, "rgb(0, 0, 0)"); + }).catch((reason) => { + assert_unreached(`Promise was rejected (${reason}) when it should have been resolved`); + }); +}, 'CSSStyleSheet.replace allows, but ignores, import rule inside'); + +promise_test(() => { + const span = document.createElement("span"); + thirdSection.appendChild(span); + const shadowDiv = attachShadowDiv(span); + const targetSpan = document.createElement("span"); + targetSpan.classList.add("target"); + shadowDiv.appendChild(targetSpan); + const sheet = new CSSStyleSheet(); + span.shadowRoot.adoptedStyleSheets = [sheet]; + assert_equals(getComputedStyle(shadowDiv).color, "rgb(0, 0, 0)"); + // Replace and assert that the imported rule is NOT applied, but regular rule does apply. + const sheet_promise = sheet.replace(import_text + ".target { color: blue; }"); return sheet_promise.then((sheet) => { assert_equals(sheet.cssRules.length, 1); - assert_equals(sheet.cssRules[0].cssText, import_text); - assert_equals(getComputedStyle(shadowDiv).color, "rgb(255, 0, 0)"); + // @import not applied: + assert_equals(getComputedStyle(shadowDiv).color, "rgb(0, 0, 0)"); + // regular rule applied: + assert_equals(getComputedStyle(targetSpan).color, "rgb(0, 0, 255)"); }).catch((reason) => { assert_unreached(`Promise was rejected (${reason}) when it should have been resolved`); }); -}, 'CSSStyleSheet.replace allows import rule inside'); +}, 'CSSStyleSheet.replace ignores @import rule but still loads other rules'); + +test(() => { + const span = document.createElement("span"); + thirdSection.appendChild(span); + const shadowDiv = attachShadowDiv(span); + const sheet = new CSSStyleSheet(); + span.shadowRoot.adoptedStyleSheets = [sheet]; + assert_equals(getComputedStyle(shadowDiv).color, "rgb(0, 0, 0)"); + // Replace and assert that the imported rule is NOT applied. + try { + sheet.replaceSync(import_text); + // replaceSync() ignores @import rules: + assert_equals(sheet.cssRules.length, 0); + assert_equals(getComputedStyle(shadowDiv).color, "rgb(0, 0, 0)"); + } catch(reason) { + assert_unreached(`replaceSync threw an exception (${reason}) when it shouldn't have`); + } +}, 'CSSStyleSheet.replaceSync allows, but ignores, import rule inside'); promise_test(() => { const sheet = new CSSStyleSheet(); const sheet_promise = sheet.replace("@import url('not-there.css');"); return sheet_promise.then((sheet) => { - assert_unreached("Promise was resolved when it should have been rejected"); + // No exception here }).catch((reason) => { - assert_equals(reason.name, "NetworkError"); + assert_unreached("Promise was rejected"); }); -}, 'CSSStyleSheet.replace returns rejected promise on failed imports'); +}, 'CSSStyleSheet.replace does not reject on failed imports'); test(() => { const span = document.createElement("span"); diff --git a/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/css-module/import-css-module-basic.html b/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/css-module/import-css-module-basic.html index 902430d0779e3b..1cb290de3feb58 100644 --- a/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/css-module/import-css-module-basic.html +++ b/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/css-module/import-css-module-basic.html @@ -22,13 +22,13 @@ const iframe = document.createElement("iframe"); iframe.src = "resources/css-module-at-import-iframe.html"; iframe.onload = test.step_func_done(function () { - assert_equals(iframe.contentDocument.load_error, "NotAllowedError"); + assert_equals(iframe.contentDocument.load_error, undefined); assert_not_equals(getComputedStyle(iframe.contentDocument.querySelector('#test')) .backgroundColor, "rgb(255, 0, 0)", "CSS module @import should not succeed"); }); document.body.appendChild(iframe); - }, "An @import CSS Module should not load"); + }, "An @import CSS Module should not load, but should not throw an exception"); async_test(function (test) { const iframe = document.createElement("iframe");