Oh, I forgot to say that this issue is also a Mozilla WebExtensions bug, bug #1247455. 
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1247455

Could someone from Dev team please look into this feature request.

Thank you.

Hmm... interesting.  From an extension side, this is probably doable.  But I don't know if blink has support for removing user style sheets... Adding pdr@ in case he knows or knows someone who does.

In blink we just have a vector of injected stylesheets so it would be pretty easy to remove user added style sheet if there's some way to identify which one needs to be removed. I don't see an easy way to identify which inserted css would need to be removed. In any case, there's no issue in fixing this from the blink side as long as a suitable extension api can be added.

@4 Does blink have an internal way of identifying style sheets?  (The insertStyleSheet on the WebDocument API just takes a string.)

I don't see a concept of sheet ids so I think we'd need to change the insertCSS to return an id.

The code I'm looking at is m_injectedAuthorStyleSheets in:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/dom/StyleEngine.h&sq=package:chromium&type=cs&l=233&rcl=1463743191

cc'ing the CSS folks to keep me honest here.

pdr@'s comments seem sensible enough to me, it looks like it should be straightforwards to implement the Blink parts if we have some sort of id to refer to inserted sheets.

Mozilla is shipping this in Firefox 49. https://developer.mozilla.org/en-US/Firefox/Releases/49#WebExtensions

devlin@, pdr@.

Can I take this issue? 

It does look like the implementation should be pretty straightforward.

Basically, I'm going to follow the exactly same specification that Mozilla already implemented tabs.removeCss[1].

As pdr@ mentioned above, 
Blink Part: StyleEngine holds a vector for injected css. so the implementation should remove the corresponding css from the vector.

Extension Part: First add new method on tabs.json and then it sends IPC message to call something like ScriptInject::RemoveInjectedCss(id).

[1] https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/tabs/removeCSS

It looks like you have plenty of experience to implement this. I'll have to defer to rdevlin for the review though.

I've no objection to implementing this, and would be happy to review it!  If the below sounds good to you, feel free to grab the bug. :)

I do have some concerns around the FireFox API - in particular, the way to remove css is to pass in the same css that was added (which must exactly match).  This means the browser has to cache each addition of css.  In the case of files, this isn't too bad, since those are relatively short strings.  But raw css code could be very large indeed, and persisting a map of those would be...painful.

I'd prefer we make a tweak where instead we have an optional id parameter in injectCss, and then use that id to remove it, e.g.

chrome.tabs.injectCss({tabId: 1, code: 'body { background: "red"; }', id: 142});

chrome.tabs.removeCss({tabId: 1, id: 142});

id would be optional in injectCss, and required in removeCss.

It's probably also worth filling out a short proposal for this from the template at https://www.chromium.org/developers/design-documents/extensions/proposed-changes/apis-under-development.

Thank you for the comment. I'll write proposal first.

Awesome, thanks!  Looking forward to it! :)

Can removeCSS() take a injection id or a file path? Also, the insertCSS callback could include the CSSid as the argument, which would be the specified id or an automatically generated one. And what about support for frames?

Examples:

chrome.tabs.insertCSS(1, { file: "main.css", allFrames: true }, function() {
 chrome.tabs.removeCSS(1, { file: "main.css", allFrames: false })
})
chrome.tabs.insertCSS(2, { code: "body { color: blue }" }, function(id) {
 chrome.tabs.removeCSS(2, { id })
})

Also, that would be compatible with Firefox, at least for removal by path.

Hi Daniel,

Here's API proposal, 
https://docs.google.com/document/d/1rg88OKK88jGqG8Ez971vtjdpBkpuxpHgmQIRTsE89lE/edit?usp=sharing

I'm going to make chrome.tabs.insertCSS have a callback that has a parameter cssId(auto generated). and chrome.tabs.removeCSS will use the cssId.

I agree that chrome.tabs.removeCSS should be compatible with Mozilla implementation.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0ffd86f3ca15a4728605fa70435297d334ccc35a

commit 0ffd86f3ca15a4728605fa70435297d334ccc35a
Author: limasdf <limasdf@gmail.com>
Date: Fri May 26 05:21:38 2017

Provide a method to remove inserted style sheet

InsertStyleSheet() returns sequence id. And RemoveInsertedStyleSheet()
removes the inserted style sheet with its id.

TEST=webkit_unit_tests --gtest_fiter=StyleEngineTest.AnalyzedInject, WebDocumentTest.InsertAndRemoveStyleSheet
BUG=608854

Review-Url: https://codereview.chromium.org/2835183002
Cr-Commit-Position: refs/heads/master@{#474919}

[modify] https://crrev.com/0ffd86f3ca15a4728605fa70435297d334ccc35a/third_party/WebKit/Source/core/dom/DocumentStyleSheetCollection.cpp
[modify] https://crrev.com/0ffd86f3ca15a4728605fa70435297d334ccc35a/third_party/WebKit/Source/core/dom/StyleEngine.cpp
[modify] https://crrev.com/0ffd86f3ca15a4728605fa70435297d334ccc35a/third_party/WebKit/Source/core/dom/StyleEngine.h
[modify] https://crrev.com/0ffd86f3ca15a4728605fa70435297d334ccc35a/third_party/WebKit/Source/core/dom/StyleEngineTest.cpp
[modify] https://crrev.com/0ffd86f3ca15a4728605fa70435297d334ccc35a/third_party/WebKit/Source/web/WebDocument.cpp
[modify] https://crrev.com/0ffd86f3ca15a4728605fa70435297d334ccc35a/third_party/WebKit/Source/web/tests/WebDocumentTest.cpp
[modify] https://crrev.com/0ffd86f3ca15a4728605fa70435297d334ccc35a/third_party/WebKit/public/web/WebDocument.h

What's the status on the API side of this?

We're going to need this in Adblock Plus. Adblock Plus adds a style sheet with tabs.insertCSS, but we want to be able to modify the style sheet as the document changes over time. For example, if an element has moved, we want to be able to update the selectors to target that element in its new position. We're using child and descendant selectors.

I see that the underlying implementation has already landed in Blink. I'm happy to do the API part if it's approved.

In my opinion the tabs.removeCSS should not take an ID parameter but rather randomly generate one and return it in the callback. This way extensions won't accidentally step on each other's toes. I'm also OK with the API just accepting an ID in the call and let developers generate one that's random enough, but I think the former might be better.

Anyway I'd love to hear about the status and I'm happy to contribute with my time if it can help move things faster.

By the way we just changed how injected style sheets work: https://crrev.com/30168b

Hi m.jethani,

Sadly I cannot find time due to daywork. I'll mark as available so anyone who interested in implement it.

Regarding your opinion:
 > In my opinion the tabs.removeCSS should not take an ID parameter ...

We should not make the api remove CSS injected by other extension.

I have started work on this at crrev.com/c/877663

Now I have a few questions in my mind on how this API should work:

 1.  Should extensions be allowed to remove or overwrite CSS injected by other extensions?

    // Extension 1
    chrome.tabs.insertCSS(1, {
      code: "#header { color: silver }",
      cssId: 123
    });

    // Extension 2
    chrome.tabs.insertCSS(1, {
      code: "#header { color: orange }",
      cssId: 123
    });

    // Extension 3
    chrome.tabs.removeCSS(1, {cssId: 123});

 2.  Should extensions be required to pass in a "cssId", or should one be autogenerated for each injection and passed back to the extension, or both (optionally autogenerated)?

 3.  Should it be allowed to reuse the same "cssId" for multiple injections, and if yes then what should be the semantics?

    chrome.tabs.insertCSS(1, {
      code: "#header { color: silver }",
      cssId: 123
    });

    // What happens here?
    chrome.tabs.insertCSS(1, {
      code: "#header { font-weight: bold }",
      cssId: 123
    });

    // And what happens here?
    chrome.tabs.removeCSS(1, {cssId: 123});

Any thoughts?

I have a proposal.

tabs.insertCSS should take a new optional "name" option.

  tabs.insertCSS(1, {
    code: "#header { color: silver }",
    name: "midnight-theme"
  });

The same name can be used for multiple injections: each injection is simply a new style sheet with the same name.

  // Add some more CSS with the same name.
  tabs.insertCSS(1, {
    code: "#header { font-weight: bold }",
    name: "midnight-theme"
  });

And all the CSS injected with a given name can be removed at once.

  // Remove both of the above style sheets.
  tabs.removeCSS(1, {name: "midnight-theme"});

Other extensions can also remove or add to the CSS under a name.

  // Some other extension
  tabs.insertCSS(1, {
    code: "#header { font-size: x-large }",
    name: "midnight-theme"
  });

  // Yet some other extension
  tabs.removeCSS(1, {name: "midnight-theme"});

If an extension doesn't want other extensions messing with its own CSS, it can either namespace it (to prevent accidental overwrites) or use a random identifier (to prevent intentional overwrites).

  // Namespace.
  tabs.insertCSS(1, {
    code: "#header { font-size: x-large }",
    name: "com.example:midnight-theme"
  });

  // Use a random identifier.
  let id = Math.random().toString(36);
  tabs.insertCSS(1, {
    code: "#header { font-size: x-large }",
    name: id
  });
  // Save in a Set object for later clearing up.
  cssSet.add(id);

The usual rules for "frameId" and "allFrames" would apply, i.e. if "allFrames" is false for a call to tabs.removeCSS then only the CSS in the top frame (assuming frame 0) with the given name will be removed.

I think having a "name" option that can be reused and shared between extensions gives us more flexibility.

Any thoughts?

just a user but i think it'd be best if it was compatible with the removeCSS already implemented in firefox (minus the part about returning a promise since firefox just uses them instead of callbacks):

https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/tabs/removeCSS  
https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/tabs/insertCSS (for reference)

i don't know if it allows you to remove other extensions' css (i'd be okay if it didn't) and i haven't looked at the implementation but from what i know, you use it by passing it the same arguments as were passed to insertCSS, though the objects don't have to be referentially "the same" as the example shows (it seems like just the properties are compared for sameness)

thanks for the work on cssOrigin by the way, it looks compatible with the firefox implementation so there's no extra code needed for a cross-browser extension to use it in both browsers

I agree, compatibilty with other implementation should be the goal for initial first implementation. Additions should likely be discussed with other browsers beforhand, to ensure implementations stay reasonably compatible.

One question/suggestion I had about `insertCss` was if the implementation can provide any guarantees on the location of the inserted css in the cascade. I think most extension would like to insert their css as the last stylesheet in the specified `cssOrigin`, maybe this behaviour could be specced.

OK, I agree that the implementation should be compatible with the one in Firefox.

rdevlin.cronin@ we don't have to store the style sheet contents, we can simply store the MD5 digest.

As for how these APIs behave exactly on Firefox (tested version 57.0):

 1.  Calling tabs.insertCSS with the same code twice is an error; however, another extension can call tabs.insertCSS with exactly the same code without problems

 2.  Interestingly, tabs.removeCSS works across extensions, i.e. other extensions are allowed to remove your CSS

 3.  If exactly the same code is injected by another extension, it will be added to the end of the list; if you later call tabs.removeCSS with the same code, the other extension's copy will be deleted from the list; if you call it again with the same code, then your copy will be deleted (i.e. it searches from the end of list backwards and deletes only the first instance found)

I'm OK with implementing most of this behavior in the interest of compatibility.

I don't like that an extension is not allowed to inject the same CSS twice. I really don't see the point of the restriction, I'm going to skip that bit.

silv3rwind@ tabs.insertCSS inserts the style sheet at the end of the list within the same origin; however the list of injected author style sheets comes before the list of author style sheets from the document itself.

The order is:

 1.  UA style sheets
 2.  Injected user style sheets
 3.  Inject author style sheets
 4.  Author style sheets from the document itself

I don't know how it is in Firefox, but we cannot simply swap 3 and 4 without breaking compatibility.

I'm not sure I'm comfortable with removeCSS working across extensions.  I'd prefer these APIs be scoped to an extension's own injections.

> tabs.insertCSS inserts the style sheet at the end of the list within the same origin

That's what I wanted to hear, thanks. If the order of styles is guaranteed to be the same as the calls to `insertCss`, a extension can implement customizable ordering of styles.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9a107b936d3e7fbd9d7771e789ef3cecf9d7b333

commit 9a107b936d3e7fbd9d7771e789ef3cecf9d7b333
Author: Manish Jethani <m.jethani@eyeo.com>
Date: Fri Feb 16 10:59:13 2018

Inject and remove style sheets by key

This is the first in a series of patches to implement tabs.removeCSS

A style sheet injected with tabs.insertCSS will have an internal
autogenerated "key" that will be passed to StyleEngine. In the case of
files, this will likely be just the fully qualified name of the file,
whereas in the case of code it could be the code string itself or a hash
digest of the code string. In either case, a call to tabs.removeCSS with
the same options will deterministically generate the same internal key,
thus enabling the removal of a previously injected style sheet without
the explicit use of IDs and such.

This patch lays the groundwork in StyleEngine that will enable us to
implement behavior similar to that observed in Firefox 57.0.

Changes:

 1.  WebStyleSheetId is now WebStyleSheetKey (WebString)
 2.  WebDocument::InsertStyleSheet now accepts an optional key as a
     parameter
 3.  StyleEngine::InjectSheet and StyleEngine::RemoveInjectedSheet now
     take StyleSheetKey (AtomicString) instead of WebStyleSheetId
 4.  Both WebDocument::RemoveInsertedStyleSheet and
     StyleEngine::RemoveInjectedSheet now take the CSS origin as a
     parameter
 5.  StyleEngine::RemoveInjectedSheet now removes only the last style
     sheet injected with the given key and only from the specified
     CSS origin

BUG=608854

Change-Id: Ie62bfcb0d8309b18cbf0d2cd119c74908e396767
Reviewed-on: https://chromium-review.googlesource.com/877663
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Manish Jethani <m.jethani@eyeo.com>
Cr-Commit-Position: refs/heads/master@{#537281}
[modify] https://crrev.com/9a107b936d3e7fbd9d7771e789ef3cecf9d7b333/extensions/renderer/script_injection.cc
[modify] https://crrev.com/9a107b936d3e7fbd9d7771e789ef3cecf9d7b333/third_party/WebKit/Source/core/css/StyleEngine.cpp
[modify] https://crrev.com/9a107b936d3e7fbd9d7771e789ef3cecf9d7b333/third_party/WebKit/Source/core/css/StyleEngine.h
[modify] https://crrev.com/9a107b936d3e7fbd9d7771e789ef3cecf9d7b333/third_party/WebKit/Source/core/css/StyleEngineTest.cpp
[modify] https://crrev.com/9a107b936d3e7fbd9d7771e789ef3cecf9d7b333/third_party/WebKit/Source/core/exported/WebDocument.cpp
[modify] https://crrev.com/9a107b936d3e7fbd9d7771e789ef3cecf9d7b333/third_party/WebKit/Source/core/exported/WebDocumentTest.cpp
[modify] https://crrev.com/9a107b936d3e7fbd9d7771e789ef3cecf9d7b333/third_party/WebKit/public/web/WebDocument.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a9f3bfea51cbea0c5640a083ee4411565f33bd76

commit a9f3bfea51cbea0c5640a083ee4411565f33bd76
Author: Manish Jethani <m.jethani@eyeo.com>
Date: Mon Feb 19 14:42:47 2018

Use std::find_if to find style sheet by key

Instead of a for loop we use std::find_if with a lambda, which makes the
code easier to follow.

BUG=608854

Change-Id: Ib08efabfb8d0d21da4d3397b4fb27406fe096dc1
Reviewed-on: https://chromium-review.googlesource.com/923348
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Manish Jethani <m.jethani@eyeo.com>
Cr-Commit-Position: refs/heads/master@{#537649}
[modify] https://crrev.com/a9f3bfea51cbea0c5640a083ee4411565f33bd76/third_party/WebKit/Source/core/css/StyleEngine.cpp

Tested the issue on chrome version 49.0.2623.0 using Ubuntu 14.04 with steps mentioned below:
We have followed the steps to reproduce the issue from URL: https://issues.adblockplus.org/ticket/309#comment:10(as per comment#0)
1) Launched chrome version, Enabled the extension Adblock plus (Extension URL: https://chrome.google.com/webstore/search/adblock%20plus%201.7%206?utm_source=chrome-ntp-icon)
2) Navigated to URL: www.ptable.com and checked the "Electrons" checkbox in top right, even reloaded the site and observed the styles
3) Site looks normal and is properly styled
Observations:
Tested the issue on latest chrome version 66.0.3352.0 with the above mentioned steps by enabling and disabling the Adblock plus extension, seen the same behaviour i.e., "Site looks normal and is properly styled"

@m.jethani:
Please find the attached screen casts for chrome versions(49.0.2623.0 & 66.0.3352.0) and let us know if we missed anything in reproducing the issue. If so, could you please provide any alternate URL / Test file / reproducible steps for verifying the fix.

Thanks!

Deleted: 608854-49.0.2623.0.mp4 7.5 MB

	608854-49.0.2623.0.mp4 7.5 MB View Download

Deleted: 608854-66.0.3352.0.mp4 7.0 MB

	608854-66.0.3352.0.mp4 7.0 MB View Download

viswa.karala@ if you're testing for tabs.removeCSS support, we have not finished adding support yet, it's still a work in progress

See crrev.com/c/913628 and crrev.com/c/918606

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ff6ff85a93111903f5bf1b238c0428665f98ae55

commit ff6ff85a93111903f5bf1b238c0428665f98ae55
Author: Manish Jethani <m.jethani@eyeo.com>
Date: Fri Feb 23 07:24:55 2018

Add injection key for programmatic CSS injections

This builds on crrev.com/c/877663

For programmatic CSS injections via tabs.insertCSS, we calculate an
"injection key" deterministically based on either the file URL or the
code string. This enables us to later remove the injected CSS by the
same parameters that were used to inject it.

Changes:

 1.  ExtensionMsg_ExecuteCode_Params has a new member called
     injection_key of type base::Optional<std::string>
 2.  ScriptExecutor::ExecuteScript now calculates an injection key for
     CSS injections; this is a combination of the type (file or code),
     the host ID, and the hash digest (base::Hash) of either the file
     URL, if available, or the the code string
 3.  ScriptInjector has a new method called GetInjectionKey;
     UserScriptInjector always returns null, whereas
     ProgrammaticScriptInjector returns the value passed to it via
     ExtensionMsg_ExecuteCode_Params
 4.  ScriptInjection::InjectCss now passes the actual injection key to
     WebDocument::InsertStyleSheet

BUG=608854

Change-Id: I91a3983682cc61d739d3c31a11e8f58dabb8e12c
Reviewed-on: https://chromium-review.googlesource.com/913628
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Manish Jethani <m.jethani@eyeo.com>
Cr-Commit-Position: refs/heads/master@{#538722}
[modify] https://crrev.com/ff6ff85a93111903f5bf1b238c0428665f98ae55/extensions/browser/script_executor.cc
[modify] https://crrev.com/ff6ff85a93111903f5bf1b238c0428665f98ae55/extensions/common/extension_messages.h
[modify] https://crrev.com/ff6ff85a93111903f5bf1b238c0428665f98ae55/extensions/renderer/programmatic_script_injector.cc
[modify] https://crrev.com/ff6ff85a93111903f5bf1b238c0428665f98ae55/extensions/renderer/programmatic_script_injector.h
[modify] https://crrev.com/ff6ff85a93111903f5bf1b238c0428665f98ae55/extensions/renderer/script_injection.cc
[modify] https://crrev.com/ff6ff85a93111903f5bf1b238c0428665f98ae55/extensions/renderer/script_injector.h
[modify] https://crrev.com/ff6ff85a93111903f5bf1b238c0428665f98ae55/extensions/renderer/user_script_injector.cc
[modify] https://crrev.com/ff6ff85a93111903f5bf1b238c0428665f98ae55/extensions/renderer/user_script_injector.h

uBlock Origin has decided not to use "cssOrigin: 'user'" because of lack of tabs.removeCSS [1]. I believe Stylus was also waiting for tabs.removeCSS at one point [2]. Adblock Plus does not use user style sheets for certain ad blocking filters again because there's no tabs.removeCSS.

There is one last patch waiting for review. Is there anything I can do to help expedite this?

[1]: https://github.com/gorhill/uBlock/issues/3588#issuecomment-380812328
[2]: https://github.com/openstyles/stylus/issues/248#issuecomment-345522529

The lack of tabs.removeCSS is causing issues like https://issues.adblockplus.org/ticket/6100 in Adblock Plus.

Since we can't use a style sheet to reliably hide an element in the document, we must resort to setting the style attribute to "display: none !important". In some cases the document decides to fight back by resetting the style attribute, causing a race condition and making the page unresponsive.

rdevlin.cronin@ any ETA on the patch?

No updates for the past 6 months, is this still planned to be merged at some point? User stylesheets in extensions have a significant disadvantage here in comparison to Firefox's implementation.

How long will the patch remain stuck in review ? Currently uBlock Origin users cannot see the removed cosmetic filters in real time and it breaks the DOM Inspector which is a part of uBO functionality.