Open yurikhan opened 6 years ago
tabs.insertCSS approach has some problems which I've described several times over the last year here and there.
Right now I'm not interested in the least. In the future, yeah quite possible. Maybe someone else will do it as I'm just another user and I don't visit weird sites that remove our styles or overwrite their pages entirely.
Observe that the page script notices the user style and removes it, restoring its ugly brown background
We can protect the style elements, it's neither hard nor CPU-intensive.
Makes sense, thanks. I will watch for any cases of style disappearance and try to understand how it happens.
I remember when we dealt with mmo-champion.com page, which was rewritten by uBlock Extra extension via document.write, I experimented with protecting and used for that a non-recursive (hence superfast) MutationObserver on the documentElement. It turned out unnecessary so the change wasn't committed. If you find real sites (not PoC) that delete our style elements, I'll reintroduce the feature. Probably with some kind of time limit in order not to cause deadlocks with a super aggressive site script.
Another approach is to insert style code inside a ShadowDOM root on documentElement - it requires dynamic rewriting of all CSS selectors in order to add :host
or something like that, also doable since we have CSS parser.
It looks like there is a Chrome feature request to add removeCSS
:
https://bugs.chromium.org/p/chromium/issues/detail?id=608854
@yurikhan actually, it looks like the patch for it already exists and has more or less passed review as well. It's also compatible with the Firefox implementation. Seems like this could land in Chrome any day now. Perhaps the solution is for someone representative (as in, a project contributor+) to draw attention to that bug?
~There'll be another problem in Chrome 64 and on - it switched to injecting user level stylesheets instead of author level - to be able to override page !important rules - and it means that it breaks lots of injected CSS that don't use !important since user level stylesheets have lower priority as per the specification, and Chrome doesn't offer cssOrigin in tabs.insertCSS~
Update: crrev.com/c/765642 adds CSSOrigin to the internals so we can hope it'll be exposed in the extensions API. Not that it solves the bigger problem with the lack of tabs.removeCSS in Chrome...
Test version that uses tabs.insertCSS everywhere in FF [edit: updated the link] fixes the issue. Needs testing with style updates, editing, basically everything. See temporary installation article.
Gave that test version a quick try in Firefox Nightly 59.0a1 (2018-01-05):
I noticed this warning when loading the temporary addon, but the extension still worked:
Reading manifest: Error processing permissions.4: Value "declarativeContent" must either: must either [must either [be one of ["clipboardRead", "clipboardWrite", "geolocation", "idle", "notifications"], be one of ["bookmarks"], be one of ["find"], be one of ["history"], be one of ["activeTab", "tabs"], be one of ["browserSettings"], be one of ["cookies"], be one of ["topSites"], be one of ["webNavigation"], or be one of ["webRequest", "webRequestBlocking"]], be one of ["alarms", "mozillaAddons", "storage", "unlimitedStorage"], be one of ["browsingData"], be one of ["devtools"], be one of ["identity"], be one of ["menus", "contextMenus"], be one of ["pkcs11"], be one of ["geckoProfiler"], be one of ["sessions"], be one of ["contextualIdentities"], be one of ["downloads", "downloads.open"], be one of ["management"], be one of ["privacy"], be one of ["proxy"], be one of ["nativeMessaging"], be one of ["theme"], or match the pattern /^experiments(\.\w+)+$/], or must either [be one of ["<all_urls>"], match the pattern /^(https?|wss?|file|ftp|\*):\/\/(\*|\*\.[^*/]+|[^*/]+)\/.*$/, or match the pattern /^file:\/\/\/.*$/]
Updating a style seems to be partially broken in this version. When I edit a style, the page style is not updated immeditately. When I reload the page, it is unstyled on first reload but when reloading a second time, the style gets applied again.
I strongly recommend using the cssOrigin: 'user'
option because that allows userstyles to outweight !important
page styles, which is pretty crucial for styles to work on many sites.
@silverwind, that version is buggy and currently not developed, mainly because I don't use FF and the implementation turned out to be more complicated - Stylus has to enumerate sub-frames on each page, and IIRC there are certain bugs in various versions of FF that aren't easy to circumvent.
Error processing permissions.4: Value "declarativeContent"
You can disregard it.
I strongly recommend using the cssOrigin: 'user' option
Bad idea. Making it the default will break all styles that don't use !important
because user
origin precedes author styles in the cascade. There are lots of styles that don't use !important
for the obvious reason (I'll cite it just in case: !important
is designed to be used as an exception, not as a rule). A better solution is to expose a per-style option (or a global one) to set the origin.
Bad idea. Making it the default will break all styles that don't use !important because user origin precedes author styles in the cascade
I'm aware of that, but that's how the old Stylish for Firefox actually worked and their docs recommend putting !important
everywhere, and IIRC correctly, most styles on userstyles.org use !important
everywhere. But I agree, a style option might be the best way to go about it.
a style option might be the best way to go about it.
It's the only way I'll accept.
Okay. BTW, can you elborate
we need to have a separate manifest.json for Chrome and Firefox
Is this because Chrome would error out when loading the extension because it requests unknown permissions?
we need to have a separate manifest.json for Chrome and Firefox
It's an old remark, no longer relevant as I found a way to use declarativeContent in Chrome.
Let's say that I'd like to add such a option and work on using insertCSS
in Firefox. Should I take current master
as a starting point for that, or should I wait for whatever rewrite you're planning?
You can use both branches, but be prepared to have a hard time as there are lots of edge cases. I don't know when I'll be interested in working on that branch. I don't plan to rewrite it per se, it's just the way it goes: I already rewrote it several times because the implementation was either incorrect or I got blocked by some bug in FF. Just found I have another attempt locally so I've pushed it into insertcss2
branch.
that's how the old Stylish for Firefox actually worked and their docs recommend putting !important everywhere
Bad outdated advice based on the web in those days with lots of inline styles etc.
Anyway, Stylus is based on Stylish-for-Chrome, with its more than a million of users, which always used author
origin because it inserted style elements in the page. We can't just change the way the styles are inserted based on the behavior of a technically unrelated Firefox addon, even though it was the original one. On the other hand, I understand that migrating Firefox users would expect the classic Stylish behavior. However, new users of Stylus in Firefox wouldn't necessarily expect it.
most styles on userstyles.org use !important everywhere.
I don't see that.
Actually according to the page I linked, even Stylish for Firefox was inserting author
sheets, so my impression that it was doing user
sheets was wrong. Anyways, I see it pretty important to allow using user
and possible (if browsers ever implement it) user-agent
sheets to allow style authors to deal with !important
page styles. This is especially needed for global styles.
Currently Stylus adds style elements at the end of DOM so a userstyle's !important
already overrides page's !important
with the same specificity. Which is why I'm reluctant to work on this branch: everything is working fine already. The only realistic benefit of tabs.insertCSS is overcoming longstanding FF bugs that prevent Stylus from styling certain dynamic iframes (FF's webNavigation API doesn't "see" them). But we could reintroduce iframe observers, which I've removed in favor of match_about_blank
in manifest.json that works correctly in Chrome. Another "benefit" is hiding the styles from sites, but that's a really contrived use case.
As for USER_AGENT, there's simply no method to implement it other than process the entire DOM manually. Obviously, it'll be slow.
Let me give you an example why I need user
sheets. I have a global style that does
body {
background-color: #111 !important;
}
Now take a look at the CSS of http://www.imdb.com/: they have
body#styleguide-v2 {
background-color: #e3e2dd !important;
}
In the old Stylish, I just used a user-agent
level sheet to overcome this specificity issue. In Stylus, I'd have to at least match the specificity of the id selector for every single page that's using !important
on their body
style. A method that obviously won't scale.
Well, USER_AGENT is out of the question.
As for the imdb example, you can simply use a specificity hack like body:not(#foo)
. IMHO this is on the same level of dirtiness as using !important
in the first place.
you can simply use a specificity hack like body:not(#foo)
Interesting. I haven't thought about exploiting :not
like that, I'll try that.
USER_AGENT is out of the question
Why thought? If we implement a select box for CSS origin and a browser supports inserting user agent sheets, why not provide the option?
tabs.insertCSS doesn't support user-agent
origin. How do you plan to override a random element's inline style with !important
inside? Obviously you need to find that element in DOM first, and replace its inline style. For that you'll have to use MutationObserver to be able to avoid FOUC, but observing makes the pages noticeably slower, especially if the user has other extensions monitoring the page like e.g. uBlock/AdBlock.
How do you plan to override a random element's inline style with !important inside
I meant we should only support it if the browser supports inserting user-agent
sheets of course, not hacks like you mentioned. Now I just need to convince Mozilla :)
Another concern is that tabs.insertCSS with author
origin doesn't have a way to specify the cascade order. Currently, we insert styles at the end of DOM so we fully control the cascade within 'author' origin and same-specificity selectors always win. I'm not sure it'll be the case with tabs.insertCSS.
tabs.insertCSS with author origin doesn't have a way to specify the cascade order
Yes, that sounds like a possible source of issues because insertCSS
does not mention the location of the inserted style. I'd say anything else than "last author sheet" should be considered a bug. Maybe they should provide an option like last
, first
to allow more granular control.
Now I just need to convince Mozilla :)
I think this discussion needs to be brought up on https://discourse.wicg.io/ to get feedback.
tabs.insertCSS with author origin doesn't have a way to specify the cascade order.
In practice, Firefox behaves sanely. My injected styles consistenly win over page styles.
In practice, Firefox behaves sanely. My injected styles consistenly win over page styles.
Uhm, this is too vague without describing whether the specificity was identical, whether !important
was used, and where. We need guarantees. My point is that without explicitly stated behavior in the WebExtensions API documentation we can't just blindly trust current behavior of tabs.insertCSS as it may change unpredictably due to some browser developer's whim.
Oh fine. I filed a documentation request for that.
I think this discussion needs to be brought up on https://discourse.wicg.io/ to get feedback.
Thanks for the pointer. That place seems not the right one for talking about web extensions, but I created a proposal for the cascade issue of user
vs author
origin styles, which is semi-related here.
FYI cssOrigin
has finally made it.
tabs.removeCSS
is on its way.
@mjethani It appears the patch is stuck in a merge conflict. is that the holdup ?
@uBlock-user the patch has been stuck in review forever. I have asked multiple times but received no response. I have no idea what the problem is, but I am disappointed by the lack of transparency and openness.
If you would like to see this resolved, please star Chromium issue #608854 and leave a comment explaining your use case.
@mjethani That's really disappointing to see that happen to you when you put so much effort and time into this. I have starred the issue.
it looks like the docs have finally been clarified for Firefox at least.
bugzilla issue: https://bugzilla.mozilla.org/show_bug.cgi?id=1428398 updated MDN docs: https://wiki.developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/tabs/insertCSS#Parameters
IIRC, Chromium is the problem. tabs.removeCSS
is not supported. It's been years now.
Right you are, sorry. I should've clarified that I was speaking specifically to this documentation request from earlier in this thread that was filed to assuage concerns about the cascade order.
@mjethani They mention there that it's stuck in a merge conflict c45. Any updates would be useful! Thanks a lot, I just want to use this in something I'm building for myself/in an extension :)
https://crrev.com/c/2250506 finally added removeCSS based on @mjethani's code so it'll be available in Chrome 87, right now in Chromium trunk builds, and Chrome Canary in a day or two.
No generate incompatible few CSS-es (?), inline <style>
can forget use !important;
in CSS / .user.css
projects.
Perhaps it is better to introduce detection of:
*[style*="important"] {
... : ...;
}
and then only try use tabs.insertCSS
/scripting.insertCSS()?
Currently, Stylus applies styles to most pages by sending them in a message to a content script which creates a
style
element and inserts it into the page DOM.(An exception to that is XML documents, for which the message gets reflected back to the background script and styles are applied via the
browser.tabs.insertCSS
API.)As a result of injection being performed from a content script, the page’s own scripts see the
style
elements and can tamper with them. As a realistic example, a slowly loading page can unintentionally overwrite the injected styles.Taken from the security point of view, this is an information disclosure vulnerability (page script has access to the userstyle source, can leak it to the site owner to fingerprint the user) and can be a tampering vulnerability (page script can deny the user’s right to customize site appearance).
I have made a proof-of-concept page.
To reproduce:
Create a global style or a style that applies specifically to that URI. E.g.:
@-moz-document url-prefix("http://centaur.ath.cx/test-styles.html") { body { background: #e4ffc7 } }
Navigate to that page. Observe that the page background is a pleasant light green, as specified by the user style.
Wait a second. Observe that the page script notices the user style and removes it, restoring its ugly brown background.
I have loaded the
apply-css
example extension that demonstrates theinsertCSS
API, and the style it injects is not detected by the page script.Is there any reason why
insertCSS
should not be used for all pages which it is able to handle?