Opened 3 years ago

Last modified 2 years ago

#5090 closed change

Switch to chrome.tabs.insertCSS() on Firefox — at Version 6

Reported by: trev Assignee:
Priority: P2 Milestone: Adblock-Plus-1.13.4-for-Chrome-Opera
Module: Platform Keywords:
Cc: sebastian, greiner, Ross Blocked By:
Blocking: Platform: Firefox
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29410607/

Description (last modified by sebastian)

Background

As of Firefox 53, chrome.tabs.insertCSS implementation on Firefox accepts a cssOrigin parameter which makes it usable for us. Unlike with Chrome, on Firefox we can now inject CSS code from the background process directly without messing with the DOM in the content script.

What to change

When a content script sends a get-selectors message , try injecting the stylesheet from the backround page using tabs.insertCSS({..., cssOrigin: "user}). If it fails because cssOrigin isn't supported (i.e. on Firefox <53 and every other browser currently) remember that condition and don't try again.

If the stylesheet was injected by insertCSS() and no devtools panel is active for the same tab, send following response:

{
  inject: false,
  trace: false
}

If the stylesheet was injected by insertCSS() and there is an active devtools panel for the same tab, send following response:

{
  inject: false,
  trace: true,
  selectors: [...]
}

If insertCSS() doesn't support cssOrigin, set inject to true and always include the selectors, regardless whether the devtools panel is active:

{
  inject: true,
  trace: ...,
  selectors: [...]
}

Adapt the content script, in order to handle the inject key, so no stlesheet is injected when it is false.

Change History (6)

comment:1 Changed 3 years ago by sebastian

In order to keep the devtools panel functional, we still have to return the selectors if there is an active devtools panel for this tab, but merely not inject them. Even if the devtools panel isn't supported on Firefox, Chrome is about to add support for user stylsheets to insertCSS() as well, and then we will use the same logic there (see #242). So I prefer to get this right from the start.

Currently, when there is an active devtools panel, get-selectors is responding like this:

{
  "trace": true,
  "selectors": [...]
}

So I suggest to add another key to indicate whether the selectors should be injected (regardless whether they are traced):

{
  "inject": false,
  "trace": true,
  "selectors": [...]
}

If the selectors neither need to be traced (i.e. if there is no active devtools panel) nor have to be injected by the content script (i.e. when insertCSS() is used), we can omit the the selectors from the response:

{
  "inject": false,
  "trace": false
}

comment:2 Changed 3 years ago by trev

  • Cc sebastian added

Fine with me for now, feel free to adjust the description accordingly.

comment:3 Changed 3 years ago by sebastian

I will, but I have one more question. Is there also a way to detect whether insertCSS() supports user stylesheets, or do we have to check the user agent for Firefox >=53?

comment:4 Changed 3 years ago by trev

I verified that calling chrome.tabs.insertCSS({code: "", dummy: "user}) will fail in both Firefox and Chrome - unknown parameters are being rejected. So I guess we could try calling chrome.tabs.insertCSS({..., cssOrigin: "user}) first and fall back to injecting selectors in the content script if it fails. It should also be possible to remember the result so that we don't keep trying - errors not related to parameter validation (such as missing permissions) are typically async.

comment:5 Changed 3 years ago by greiner

  • Cc greiner added

comment:6 Changed 3 years ago by sebastian

  • Description modified (diff)
  • Priority changed from Unknown to P2
  • Ready set

I guess, that should do for now. (We might have to revisit this though, once Chrome catches up, as they plan to use user stylesheet, without additional parameter, no matter what.)

Note: See TracTickets for help on using tickets.