Bug 1678987 - Make page style menu deal with <link disabled> better. r=mconley

<link disabled> doesn't appear in document stylesheets, but some pages
use it to use alternative stylesheets.

Differential Revision: https://phabricator.services.mozilla.com/D97988
This commit is contained in:
Emilio Cobos Álvarez 2020-12-04 01:34:15 +00:00
parent cc933e516a
commit fe363fa80f
5 changed files with 97 additions and 25 deletions

View File

@ -20,7 +20,6 @@ class PageStyleChild extends JSWindowActorChild {
if (!window || window.closed) {
return;
}
let filteredStyleSheets = this._collectStyleSheets(window);
this.sendAsyncMessage("PageStyle:Add", {
filteredStyleSheets,
@ -51,32 +50,69 @@ class PageStyleChild extends JSWindowActorChild {
}
}
/**
* Returns links that would represent stylesheets once loaded.
*/
_collectLinks(document) {
let result = [];
for (let link of document.querySelectorAll("link")) {
if (link.namespaceURI !== "http://www.w3.org/1999/xhtml") {
continue;
}
let isStyleSheet = Array.from(link.relList).some(
r => r.toLowerCase() == "stylesheet"
);
if (!isStyleSheet) {
continue;
}
if (!link.href) {
continue;
}
result.push(link);
}
return result;
}
/**
* Switch the stylesheet so that only the sheet with the given title is enabled.
*/
_switchStylesheet(title) {
let docStyleSheets = this.document.styleSheets;
let document = this.document;
let docStyleSheets = Array.from(document.styleSheets);
let links;
// Does this doc contain a stylesheet with this title?
// If not, it's a subframe's stylesheet that's being changed,
// so no need to disable stylesheets here.
let docContainsStyleSheet = !title;
if (title) {
for (let docStyleSheet of docStyleSheets) {
if (docStyleSheet.title === title) {
docContainsStyleSheet = true;
break;
links = this._collectLinks(document);
docContainsStyleSheet =
docStyleSheets.some(sheet => sheet.title == title) ||
links.some(link => link.title == title);
}
for (let sheet of docStyleSheets) {
if (sheet.title) {
if (docContainsStyleSheet) {
sheet.disabled = sheet.title !== title;
}
} else if (sheet.disabled) {
sheet.disabled = false;
}
}
for (let docStyleSheet of docStyleSheets) {
if (docStyleSheet.title) {
if (docContainsStyleSheet) {
docStyleSheet.disabled = docStyleSheet.title !== title;
// If there's no title, we just need to disable potentially-enabled
// stylesheets via document.styleSheets, so no need to deal with links
// there.
//
// We don't want to enable <link rel="stylesheet" disabled> without title
// that were not enabled before.
if (title) {
for (let link of links) {
if (link.title == title && link.disabled) {
link.disabled = false;
}
} else if (docStyleSheet.disabled) {
docStyleSheet.disabled = false;
}
}
}
@ -89,10 +125,12 @@ class PageStyleChild extends JSWindowActorChild {
*/
_collectStyleSheets(content) {
let result = [];
let document = content.document;
// Only stylesheets with a title can act as an alternative stylesheet.
for (let sheet of content.document.styleSheets) {
if (!sheet.title) {
for (let sheet of document.styleSheets) {
let title = sheet.title;
if (!title) {
// Sheets without a title are not alternates.
continue;
}
@ -102,10 +140,37 @@ class PageStyleChild extends JSWindowActorChild {
continue;
}
result.push({
title: sheet.title,
disabled: sheet.disabled,
});
// We skip links here, see below.
if (
sheet.href &&
sheet.ownerNode &&
sheet.ownerNode.nodeName.toLowerCase() == "link"
) {
continue;
}
let disabled = sheet.disabled;
result.push({ title, disabled });
}
// This is tricky, because we can't just rely on document.styleSheets, as
// `<link disabled>` makes the sheet don't appear there at all.
for (let link of this._collectLinks(document)) {
let title = link.title;
if (!title) {
continue;
}
let media = link.media;
if (media && !content.matchMedia(media).matches) {
continue;
}
let disabled =
link.disabled ||
!!link.sheet?.disabled ||
document.preferredStyleSheetSet != title;
result.push({ title, disabled });
}
return result;

View File

@ -23,6 +23,8 @@ const WEB_ROOT = getRootDirectory(gTestPath).replace(
"http://example.com"
);
const kStyleSheetsInPageStyleSample = 18;
/*
* Test that the right stylesheets do (and others don't) show up
* in the page style menu.
@ -35,7 +37,7 @@ add_task(async function test_menu() {
);
let browser = tab.linkedBrowser;
BrowserTestUtils.loadURI(browser, WEB_ROOT + "page_style_sample.html");
await promiseStylesheetsLoaded(tab, 17);
await promiseStylesheetsLoaded(tab, kStyleSheetsInPageStyleSample);
let menuitems = fillPopupAndGetItems();
let items = menuitems.map(el => ({
@ -96,7 +98,7 @@ add_task(async function test_menu() {
let disableStyles = document.getElementById("menu_pageStyleNoStyle");
let defaultStyles = document.getElementById("menu_pageStylePersistentOnly");
let otherStyles = menuitems[menuitems.length - 1];
let otherStyles = menuitems[0].parentNode.querySelector("[label='28']");
// Assert that the menu works as expected.
disableStyles.click();
@ -163,8 +165,12 @@ add_task(async function test_page_style_file() {
new FileUtils.File(getTestFilePath("page_style_sample.html"))
).spec;
await BrowserTestUtils.withNewTab(FILE_PAGE, async function(browser) {
await promiseStylesheetsLoaded(browser, 17);
await promiseStylesheetsLoaded(browser, kStyleSheetsInPageStyleSample);
let menuitems = fillPopupAndGetItems();
is(menuitems.length, 17, "Should have 17 items even for file: URI.");
is(
menuitems.length,
kStyleSheetsInPageStyleSample,
"Should have the right amount of items even for file: URI."
);
});
});

View File

@ -15,7 +15,7 @@ add_task(async function() {
);
let browser = tab.linkedBrowser;
BrowserTestUtils.loadURI(browser, PAGE);
await promiseStylesheetsLoaded(tab, 17);
await promiseStylesheetsLoaded(tab, 18);
let menupopup = document.getElementById("pageStyleMenu").menupopup;
gPageStyleMenu.fillPopup(menupopup);

View File

@ -401,6 +401,7 @@ async function promiseStylesheetsLoaded(tab, styleSheetCount) {
await TestUtils.waitForCondition(() => {
let menu = styleMenu._pageStyleSheets.get(permanentKey);
info(`waiting for sheets: ${menu && menu.filteredStyleSheets.length}`);
return menu && menu.filteredStyleSheets.length >= styleSheetCount;
}, "waiting for style sheets to load");
}

View File

@ -38,8 +38,8 @@
<link data-state="1" href="404.css" title="25" rel="alternate stylesheet" media="only screen">
<link data-state="1" href="404.css" title="26" rel="alternate stylesheet" media="screen and (min-device-width: 1px)">
<link data-state="0" href="404.css" title="27" rel="alternate stylesheet" media="screen and (max-device-width: 1px)">
<!-- It's important that this is the last one for the purposes of the test. -->
<style data-state="1" title="28">:root { color: blue }</style>
<link data-state="1" href="404.css" title="29" rel="alternate stylesheet" disabled>
</head>
<body></body>
</html>