1310026 - Insert user CSS

Status: RESOLVED FIXED

(WebExtensions :: Frontend, defect, P3)

Description

[Andy McKay]

We have tabs.insertCSS, but in bug 1226547 comment 1 its mentioned why we'd want a user CSS to be installed. This would be a CSS that:

"The latter likely needs to come with a "super-power" over its Chrome counterpart, which gives inserted stylesheets lower priority than content and does not honor !important: we should have an "overrideContent" flag or something like that, in order to prevent web pages from trumping extension-provided CSS rules."

Comment 1

[Andy McKay]

Currently tabs.insertCSS inserts a CSS as a USER_SHEET:

https://dxr.mozilla.org/mozilla-central/source/toolkit/components/extensions/ExtensionContent.jsm#193

If we allowed it to be inserted as an AUTHOR_SHEET:

https://dxr.mozilla.org/mozilla-central/source/layout/base/nsIStyleSheetService.idl

Then it would be able to override the content sheets.

Comment 2

[Andy McKay]

Correction. It's currently inserted as AUTHOR_SHEET. This bug would allow insertion as a USER_SHEET.

Sorry for the confusion.

Comment 3

[Nancy Grossman]

This can be easily done by adding an `origin` property which defaults to `AUTHOR_SHEET` to the `details` argument of `tabs.insertCSS` and `tabs.removeCSS` (bug 1247455). This is also useful to Stylish-y add-ons, both to override `!important` style attributes and to style scrollers in the page.

Comment 4

[Tom S]

Attachment: WIP

Comment 5

[Tom S]

I am open to naming suggestion, I don't feel like "origin" is really that great. Origin has another much more common meaning in the context of HTTP. Uh and we seem to share InjectDetails between JS and CSS.

Comment 6

[Matt]

I'm not really involved in the details here, but I like naming things.

What about 'definer' or 'cssDefiner'? It gets at the heart of what an AUTHOR or USER _SHEET means.

Comment 7

[Andy McKay]

I got to about the same point you did with a patch, which naughty me, I didn't add it to bugzilla. I called it type, purely because the SDK calls it that: https://developer.mozilla.org/en-US/Add-ons/SDK/Low-Level_APIs/stylesheet_utils

But I agree origin is so overloaded with other meanings.

Comment 8

[Stephan Sokolow]

(In reply to Matt from comment #6)
> I'm not really involved in the details here, but I like naming things.
> 
> What about 'definer' or 'cssDefiner'? It gets at the heart of what an AUTHOR
> or USER _SHEET means.

Or it could be something like 'precedence'. That's even more clear as to its role, being similar in meaning to "priority" but with more of a "which wins" connotation than priority's "what's the ordering?"

Comment 9

[Masatoshi Kimura [:emk]]

I don't think "priority" or "precedence" is appropriate because the cascading order is
1. user "!important"
2. author "!important"
3. author non-"!important"
4. user non-"!important"
.

Comment 10

[Stephan Sokolow]

Hmm. You make a good point. I'd forgotten that customizing the browser default and overriding a site's choices were two sides of the same mechanism.

I'd suggest "role" but that's already taken.

That said, I still think using an -er suffix sounds wrong unless used to name a class which implements some action. (eg. FooDownloader, BarCopier, BazHasher)

Comment 11

[Tom S]

I like "role"! In which sense is it taken? It's not a property of InjectDetails.

Comment 12

[Stephan Sokolow]

Huh. You're right. For some reason, my tired mind was telling me that screen/print/etc. were "roles" rather than "media types" and, if that were true, it'd have the same kind of problem as "origin".

However, I'd say that stylesheets and HTML element accessibility are distinct enough for such a common English word to be used by both without confusion.

Comment 13

[Tom S]

Attachment: Add "role" option for insertCSS/removeCSS

Comment 14

[Andrew Swan [:aswan]]

Comment on attachment 8821688 [details] [diff] [review]
Add "role" option for insertCSS/removeCSS

Redirecting to Kris.  My only minor comment is that you've misspelled the name of the new property in the patch subject (rule/role)

Comment 15

[Kris Maglione [:kmag]]

Comment on attachment 8821688 [details] [diff] [review]
Add "role" option for insertCSS/removeCSS

Review of attachment 8821688 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with "role" changed to "origin".

Thanks!

::: browser/components/extensions/ext-tabs.js
@@ +844,4 @@
>          } else {
>            options.run_at = "document_idle";
>          }
> +        if (details.role !== null) {

This CSS spec calls this the origin, not the role:

https://www.w3.org/TR/CSS2/cascade.html#cascade

Comment 16

[Tom S]

Please see the previous discussion, I don't think origin is a good name to use.

Comment 17

[Kris Maglione [:kmag]]

I'm still not comfortable defining a whole new term for this, when one already exists in the spec... How about something like "originRole"?

Comment 18

[Tom S]

We agreed to cssOrigin on IRC.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/68402c8f7fd2
webext: Add cssOrigin option to insertCSS/removeCSS. r=kmag

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/68402c8f7fd2

Comment 21

[Will Bamberg [:wbamberg]]

I've added this here: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/tabs/insertCSS

Let me know if this covers it.

Comment 22

[Tom S]

The text looks good. don't forget to add a new row to the compatibility table and update https://developer.mozilla.org/en-US/Firefox/Releases/53.

Comment 23

[Frédéric Wang (:fredw)]

Hey, IIUC the cssOrigin arguments should also be added to https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/tabs/removeCSS and maybe https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/extensionTypes/InjectDetails (indicating it's specific to *CSS functions)?

Comment 24

[perdrisat]

I'm using this on https://addons.mozilla.org/en-US/developers/addon/no-transition but it doesn't really works.

I do somethings like that (all the code is on https://github.com/gagarine/no-transition) :

const CSS = `
body * {
  /*CSS transition*/
  transition: none !important;
  /*CSS animations*/
  animation: none !important;
 }
`;
browser.tabs.insertCSS(tab.id, { code: CSS, cssOrigin: 'user' });


I can overwrite almost all animation, but not all. It seem !important is not taken in account and cssOrigin: 'user' doesn't mean it can't be overwrite.

For example on https://www.zuerich.com/en the "more" button is still changing his with an animation.

Comment 25

[Will Bamberg [:wbamberg]]

> the cssOrigin arguments should also be added to...

Sorry to be so slow on this. This is done (not by me...)