w3c / webextensions

Charter and administrivia for the WebExtensions Community Group (WECG)
Other
599 stars 56 forks source link

Event listeners in Content Scripts and communication between main and isolated CS worlds #241

Open bershanskiy opened 2 years ago

bershanskiy commented 2 years ago

Background

Currently, extension content scripts add event listeners on DOM nodes for two purposes:

These are two fundamentally different use cases. Developers are probably already performing some validation on messages sent via the second method. However, I observed (anecdotal evidence) that some developers inject extension UIs into pages and do not realize that page can dispatch events automatically without user interaction. Lack of validation is, of course, mostly a failure on extension developers' side, but I believe user agents can also fix it.

Proposal

Proposal (breaking change): main-world JS should not be able to trigger event listeners added in isolated world. This will break the second use case, however developers can switch to "official" messaging methods like runtime.sendMessage and runtime.onMessage and make extensions "externally connectable".

I'm sure this proposal is possible to implement because Developer Tools already are able to distinguish between event listeners added in two worlds and display only main-world ones (if I recall correctly, I was able to see these event listeners before).

bershanskiy commented 2 years ago

Here is a test extension which demonstrates the first use case and reacts to events created by the page itself:

manifest.json

{
  "name": "Test",
  "version": "1.0",
  "manifest_version": 3,
  "background": {
    "service_worker": "background.js"
  },
  "action": {},
  "content_scripts": [
    {
      "matches": [
        "<all_urls>"
      ],
      "js": [
        "cs.js"
      ],
      "run_at": "document_end",
      "match_about_blank": true
    }
  ]
}

background.js:

chrome.runtime.onMessage.addListener(console.log);

cs.js

function listener(message) {
  return () => chrome.runtime.sendMessage(`click ${message}`);
}

document.addEventListener('click', listener('regular'));

const button = document.createElement('button');
button.innerText = 'HELLO';
button.addEventListener('click', listener('button'));
document.body.appendChild(button);
theseanl commented 2 years ago

This will break the second use case, however developers can switch to "official" messaging methods like runtime.sendMessage and runtime.onMessage and make extensions "externally connectable".

Just want to add that this can't address all use cases, because adding a domain as externally connectable allows any scripts in the domain to communicate with the extension, but extensions mostly wants to only communicate to first-party scripts.

bershanskiy commented 2 years ago

Just want to add that this can't address all use cases, because adding a domain as externally connectable allows any scripts in the domain to communicate with the extension, but extensions mostly wants to only communicate to first-party scripts.

This is a valid point. Also, extensions currently can not specify very generic patterns in externally connectable patterns. Also, in MV3 extensions can change content scripts (register/unregister them) while externally connectable patterns are static (specified only in manifest.

Rob--W commented 2 years ago

event.isTrusted can already be used if developers really want to distinguish user-initiated events from "artificially" synthesized events. But even with event.isTrusted, the extension cannot know for sure that the user intentionally triggered the event for the expected reason. Hostile pages could change the appearance of the UI element (e.g. button) to mislead the user.

Extensions that interface through web pages should be designed to work well with a hostile web page. Critical extension UI should not share the same DOM as the web page.

On the topic of communication between the web page and content scripts, several discussions have happened before in the WECG: see #57, #77, #78, #79 and the meeting notes linked to these issues (and also today's meeting notes linked to this issue).

This issue prescribed a solution that is a backwards-incompatible change, without compelling reasons to support such a big change. The mentioned scenario's are incomplete: besides the two use cases (DOM events and extension communication), extensions may be interested in subscribing to custom events from the web page itself (e.g. I have written extensions that subscribes to custom events on YouTube to detect website-specific state transitions).

tophf commented 2 years ago

This proposal will also break Tampermonkey and all the other userscript engines, which use CustomEvent messages for synchronous communication, unachievable via chrome.runtime messaging. They also use MouseEvent to pass DOM nodes, which is otherwise impossible between worlds. On the other hand, it was a bad solution anyway, and a better one must be implemented, also synchronous, of course.