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.
This commit is contained in:
athenian200 2020-08-11 05:27:40 -05:00 • committed by Roy Tam
commit 6835425e20
13 changed files with 79 additions and 31 deletions

View file

@ -789,13 +789,14 @@ nsContentSink::ProcessStyleLink(nsIContent* aElement,
// If this is a fragment parser, we don't want to observe. // If this is a fragment parser, we don't want to observe.
// We don't support CORS for processing instructions // We don't support CORS for processing instructions
bool isAlternate; bool isAlternate;
bool isExplicitlyEnabled;
rv = mCSSLoader->LoadStyleLink(aElement, url, aTitle, aMedia, aAlternate, rv = mCSSLoader->LoadStyleLink(aElement, url, aTitle, aMedia, aAlternate,
CORS_NONE, mDocument->GetReferrerPolicy(), CORS_NONE, mDocument->GetReferrerPolicy(),
integrity, mRunsToCompletion ? nullptr : this, integrity, mRunsToCompletion ? nullptr : this,
&isAlternate); &isAlternate, &isExplicitlyEnabled);
NS_ENSURE_SUCCESS(rv, rv); NS_ENSURE_SUCCESS(rv, rv);
if (!isAlternate && !mRunsToCompletion) { if ((!isAlternate || isExplicitlyEnabled) && !mRunsToCompletion) {
++mPendingSheetCount; ++mPendingSheetCount;
mScriptLoader->AddParserBlockingScriptExecutionBlocker(); mScriptLoader->AddParserBlockingScriptExecutionBlocker();
} }

View file

@ -393,8 +393,9 @@ nsStyleLinkElement::DoUpdateStyleSheet(nsIDocument* aOldDocument,
nsAutoString title, type, media; nsAutoString title, type, media;
bool isScoped; bool isScoped;
bool isAlternate; bool isAlternate;
bool isExplicitlyEnabled;
GetStyleSheetInfo(title, type, media, &isScoped, &isAlternate); GetStyleSheetInfo(title, type, media, &isScoped, &isAlternate, &isExplicitlyEnabled);
if (!type.LowerCaseEqualsLiteral("text/css")) { if (!type.LowerCaseEqualsLiteral("text/css")) {
return NS_OK; return NS_OK;
@ -425,7 +426,7 @@ nsStyleLinkElement::DoUpdateStyleSheet(nsIDocument* aOldDocument,
// Parse the style sheet. // Parse the style sheet.
rv = doc->CSSLoader()-> rv = doc->CSSLoader()->
LoadInlineStyle(thisContent, text, mLineNumber, title, media, LoadInlineStyle(thisContent, text, mLineNumber, title, media,
scopeElement, aObserver, &doneLoading, &isAlternate); scopeElement, aObserver, &doneLoading, &isAlternate, &isExplicitlyEnabled);
} }
else { else {
nsAutoString integrity; nsAutoString integrity;
@ -452,13 +453,14 @@ nsStyleLinkElement::DoUpdateStyleSheet(nsIDocument* aOldDocument,
rv = doc->CSSLoader()-> rv = doc->CSSLoader()->
LoadStyleLink(thisContent, clonedURI, title, media, isAlternate, LoadStyleLink(thisContent, clonedURI, title, media, isAlternate,
GetCORSMode(), referrerPolicy, integrity, GetCORSMode(), referrerPolicy, integrity,
aObserver, &isAlternate); aObserver, &isAlternate, &isExplicitlyEnabled);
if (NS_FAILED(rv)) { if (NS_FAILED(rv)) {
// Don't propagate LoadStyleLink() errors further than this, since some // Don't propagate LoadStyleLink() errors further than this, since some
// consumers (e.g. nsXMLContentSink) will completely abort on innocuous // consumers (e.g. nsXMLContentSink) will completely abort on innocuous
// things like a stylesheet load being blocked by the security system. // things like a stylesheet load being blocked by the security system.
doneLoading = true; doneLoading = true;
isAlternate = false; isAlternate = false;
isExplicitlyEnabled = false;
rv = NS_OK; rv = NS_OK;
} }
} }

View file

@ -98,7 +98,8 @@ protected:
nsAString& aType, nsAString& aType,
nsAString& aMedia, nsAString& aMedia,
bool* aIsScoped, bool* aIsScoped,
bool* aIsAlternate) = 0; bool* aIsAlternate,
bool* aIsExplicitlyEnabled) = 0;
virtual mozilla::CORSMode GetCORSMode() const virtual mozilla::CORSMode GetCORSMode() const
{ {

View file

@ -126,7 +126,6 @@ HTMLLinkElement::SetMozDisabled(bool aDisabled)
return rv.StealNSResult(); return rv.StealNSResult();
} }
NS_IMPL_STRING_ATTR(HTMLLinkElement, Charset, charset) NS_IMPL_STRING_ATTR(HTMLLinkElement, Charset, charset)
NS_IMPL_URI_ATTR(HTMLLinkElement, Href, href) NS_IMPL_URI_ATTR(HTMLLinkElement, Href, href)
NS_IMPL_STRING_ATTR(HTMLLinkElement, Hreflang, hreflang) NS_IMPL_STRING_ATTR(HTMLLinkElement, Hreflang, hreflang)
@ -407,9 +406,14 @@ HTMLLinkElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
aName == nsGkAtoms::disabled)); aName == nsGkAtoms::disabled));
} }
} else { } else {
// Since removing href or rel makes us no longer link to a // If the disabled attribute is removed from a link element, the
// stylesheet, force updates for those too. // stylesheet may be explicitly enabled.
if (aNameSpaceID == kNameSpaceID_None) { 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 || if (aName == nsGkAtoms::href ||
aName == nsGkAtoms::rel || aName == nsGkAtoms::rel ||
aName == nsGkAtoms::title || aName == nsGkAtoms::title ||
@ -508,13 +512,15 @@ HTMLLinkElement::GetStyleSheetInfo(nsAString& aTitle,
nsAString& aType, nsAString& aType,
nsAString& aMedia, nsAString& aMedia,
bool* aIsScoped, bool* aIsScoped,
bool* aIsAlternate) bool* aIsAlternate,
bool* aIsExplicitlyEnabled)
{ {
aTitle.Truncate(); aTitle.Truncate();
aType.Truncate(); aType.Truncate();
aMedia.Truncate(); aMedia.Truncate();
*aIsScoped = false; *aIsScoped = false;
*aIsAlternate = false; *aIsAlternate = false;
*aIsExplicitlyEnabled = false;
nsAutoString rel; nsAutoString rel;
GetAttr(kNameSpaceID_None, nsGkAtoms::rel, rel); GetAttr(kNameSpaceID_None, nsGkAtoms::rel, rel);
@ -524,10 +530,16 @@ HTMLLinkElement::GetStyleSheetInfo(nsAString& aTitle,
return; return;
} }
// Is the link disabled?
if (Disabled()) { if (Disabled()) {
return; return;
} }
// Is it explicitly enabled?
if (mExplicitlyEnabled) {
*aIsExplicitlyEnabled = true;
}
nsAutoString title; nsAutoString title;
GetAttr(kNameSpaceID_None, nsGkAtoms::title, title); GetAttr(kNameSpaceID_None, nsGkAtoms::title, title);
title.CompressWhitespace(); title.CompressWhitespace();

View file

@ -181,10 +181,18 @@ protected:
nsAString& aType, nsAString& aType,
nsAString& aMedia, nsAString& aMedia,
bool* aIsScoped, bool* aIsScoped,
bool* aIsAlternate) override; bool* aIsAlternate,
protected: bool* aIsExplicitlyEnabled) override;
RefPtr<nsDOMTokenList> mRelList; RefPtr<nsDOMTokenList> 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: private:
RefPtr<ImportLoader> mImportLoader; RefPtr<ImportLoader> mImportLoader;
}; };

View file

@ -223,12 +223,14 @@ HTMLStyleElement::GetStyleSheetInfo(nsAString& aTitle,
nsAString& aType, nsAString& aType,
nsAString& aMedia, nsAString& aMedia,
bool* aIsScoped, bool* aIsScoped,
bool* aIsAlternate) bool* aIsAlternate,
bool* aIsExplicitlyEnabled)
{ {
aTitle.Truncate(); aTitle.Truncate();
aType.Truncate(); aType.Truncate();
aMedia.Truncate(); aMedia.Truncate();
*aIsAlternate = false; *aIsAlternate = false;
*aIsExplicitlyEnabled = false;
nsAutoString title; nsAutoString title;
GetAttr(kNameSpaceID_None, nsGkAtoms::title, title); GetAttr(kNameSpaceID_None, nsGkAtoms::title, title);

View file

@ -88,7 +88,8 @@ protected:
nsAString& aType, nsAString& aType,
nsAString& aMedia, nsAString& aMedia,
bool* aIsScoped, bool* aIsScoped,
bool* aIsAlternate) override; bool* aIsAlternate,
bool* aIsExplicitlyEnabled) override;
/** /**
* Common method to call from the various mutation observer methods. * Common method to call from the various mutation observer methods.
* aContent is a content node that's either the one that changed or its * aContent is a content node that's either the one that changed or its

View file

@ -271,9 +271,11 @@ SVGStyleElement::GetStyleSheetInfo(nsAString& aTitle,
nsAString& aType, nsAString& aType,
nsAString& aMedia, nsAString& aMedia,
bool* aIsScoped, bool* aIsScoped,
bool* aIsAlternate) bool* aIsAlternate,
bool* aIsExplicitlyEnabled)
{ {
*aIsAlternate = false; *aIsAlternate = false;
*aIsExplicitlyEnabled = false;
nsAutoString title; nsAutoString title;
GetAttr(kNameSpaceID_None, nsGkAtoms::title, title); GetAttr(kNameSpaceID_None, nsGkAtoms::title, title);

View file

@ -95,7 +95,8 @@ protected:
nsAString& aType, nsAString& aType,
nsAString& aMedia, nsAString& aMedia,
bool* aIsScoped, bool* aIsScoped,
bool* aIsAlternate) override; bool* aIsAlternate,
bool* aIsExplicitlyEnabled) override;
virtual CORSMode GetCORSMode() const override; virtual CORSMode GetCORSMode() const override;
/** /**

View file

@ -131,13 +131,15 @@ XMLStylesheetProcessingInstruction::GetStyleSheetInfo(nsAString& aTitle,
nsAString& aType, nsAString& aType,
nsAString& aMedia, nsAString& aMedia,
bool* aIsScoped, bool* aIsScoped,
bool* aIsAlternate) bool* aIsAlternate,
bool* aIsExplicitlyEnabled)
{ {
aTitle.Truncate(); aTitle.Truncate();
aType.Truncate(); aType.Truncate();
aMedia.Truncate(); aMedia.Truncate();
*aIsScoped = false; *aIsScoped = false;
*aIsAlternate = false; *aIsAlternate = false;
*aIsExplicitlyEnabled = false;
// xml-stylesheet PI is special only in prolog // xml-stylesheet PI is special only in prolog
if (!nsContentUtils::InProlog(this)) { if (!nsContentUtils::InProlog(this)) {

View file

@ -82,7 +82,8 @@ protected:
nsAString& aType, nsAString& aType,
nsAString& aMedia, nsAString& aMedia,
bool* aIsScoped, bool* aIsScoped,
bool* aIsAlternate) override; bool* aIsAlternate,
bool* aIsExplicitlyEnabled) override;
virtual nsGenericDOMDataNode* CloneDataNode(mozilla::dom::NodeInfo *aNodeInfo, virtual nsGenericDOMDataNode* CloneDataNode(mozilla::dom::NodeInfo *aNodeInfo,
bool aCloneText) const override; bool aCloneText) const override;
}; };

View file

@ -1278,7 +1278,8 @@ Loader::PrepareSheet(StyleSheet* aSheet,
const nsSubstring& aMediaString, const nsSubstring& aMediaString,
nsMediaList* aMediaList, nsMediaList* aMediaList,
Element* aScopeElement, Element* aScopeElement,
bool isAlternate) bool isAlternate,
bool isExplicitlyEnabled)
{ {
NS_PRECONDITION(aSheet, "Must have a sheet!"); NS_PRECONDITION(aSheet, "Must have a sheet!");
@ -1307,7 +1308,7 @@ Loader::PrepareSheet(StyleSheet* aSheet,
sheet->SetMedia(mediaList); sheet->SetMedia(mediaList);
sheet->SetTitle(aTitle); sheet->SetTitle(aTitle);
sheet->SetEnabled(!isAlternate); sheet->SetEnabled(!isAlternate || isExplicitlyEnabled);
sheet->SetScopeElement(aScopeElement); sheet->SetScopeElement(aScopeElement);
} }
@ -1985,7 +1986,8 @@ Loader::LoadInlineStyle(nsIContent* aElement,
Element* aScopeElement, Element* aScopeElement,
nsICSSLoaderObserver* aObserver, nsICSSLoaderObserver* aObserver,
bool* aCompleted, bool* aCompleted,
bool* aIsAlternate) bool* aIsAlternate,
bool* aIsExplicitlyEnabled)
{ {
LOG(("css::Loader::LoadInlineStyle")); LOG(("css::Loader::LoadInlineStyle"));
NS_ASSERTION(mParsingDatas.Length() == 0, "We're in the middle of a parse?"); 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"); "Inline sheets should not be cached");
LOG((" Sheet is alternate: %d", *aIsAlternate)); 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)) { if (aElement->HasFlag(NODE_IS_IN_SHADOW_TREE)) {
ShadowRoot* containingShadow = aElement->GetContainingShadow(); ShadowRoot* containingShadow = aElement->GetContainingShadow();
@ -2059,7 +2062,8 @@ Loader::LoadStyleLink(nsIContent* aElement,
ReferrerPolicy aReferrerPolicy, ReferrerPolicy aReferrerPolicy,
const nsAString& aIntegrity, const nsAString& aIntegrity,
nsICSSLoaderObserver* aObserver, nsICSSLoaderObserver* aObserver,
bool* aIsAlternate) bool* aIsAlternate,
bool* aIsExplicitlyEnabled)
{ {
LOG(("css::Loader::LoadStyleLink")); LOG(("css::Loader::LoadStyleLink"));
NS_PRECONDITION(aURL, "Must have URL to load"); NS_PRECONDITION(aURL, "Must have URL to load");
@ -2112,8 +2116,9 @@ Loader::LoadStyleLink(nsIContent* aElement,
NS_ENSURE_SUCCESS(rv, rv); NS_ENSURE_SUCCESS(rv, rv);
LOG((" Sheet is alternate: %d", *aIsAlternate)); 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); rv = InsertSheetInDoc(sheet, aElement, mDocument);
NS_ENSURE_SUCCESS(rv, rv); NS_ENSURE_SUCCESS(rv, rv);
@ -2138,9 +2143,10 @@ Loader::LoadStyleLink(nsIContent* aElement,
aObserver, principal, requestingNode); aObserver, principal, requestingNode);
NS_ADDREF(data); 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 && if (aURL && state == eSheetNeedsParser && mSheets->mLoadingDatas.Count() != 0 &&
*aIsAlternate) { *aIsAlternate && !*aIsExplicitlyEnabled) {
LOG((" Deferring alternate sheet load")); LOG((" Deferring alternate sheet load"));
URIPrincipalReferrerPolicyAndCORSModeHashKey key(data->mURI, URIPrincipalReferrerPolicyAndCORSModeHashKey key(data->mURI,
data->mLoaderPrincipal, data->mLoaderPrincipal,
@ -2267,6 +2273,7 @@ Loader::LoadChildSheet(StyleSheet* aParentSheet,
state = eSheetComplete; state = eSheetComplete;
} else { } else {
bool isAlternate; bool isAlternate;
bool isExplicitlyEnabled;
const nsSubstring& empty = EmptyString(); const nsSubstring& empty = EmptyString();
// For now, use CORS_NONE for child sheets // For now, use CORS_NONE for child sheets
rv = CreateSheet(aURL, nullptr, principal, rv = CreateSheet(aURL, nullptr, principal,
@ -2277,7 +2284,7 @@ Loader::LoadChildSheet(StyleSheet* aParentSheet,
false, empty, state, &isAlternate, &sheet); false, empty, state, &isAlternate, &sheet);
NS_ENSURE_SUCCESS(rv, rv); 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); rv = InsertChildSheet(sheet, aParentSheet, aParentRule);
@ -2390,6 +2397,7 @@ Loader::InternalLoadNonDocumentSheet(nsIURI* aURL,
StyleSheetState state; StyleSheetState state;
bool isAlternate; bool isAlternate;
bool isExplicitlyEnabled;
RefPtr<StyleSheet> sheet; RefPtr<StyleSheet> sheet;
bool syncLoad = (aObserver == nullptr); bool syncLoad = (aObserver == nullptr);
const nsSubstring& empty = EmptyString(); const nsSubstring& empty = EmptyString();
@ -2399,7 +2407,7 @@ Loader::InternalLoadNonDocumentSheet(nsIURI* aURL,
false, empty, state, &isAlternate, &sheet); false, empty, state, &isAlternate, &sheet);
NS_ENSURE_SUCCESS(rv, rv); NS_ENSURE_SUCCESS(rv, rv);
PrepareSheet(sheet, empty, empty, nullptr, nullptr, isAlternate); PrepareSheet(sheet, empty, empty, nullptr, nullptr, isAlternate, isExplicitlyEnabled);
if (state == eSheetComplete) { if (state == eSheetComplete) {
LOG((" Sheet already complete")); LOG((" Sheet already complete"));

View file

@ -230,6 +230,8 @@ public:
* @param [out] aCompleted whether parsing of the sheet completed. * @param [out] aCompleted whether parsing of the sheet completed.
* @param [out] aIsAlternate whether the stylesheet ended up being an * @param [out] aIsAlternate whether the stylesheet ended up being an
* alternate sheet. * alternate sheet.
* @param [out] aIsExplicitlyEnabled whether the stylesheet was explicitly
* enabled by having the disabled attribute removed.
*/ */
nsresult LoadInlineStyle(nsIContent* aElement, nsresult LoadInlineStyle(nsIContent* aElement,
const nsAString& aBuffer, const nsAString& aBuffer,
@ -239,7 +241,8 @@ public:
mozilla::dom::Element* aScopeElement, mozilla::dom::Element* aScopeElement,
nsICSSLoaderObserver* aObserver, nsICSSLoaderObserver* aObserver,
bool* aCompleted, bool* aCompleted,
bool* aIsAlternate); bool* aIsAlternate,
bool* aIsExplicitlyEnabled);
/** /**
* Load a linked (document) stylesheet. If a successful result is returned, * 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 * @param [out] aIsAlternate whether the stylesheet actually ended up beinga
* an alternate sheet. Note that this need not match * an alternate sheet. Note that this need not match
* aHasAlternateRel. * aHasAlternateRel.
* @param [out] aIsExplicitlyEnabled whether the stylesheet was explicitly
* enabled by having the disabled attribute removed.
*/ */
nsresult LoadStyleLink(nsIContent* aElement, nsresult LoadStyleLink(nsIContent* aElement,
nsIURI* aURL, nsIURI* aURL,
@ -270,7 +275,8 @@ public:
ReferrerPolicy aReferrerPolicy, ReferrerPolicy aReferrerPolicy,
const nsAString& aIntegrity, const nsAString& aIntegrity,
nsICSSLoaderObserver* aObserver, nsICSSLoaderObserver* aObserver,
bool* aIsAlternate); bool* aIsAlternate,
bool* aIsExplicitlyEnabled);
/** /**
* Load a child (@import-ed) style sheet. In addition to loading the sheet, * Load a child (@import-ed) style sheet. In addition to loading the sheet,
@ -476,7 +482,8 @@ private:
const nsAString& aMediaString, const nsAString& aMediaString,
nsMediaList* aMediaList, nsMediaList* aMediaList,
dom::Element* aScopeElement, dom::Element* aScopeElement,
bool isAlternate); bool isAlternate,
bool isExplicitlyEnabled);
nsresult InsertSheetInDoc(StyleSheet* aSheet, nsresult InsertSheetInDoc(StyleSheet* aSheet,
nsIContent* aLinkingContent, nsIContent* aLinkingContent,