1678987 - View -> Page Style menu is broken after changes in document.styleSheets

Status: RESOLVED FIXED

(Firefox :: Menus, defect, P1)

Description

[Siemens]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:83.0) Gecko/20100101 Firefox/83.0

Steps to reproduce:

Press Alt key on any site with alternative stylesheets (e.g. https://iichan.hk/b/).
Choose View -> Page Style. You can't select alternative stylesheets - there aren't any.

It's a regression due to removing disabled stylesheets from document.styleSheets for consistency with weird Blink/Webkit behavior. (https://github.com/whatwg/html/issues/3840) But this menu relies on the assumption that document.styleSheets contains all available stylesheets on the page.

Actual results:

The menu displays only currently selected stylesheet and No Style.

Expected results:

The menu should display all alternative stylesheets.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

So the relevant code is this:

function set_stylesheet(styletitle,target)
{
	set_cookie("wakabastyle",styletitle,365);

	var links = target ? target.document.getElementsByTagName("link") : document.getElementsByTagName("link");
	var found=false;
	for(var i=0;i<links.length;i++)
	{
		var rel=links[i].getAttribute("rel");
		var title=links[i].getAttribute("title");
		if(rel.indexOf("style")!=-1&&title)
		{
			links[i].disabled=true; // IE needs this to work. IE needs to die.
			if(styletitle==title) { links[i].disabled=false; found=true; }
		}
	}
	if(!found)
	{
		if(target) set_preferred_stylesheet(target);
		else set_preferred_stylesheet();
	}
}

function set_preferred_stylesheet(target)
{
	var links = target ? target.document.getElementsByTagName("link") : document.getElementsByTagName("link");
	for(var i=0;i<links.length;i++)
	{
		var rel=links[i].getAttribute("rel");
		var title=links[i].getAttribute("title");
		if(rel.indexOf("style")!=-1&&title) links[i].disabled=(rel.indexOf("alt")!=-1);
	}
}

If instead of links[i].disabled the page would use links[i].sheet?.disabled, then it'd work as before.

I think this is kind of expected given the changes in bug 1281135. But we could tweak this menu to look at those links as well, potentially.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

I can try to poke when I have some time, though this is probably not super-high priority.

The relevant code lives in browser/actors/PageStyleChild.jsm, and I think it should be relatively easy to handle this case, if someone wants to steal I'd appreciate it :-)

Comment 3

[Siemens]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #1)

If instead of links[i].disabled the page would use links[i].sheet?.disabled, then it'd work as before.

And it'd stop work in Blink :)

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Hmm, Blink should've fixed to match our behavior already iiuc. But yeah it'd stop working in WebKit I guess.

Comment 5

[Siemens]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)

Hmm, Blink should've fixed to match our behavior already iiuc. But yeah it'd stop working in WebKit I guess.

Chrome 87, works only if links were toggled from DOM beforehand (link.disabled = true; link.disabled = false). Then you can switch styles through link.sheet.disabled. But in case of the DOM attribute I can manipulate it right after <link> tag appeared in the DOM tree, and for link.sheet I have to wait until the stylesheets are loaded, and only then I can toggle them. (link.sheet is null while they are loading)
For example, if I have to switch the style to a user-selected one by javascript in browser while page loading (e.g. by cookie), the user at first will see the page with default stylesheet applied and only then in the selected one, because the safe time when I can toggle link.sheet.disabled in only after onload event.
If I toggle stylesheets using link.disabled, user will see preferred stylesheet without "blinking" with standard one, because I don't have to wait onload event in this case.
So, using link.sheet.disabled is definitely inconvenient, and link.disabled can't be safely replaced to link.sheet.disabled.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

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

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

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5e796e573157
Make page style menu deal with <link disabled> better. r=mconley

Comment 8

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/5e796e573157