Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inject styles in the head #252

Closed
yurikhan opened this issue Nov 20, 2017 · 10 comments
Closed

Inject styles in the head #252

yurikhan opened this issue Nov 20, 2017 · 10 comments

Comments

@yurikhan
Copy link

  • Browser: Firefox 59 Nightly but it probably does not matter
  • Operating System: GNU/Linux but it does not matter

There is this webcomic network called Hiveworks. At least some of the comics published there include this script: http://www.thehiveworks.com/jumpbar.js

Its last line is:

window.addEventListener("load",breakbadtoys2,true);

where the referenced function contains, among other things:

//script kiddies and malware break html standards
var St = document.getElementsByTagName('style');
for( var x = 0; St[x]; x++ ) 
{
    if(St[x].parentNode!=document.head)
    {
    
	St[x].parentNode.removeChild(St[x]);
    }
}

Incidentally, they are right — style is only valid under head.

Stylus injects user styles after the body, which gets them removed by the above script.

What are the consequences of injecting styles at the end of head? One is that user style rules with specificity equal to those in the page body will no longer win by position tiebreaker; are there others?

@Mottie
Copy link
Member

Mottie commented Nov 20, 2017

Adding the style after the closing body tag ensures that the style is always last and thus overrides selectors with the same specificity. It's especially needed with the increase use of !important flags in some frameworks (e.g. semantic UI and GitHub's primer).

I think your best bet to get around this is to overwrite the breakbadtoys2 function using this userscript:

// ==UserScript==
// @name         Allow Stylus
// @namespace    http://tampermonkey.net/
// @version      0.1.0
// @description  Disable style tag removal
// @author       Rob Garrison
// @match        http://*/*
// @grant        none
// ==/UserScript==
(() => {
 "use strict";
 const scrpt = document.querySelector("script[src*='thehiveworks.com/jumpbar.js']");
 if (scrpt && window.breakbadtoys2) {
  window.breakbadtoys2 = () => false;
 }
})();

@yurikhan
Copy link
Author

Hm. I would be content to artificially increase my selectors’ specificity (by prefixing with html body, or by doubling selectors .myclass.myclass, or using :not(:not()), etc.), in exchange for warm fuzzies from using an extension that does not deliberately violate the specification.

@Mottie
Copy link
Member

Mottie commented Nov 20, 2017

The problem actually is that the original Stylish extension behaved the same way, so moving the styles back to the head would likely break many existing userstyles... either way, I'll let @tophf chime in here.

@yurikhan
Copy link
Author

I do believe the original Stylish for Firefox, which I believe was the original Stylish, did not. In fact, I do not remember it ever injecting styles into the actual DOM. It used the loadAndRegisterSheet method of the browser’s Stylesheet Service to add userstyles to the same collection that had the user agent stylesheet (browser defaults) and the userChrome.css and userContent.css overrides.

It did register styles as author sheets, though, so yes, existing styles would probably rely on having ties broken in their favor.

@mechalynx
Copy link

mechalynx commented Nov 20, 2017

@yurikhan original Stylish for Firefox used XPCOM, which has been taken out. Not sure there's an alternative to doing it this way with webExtensions (which is similar to how Stylish for Chrome would have to work, which is what Stylus is forked from).

You might be ok with increasing specificity, but it's not just existing styles that would break, it's also that the behavior would be surprising to userstyle authors in general. Not sure there's a way to wrap an existing style in a way that would work around the higher specificity needs (and if there is, that site's function would be pointless). I think most userstyle authors would reasonably consider behavior like this from a userstyle manager to be a bug, not a feature.

Adding a style at the end of the <body> tag wouldn't override inline CSS with !important for example, which, as Mottie said, is something frameworks sometimes do.

@yurikhan
Copy link
Author

@mechalynx

Not sure there's an alternative to doing it this way with webExtensions

I believe there is but @tophf has issues with it. For not being Chrome-friendly, of all things.

@Mottie
Copy link
Member

Mottie commented Nov 20, 2017

We need control over the css to allow enabling & disabling when the user changes the state in the popup; insertCSS doesn't easily allow for this need.

@mechalynx
Copy link

@yurikhan seems this is already addressed in #248 ? I don't think @tophf decided not to do anything about it, just hasn't come around to dealing with it yet. The problem wasn't that it wasn't Chrome-friendly, but that it's different between the two major browsers, increasing maintenance cost.

@tophf
Copy link
Member

tophf commented Nov 21, 2017

Lots of sites and frameworks put stuff directly under the root element so it's not a problem for the browsers. The only thing worth discussing here is whether it's worth adding a style protector that would restore the styles immediately.

@tophf
Copy link
Member

tophf commented Nov 21, 2017

Also, DOM specification doesn't restrict documentElement children elements type so we don't violate it.

@tophf tophf closed this as completed in 8a1908b Nov 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants