openstyles / stylus

Stylus - Userstyles Manager
https://add0n.com/stylus.html
GNU General Public License v3.0
5.29k stars 299 forks source link

Inject styles in the head #252

Closed yurikhan closed 6 years ago

yurikhan commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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