From 6835425e2095bec5321b60106985e0300c482d61 Mon Sep 17 00:00:00 2001 From: athenian200 Date: Tue, 11 Aug 2020 05:27:40 -0500 Subject: [PATCH] Issue #1629 - Part 2: Implement the Explicitly Enabled flag. This part of the bug was significantly complicated by the following major refactors: https://bugzilla.mozilla.org/show_bug.cgi?id=1456435 https://bugzilla.mozilla.org/show_bug.cgi?id=1459498 As best as I can tell, we just need to implement the explicitly enabled flag on every instance of GetStyleSheetInfo, make sure aIsExplicitlyEnabled is false in every situation except the one where the disabled content attribute is removed from a link element, and enable alternate stylesheets if this flag is set on them. So we take the explicitly enabled flag as an input to PrepareSheet, and also add it to LoadStyleLink and LoadInlineStyle. I also decided not to defer loading of alternate stylesheets that have been explicitly enabled. --- dom/base/nsContentSink.cpp | 5 ++-- dom/base/nsStyleLinkElement.cpp | 8 ++++-- dom/base/nsStyleLinkElement.h | 3 +- dom/html/HTMLLinkElement.cpp | 20 ++++++++++--- dom/html/HTMLLinkElement.h | 12 ++++++-- dom/html/HTMLStyleElement.cpp | 4 ++- dom/html/HTMLStyleElement.h | 3 +- dom/svg/SVGStyleElement.cpp | 4 ++- dom/svg/SVGStyleElement.h | 3 +- .../XMLStylesheetProcessingInstruction.cpp | 4 ++- dom/xml/XMLStylesheetProcessingInstruction.h | 3 +- layout/style/Loader.cpp | 28 ++++++++++++------- layout/style/Loader.h | 13 +++++++-- 13 files changed, 79 insertions(+), 31 deletions(-) diff --git a/dom/base/nsContentSink.cpp b/dom/base/nsContentSink.cpp index 1e6465a1bf..59f4a9f9a8 100644 --- a/dom/base/nsContentSink.cpp +++ b/dom/base/nsContentSink.cpp @@ -789,13 +789,14 @@ nsContentSink::ProcessStyleLink(nsIContent* aElement, // If this is a fragment parser, we don't want to observe. // We don't support CORS for processing instructions bool isAlternate; + bool isExplicitlyEnabled; rv = mCSSLoader->LoadStyleLink(aElement, url, aTitle, aMedia, aAlternate, CORS_NONE, mDocument->GetReferrerPolicy(), integrity, mRunsToCompletion ? nullptr : this, - &isAlternate); + &isAlternate, &isExplicitlyEnabled); NS_ENSURE_SUCCESS(rv, rv); - if (!isAlternate && !mRunsToCompletion) { + if ((!isAlternate || isExplicitlyEnabled) && !mRunsToCompletion) { ++mPendingSheetCount; mScriptLoader->AddParserBlockingScriptExecutionBlocker(); } diff --git a/dom/base/nsStyleLinkElement.cpp b/dom/base/nsStyleLinkElement.cpp index 8ab2dab0bb..2e5cdac6f4 100644 --- a/dom/base/nsStyleLinkElement.cpp +++ b/dom/base/nsStyleLinkElement.cpp @@ -393,8 +393,9 @@ nsStyleLinkElement::DoUpdateStyleSheet(nsIDocument* aOldDocument, nsAutoString title, type, media; bool isScoped; bool isAlternate; + bool isExplicitlyEnabled; - GetStyleSheetInfo(title, type, media, &isScoped, &isAlternate); + GetStyleSheetInfo(title, type, media, &isScoped, &isAlternate, &isExplicitlyEnabled); if (!type.LowerCaseEqualsLiteral("text/css")) { return NS_OK; @@ -425,7 +426,7 @@ nsStyleLinkElement::DoUpdateStyleSheet(nsIDocument* aOldDocument, // Parse the style sheet. rv = doc->CSSLoader()-> LoadInlineStyle(thisContent, text, mLineNumber, title, media, - scopeElement, aObserver, &doneLoading, &isAlternate); + scopeElement, aObserver, &doneLoading, &isAlternate, &isExplicitlyEnabled); } else { nsAutoString integrity; @@ -452,13 +453,14 @@ nsStyleLinkElement::DoUpdateStyleSheet(nsIDocument* aOldDocument, rv = doc->CSSLoader()-> LoadStyleLink(thisContent, clonedURI, title, media, isAlternate, GetCORSMode(), referrerPolicy, integrity, - aObserver, &isAlternate); + aObserver, &isAlternate, &isExplicitlyEnabled); if (NS_FAILED(rv)) { // Don't propagate LoadStyleLink() errors further than this, since some // consumers (e.g. nsXMLContentSink) will completely abort on innocuous // things like a stylesheet load being blocked by the security system. doneLoading = true; isAlternate = false; + isExplicitlyEnabled = false; rv = NS_OK; } } diff --git a/dom/base/nsStyleLinkElement.h b/dom/base/nsStyleLinkElement.h index a41ae5e1d7..d9042c756d 100644 --- a/dom/base/nsStyleLinkElement.h +++ b/dom/base/nsStyleLinkElement.h @@ -98,7 +98,8 @@ protected: nsAString& aType, nsAString& aMedia, bool* aIsScoped, - bool* aIsAlternate) = 0; + bool* aIsAlternate, + bool* aIsExplicitlyEnabled) = 0; virtual mozilla::CORSMode GetCORSMode() const { diff --git a/dom/html/HTMLLinkElement.cpp b/dom/html/HTMLLinkElement.cpp index ed83836aac..352cbc7559 100644 --- a/dom/html/HTMLLinkElement.cpp +++ b/dom/html/HTMLLinkElement.cpp @@ -126,7 +126,6 @@ HTMLLinkElement::SetMozDisabled(bool aDisabled) return rv.StealNSResult(); } - NS_IMPL_STRING_ATTR(HTMLLinkElement, Charset, charset) NS_IMPL_URI_ATTR(HTMLLinkElement, Href, href) NS_IMPL_STRING_ATTR(HTMLLinkElement, Hreflang, hreflang) @@ -407,9 +406,14 @@ HTMLLinkElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, aName == nsGkAtoms::disabled)); } } else { - // Since removing href or rel makes us no longer link to a - // stylesheet, force updates for those too. + // If the disabled attribute is removed from a link element, the + // stylesheet may be explicitly enabled. if (aNameSpaceID == kNameSpaceID_None) { + if (aName == nsGkAtoms::disabled) { + mExplicitlyEnabled = true; + } + // Since removing href or rel makes us no longer link to a + // stylesheet, force updates for those too. if (aName == nsGkAtoms::href || aName == nsGkAtoms::rel || aName == nsGkAtoms::title || @@ -508,13 +512,15 @@ HTMLLinkElement::GetStyleSheetInfo(nsAString& aTitle, nsAString& aType, nsAString& aMedia, bool* aIsScoped, - bool* aIsAlternate) + bool* aIsAlternate, + bool* aIsExplicitlyEnabled) { aTitle.Truncate(); aType.Truncate(); aMedia.Truncate(); *aIsScoped = false; *aIsAlternate = false; + *aIsExplicitlyEnabled = false; nsAutoString rel; GetAttr(kNameSpaceID_None, nsGkAtoms::rel, rel); @@ -524,10 +530,16 @@ HTMLLinkElement::GetStyleSheetInfo(nsAString& aTitle, return; } + // Is the link disabled? if (Disabled()) { return; } + // Is it explicitly enabled? + if (mExplicitlyEnabled) { + *aIsExplicitlyEnabled = true; + } + nsAutoString title; GetAttr(kNameSpaceID_None, nsGkAtoms::title, title); title.CompressWhitespace(); diff --git a/dom/html/HTMLLinkElement.h b/dom/html/HTMLLinkElement.h index e024d42bbc..190213028e 100644 --- a/dom/html/HTMLLinkElement.h +++ b/dom/html/HTMLLinkElement.h @@ -181,10 +181,18 @@ protected: nsAString& aType, nsAString& aMedia, bool* aIsScoped, - bool* aIsAlternate) override; -protected: + bool* aIsAlternate, + bool* aIsExplicitlyEnabled) override; + RefPtr mRelList; + // The "explicitly enabled" flag. This flag is set whenever the 'disabled' + // attribute is explicitly unset, and makes alternate stylesheets not be + // disabled by default anymore. + // + // See https://github.com/whatwg/html/issues/3840#issuecomment-481034206. + bool mExplicitlyEnabled = false; + private: RefPtr mImportLoader; }; diff --git a/dom/html/HTMLStyleElement.cpp b/dom/html/HTMLStyleElement.cpp index 95cf025d10..c37e1a32d7 100644 --- a/dom/html/HTMLStyleElement.cpp +++ b/dom/html/HTMLStyleElement.cpp @@ -223,12 +223,14 @@ HTMLStyleElement::GetStyleSheetInfo(nsAString& aTitle, nsAString& aType, nsAString& aMedia, bool* aIsScoped, - bool* aIsAlternate) + bool* aIsAlternate, + bool* aIsExplicitlyEnabled) { aTitle.Truncate(); aType.Truncate(); aMedia.Truncate(); *aIsAlternate = false; + *aIsExplicitlyEnabled = false; nsAutoString title; GetAttr(kNameSpaceID_None, nsGkAtoms::title, title); diff --git a/dom/html/HTMLStyleElement.h b/dom/html/HTMLStyleElement.h index 8b69c3f625..7d98a494a8 100644 --- a/dom/html/HTMLStyleElement.h +++ b/dom/html/HTMLStyleElement.h @@ -88,7 +88,8 @@ protected: nsAString& aType, nsAString& aMedia, bool* aIsScoped, - bool* aIsAlternate) override; + bool* aIsAlternate, + bool* aIsExplicitlyEnabled) override; /** * Common method to call from the various mutation observer methods. * aContent is a content node that's either the one that changed or its diff --git a/dom/svg/SVGStyleElement.cpp b/dom/svg/SVGStyleElement.cpp index 22fb204dff..7655c1198c 100644 --- a/dom/svg/SVGStyleElement.cpp +++ b/dom/svg/SVGStyleElement.cpp @@ -271,9 +271,11 @@ SVGStyleElement::GetStyleSheetInfo(nsAString& aTitle, nsAString& aType, nsAString& aMedia, bool* aIsScoped, - bool* aIsAlternate) + bool* aIsAlternate, + bool* aIsExplicitlyEnabled) { *aIsAlternate = false; + *aIsExplicitlyEnabled = false; nsAutoString title; GetAttr(kNameSpaceID_None, nsGkAtoms::title, title); diff --git a/dom/svg/SVGStyleElement.h b/dom/svg/SVGStyleElement.h index 9b126e7e8f..e637dfb187 100644 --- a/dom/svg/SVGStyleElement.h +++ b/dom/svg/SVGStyleElement.h @@ -95,7 +95,8 @@ protected: nsAString& aType, nsAString& aMedia, bool* aIsScoped, - bool* aIsAlternate) override; + bool* aIsAlternate, + bool* aIsExplicitlyEnabled) override; virtual CORSMode GetCORSMode() const override; /** diff --git a/dom/xml/XMLStylesheetProcessingInstruction.cpp b/dom/xml/XMLStylesheetProcessingInstruction.cpp index 3d94e179b4..43e45131a6 100644 --- a/dom/xml/XMLStylesheetProcessingInstruction.cpp +++ b/dom/xml/XMLStylesheetProcessingInstruction.cpp @@ -131,13 +131,15 @@ XMLStylesheetProcessingInstruction::GetStyleSheetInfo(nsAString& aTitle, nsAString& aType, nsAString& aMedia, bool* aIsScoped, - bool* aIsAlternate) + bool* aIsAlternate, + bool* aIsExplicitlyEnabled) { aTitle.Truncate(); aType.Truncate(); aMedia.Truncate(); *aIsScoped = false; *aIsAlternate = false; + *aIsExplicitlyEnabled = false; // xml-stylesheet PI is special only in prolog if (!nsContentUtils::InProlog(this)) { diff --git a/dom/xml/XMLStylesheetProcessingInstruction.h b/dom/xml/XMLStylesheetProcessingInstruction.h index 818b3392f5..28061834aa 100644 --- a/dom/xml/XMLStylesheetProcessingInstruction.h +++ b/dom/xml/XMLStylesheetProcessingInstruction.h @@ -82,7 +82,8 @@ protected: nsAString& aType, nsAString& aMedia, bool* aIsScoped, - bool* aIsAlternate) override; + bool* aIsAlternate, + bool* aIsExplicitlyEnabled) override; virtual nsGenericDOMDataNode* CloneDataNode(mozilla::dom::NodeInfo *aNodeInfo, bool aCloneText) const override; }; diff --git a/layout/style/Loader.cpp b/layout/style/Loader.cpp index 68a7be21ed..c211992381 100644 --- a/layout/style/Loader.cpp +++ b/layout/style/Loader.cpp @@ -1278,7 +1278,8 @@ Loader::PrepareSheet(StyleSheet* aSheet, const nsSubstring& aMediaString, nsMediaList* aMediaList, Element* aScopeElement, - bool isAlternate) + bool isAlternate, + bool isExplicitlyEnabled) { NS_PRECONDITION(aSheet, "Must have a sheet!"); @@ -1307,7 +1308,7 @@ Loader::PrepareSheet(StyleSheet* aSheet, sheet->SetMedia(mediaList); sheet->SetTitle(aTitle); - sheet->SetEnabled(!isAlternate); + sheet->SetEnabled(!isAlternate || isExplicitlyEnabled); sheet->SetScopeElement(aScopeElement); } @@ -1985,7 +1986,8 @@ Loader::LoadInlineStyle(nsIContent* aElement, Element* aScopeElement, nsICSSLoaderObserver* aObserver, bool* aCompleted, - bool* aIsAlternate) + bool* aIsAlternate, + bool* aIsExplicitlyEnabled) { LOG(("css::Loader::LoadInlineStyle")); NS_ASSERTION(mParsingDatas.Length() == 0, "We're in the middle of a parse?"); @@ -2017,8 +2019,9 @@ Loader::LoadInlineStyle(nsIContent* aElement, "Inline sheets should not be cached"); LOG((" Sheet is alternate: %d", *aIsAlternate)); + LOG((" Sheet is explicitly enabled: %d", *aIsExplicitlyEnabled)); - PrepareSheet(sheet, aTitle, aMedia, nullptr, aScopeElement, *aIsAlternate); + PrepareSheet(sheet, aTitle, aMedia, nullptr, aScopeElement, *aIsAlternate, *aIsExplicitlyEnabled); if (aElement->HasFlag(NODE_IS_IN_SHADOW_TREE)) { ShadowRoot* containingShadow = aElement->GetContainingShadow(); @@ -2059,7 +2062,8 @@ Loader::LoadStyleLink(nsIContent* aElement, ReferrerPolicy aReferrerPolicy, const nsAString& aIntegrity, nsICSSLoaderObserver* aObserver, - bool* aIsAlternate) + bool* aIsAlternate, + bool* aIsExplicitlyEnabled) { LOG(("css::Loader::LoadStyleLink")); NS_PRECONDITION(aURL, "Must have URL to load"); @@ -2112,8 +2116,9 @@ Loader::LoadStyleLink(nsIContent* aElement, NS_ENSURE_SUCCESS(rv, rv); LOG((" Sheet is alternate: %d", *aIsAlternate)); + LOG((" Sheet is explicitly enabled: %d", *aIsExplicitlyEnabled)); - PrepareSheet(sheet, aTitle, aMedia, nullptr, nullptr, *aIsAlternate); + PrepareSheet(sheet, aTitle, aMedia, nullptr, nullptr, *aIsAlternate, *aIsExplicitlyEnabled); rv = InsertSheetInDoc(sheet, aElement, mDocument); NS_ENSURE_SUCCESS(rv, rv); @@ -2138,9 +2143,10 @@ Loader::LoadStyleLink(nsIContent* aElement, aObserver, principal, requestingNode); NS_ADDREF(data); - // If we have to parse and it's an alternate non-inline, defer it + // If we have to parse and it's an alternate non-inline, defer it unless + // it's explicitly enabled. if (aURL && state == eSheetNeedsParser && mSheets->mLoadingDatas.Count() != 0 && - *aIsAlternate) { + *aIsAlternate && !*aIsExplicitlyEnabled) { LOG((" Deferring alternate sheet load")); URIPrincipalReferrerPolicyAndCORSModeHashKey key(data->mURI, data->mLoaderPrincipal, @@ -2267,6 +2273,7 @@ Loader::LoadChildSheet(StyleSheet* aParentSheet, state = eSheetComplete; } else { bool isAlternate; + bool isExplicitlyEnabled; const nsSubstring& empty = EmptyString(); // For now, use CORS_NONE for child sheets rv = CreateSheet(aURL, nullptr, principal, @@ -2277,7 +2284,7 @@ Loader::LoadChildSheet(StyleSheet* aParentSheet, false, empty, state, &isAlternate, &sheet); NS_ENSURE_SUCCESS(rv, rv); - PrepareSheet(sheet, empty, empty, aMedia, nullptr, isAlternate); + PrepareSheet(sheet, empty, empty, aMedia, nullptr, isAlternate, isExplicitlyEnabled); } rv = InsertChildSheet(sheet, aParentSheet, aParentRule); @@ -2390,6 +2397,7 @@ Loader::InternalLoadNonDocumentSheet(nsIURI* aURL, StyleSheetState state; bool isAlternate; + bool isExplicitlyEnabled; RefPtr sheet; bool syncLoad = (aObserver == nullptr); const nsSubstring& empty = EmptyString(); @@ -2399,7 +2407,7 @@ Loader::InternalLoadNonDocumentSheet(nsIURI* aURL, false, empty, state, &isAlternate, &sheet); NS_ENSURE_SUCCESS(rv, rv); - PrepareSheet(sheet, empty, empty, nullptr, nullptr, isAlternate); + PrepareSheet(sheet, empty, empty, nullptr, nullptr, isAlternate, isExplicitlyEnabled); if (state == eSheetComplete) { LOG((" Sheet already complete")); diff --git a/layout/style/Loader.h b/layout/style/Loader.h index 209783a804..4a3088b6b4 100644 --- a/layout/style/Loader.h +++ b/layout/style/Loader.h @@ -230,6 +230,8 @@ public: * @param [out] aCompleted whether parsing of the sheet completed. * @param [out] aIsAlternate whether the stylesheet ended up being an * alternate sheet. + * @param [out] aIsExplicitlyEnabled whether the stylesheet was explicitly + * enabled by having the disabled attribute removed. */ nsresult LoadInlineStyle(nsIContent* aElement, const nsAString& aBuffer, @@ -239,7 +241,8 @@ public: mozilla::dom::Element* aScopeElement, nsICSSLoaderObserver* aObserver, bool* aCompleted, - bool* aIsAlternate); + bool* aIsAlternate, + bool* aIsExplicitlyEnabled); /** * Load a linked (document) stylesheet. If a successful result is returned, @@ -260,6 +263,8 @@ public: * @param [out] aIsAlternate whether the stylesheet actually ended up beinga * an alternate sheet. Note that this need not match * aHasAlternateRel. + * @param [out] aIsExplicitlyEnabled whether the stylesheet was explicitly + * enabled by having the disabled attribute removed. */ nsresult LoadStyleLink(nsIContent* aElement, nsIURI* aURL, @@ -270,7 +275,8 @@ public: ReferrerPolicy aReferrerPolicy, const nsAString& aIntegrity, nsICSSLoaderObserver* aObserver, - bool* aIsAlternate); + bool* aIsAlternate, + bool* aIsExplicitlyEnabled); /** * Load a child (@import-ed) style sheet. In addition to loading the sheet, @@ -476,7 +482,8 @@ private: const nsAString& aMediaString, nsMediaList* aMediaList, dom::Element* aScopeElement, - bool isAlternate); + bool isAlternate, + bool isExplicitlyEnabled); nsresult InsertSheetInDoc(StyleSheet* aSheet, nsIContent* aLinkingContent,