Closed Bug 1310026 Opened 3 years ago Closed 3 years ago

Insert user CSS

Categories

(WebExtensions :: Frontend, defect, P3)

defect

Tracking

(firefox53 fixed)

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed
Blocking Flags:
webextensions +

People

(Reporter: andy+bugzilla, Assigned: evilpie)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, Whiteboard: triaged)

Attachments

(1 file, 1 obsolete file)

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."
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.
Correction. It's currently inserted as AUTHOR_SHEET. This bug would allow insertion as a USER_SHEET.

Sorry for the confusion.
Priority: -- → P3
Whiteboard: triaged
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.
See Also: → 1288917
Assignee: nobody → evilpies
Attached patch WIP (obsolete) — Splinter Review
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.
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.
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.
webextensions: --- → +
(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?"
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"
.
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)
I like "role"! In which sense is it taken? It's not a property of InjectDetails.
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.
Attachment #8817543 - Attachment is obsolete: true
Attachment #8821688 - Flags: review?(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)
Attachment #8821688 - Flags: review?(aswan) → review?(kmaglione+bmo)
Attachment #8821688 - Attachment description: Add "rule" option for insertCSS/removeCSS → Add "role" option for insertCSS/removeCSS
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
Attachment #8821688 - Flags: review?(kmaglione+bmo) → review+
Please see the previous discussion, I don't think origin is a good name to use.
I'm still not comfortable defining a whole new term for this, when one already exists in the spec... How about something like "originRole"?
We agreed to cssOrigin on IRC.
Pushed by evilpies@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/68402c8f7fd2
webext: Add cssOrigin option to insertCSS/removeCSS. r=kmag
https://hg.mozilla.org/mozilla-central/rev/68402c8f7fd2
Status: NEW → RESOLVED
Closed: 3 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
I've added this here: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/tabs/insertCSS

Let me know if this covers it.
Flags: needinfo?(evilpies)
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.
Flags: needinfo?(evilpies)
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)?
Flags: needinfo?(wbamberg)
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.
> the cssOrigin arguments should also be added to...

Sorry to be so slow on this. This is done (not by me...)
Flags: needinfo?(wbamberg)
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.