Content script event handlers overwriting page's event handlers
Categories
(WebExtensions :: General, defect, P5)
Tracking
(firefox138 affected, firefox139 affected, firefox140 affected)
People
(Reporter: tom_xyz, Unassigned)
References
Details
(Keywords: dev-doc-complete)
Attachments
(1 file)
|
985 bytes,
application/zip
|
Details |
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:138.0) Gecko/20100101 Firefox/138.0
Steps to reproduce:
- Install _minimal_mv3-cs.zip in Firefox.
- Install _minimal_mv3-cs.zip in Chrome.
- Grant local file permission for both browsers.
- Open test.html in both browsers.
- Type something while monitoring the devtools console.
Actual results:
In Chrome, the onkeydown event handler does NOT overwrite the page's event handler because it lives in the ISOLATED world. However, in Firefox the event handler DOES overwrite the page's event handler.
Expected results:
Firefox should be consistent with Chrome and not overwrite the page's event handler. This is the expected behavior for ISOLATED world content scripts, unless Mozilla has a different interpretation than Google, which then makes for an inconsistent developer experience.
|
||
Updated•1 year ago
|
|
||
Updated•1 year ago
|
|
||
Comment 1•1 year ago
|
||
Is this just the same as https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Chrome_incompatibilities#content_script_environment?
|
||
Comment 2•1 year ago
|
||
Hello,
I reproduced the issue on the latest Nightly (140.0a1/20250514211700), Beta (139.0b8/20250514034321) and Release (138.0.3/20250512124033) under Windows 11.
Compared to Chrome, where the console logs a WEB PAGE keydown and a CONTENT SCRIPT keydown event with each key press, on Firefox only a CONTENT SCRIPT keydown event is logged to console for the same action, as the content script event handler overwrites the page event handler.
|
||
Updated•1 year ago
|
|
||
Updated•1 year ago
|
|
|
||
Comment 4•1 year ago
|
||
Firefox's behavior is consistent, and I have no reason to change it:
onkeydown is part of the document interface, as are the title and other properties. A content script has access to those properties both via setters and getters. Using a setter, we overwrite the previous value of the selected property. Imagine this scenario:
- the page does
document.title = "foo" - the content script does
document.title = "bar"
What's the final document.title value? Answer: "bar". There are no two titles. Why should we have two "onkeydown" event handlers?
This seems to be a bug in the Chrome implementation.
The content script can still add a second event handler using addEventHandler.
Implementation-wise, if we want to follow the Chrome implementation, we can still apply a small change in our logic. But to me, this would be confusing because it means that the content script would not have access to the original event handler set by the page.
|
||
Comment 5•1 year ago
|
||
Firefox and Chrome both do something different, and I understand the reasons behind both. IMO, the biggest motivation for keeping Firefox's behavior is backwards-compatibility, not because it is a great design. Chrome's current design makes more sense from me given the design of isolated worlds and the expectations of web/extension developers. The expectation when a web developer or extension developer does element.onclick = xxx is to register an event listener, and not necessarily to replace the event listener of the web page or (other) extensions' content scripts (which is what Firefox does right now).
The specification of EventTarget defines event handers, including an event handler map. Besides the obvious (setting and retrieving the assigned function), the get the current value of the event handler algorithm also includes initialization of the default event handler from inline event attributes (<body onclick="alert(0)"> -> document.body.onclick will hold function() {alert(0)}, but not the other way around!).
Now clearly the spec was not written with extension content scripts in mind. By design, content scripts have an execution environment that is distinct from the web page, but shares the same DOM.
In Chrome, isolation of execution environments is implemented as an isolated world, which shares DOM object but with separate wrappers around it. On event registration:
- Chrome maintains separate event handlers per world; It did not do so before, but that lead to a security bug:
- Security: Webpage can gain access to extension content-script variables when content-script triggers events
- Developers complained after the change was made, but that was marked WontFix: unsafeWindow workaround for extensions does not work in latest Chrome canary, but works in dev-m
- Here is an example of the
unsafeWindowhack that used to work in Chrome but does not work any more, but still works today in Firefox: https://gist.github.com/mathiasbynens/1143845
- Event handlers are defined with the
DEFINE_ATTRIBUTE_EVENT_LISTENERmacro fromevent_target.h - The implementation of the event listener property getter/setter is aware of the world when it looks up the function reference.
- The implementation basically stores the listener functions in an internal map (keyed by world), and uses
addEventListener&removeEventListenerto hook up the listener to the event handling backend. The event properties (onkeydown,onclick, etc.) are lookups that look into that internal map.
In Firefox, isolation is provided through XrayWrappers (Xray Vision). When a content script calls document.body.onclick = function() { /*...*/ }, the internals will call the native implementation with the received function. And it seems that Firefox's implementation does not special case different contexts (more on that later). On event registration:
- Event handlers are defined with the
EVENTmacro in subclasses ofEventTarget- E.g. nsINode defines
EVENTthat simply callsEventTarget::SetEventHandler, which in its turn callsEventListenerManager::SetEventHandlerthat is common to theEVENTmacro implementations.
- E.g. nsINode defines
EventListenerManager::SetEventHandlerInternalhandles the registration, but the event handler map is only keyed by the event name, without world-specific handlers.- If we want to change this to match Chrome, then we would have to make the event handler getter/setter aware of the execution context that it is being called from, and perhaps differentiate the map if
CompartmentPrivate::isWebExtensionContentScriptis set (or a new flag).
In Firefox, there are defined mechanisms to interact with objects across worlds, controlled through XrayWrappers, where typically a higher-privileged script can access functionality in a lower-privileged script (a content script can reach into the web page's context with .wrappedJSObject, for example). The other way around is also possible, e.g. when a content script calls exportFunction on a function and exposes it to the web page. If a higher-privileged value is (unintentionally) exposed to a lower-privileged context, XrayWrappers protect the higher-privileged value by throwing errors when a lower-privileged context tries to access such a value.
- In this bug specifically, we see that in action: our implementation does not specifically account for event handlers from content scripts vs the page it is running on, and consequently changes to e.g.
document.body.onkeydownare mirrored between the two worlds. If the web page sets a function, the content script can directly call it (in Chrome, one can only call it indirectly viadispatchEvent()). But if the content script sets the listener, the web page will be denied access when it tries to call it ("Permission denied to access object"). - This mechanism is also why this
unsafeWindowhack that I linked before works in Firefox: https://gist.github.com/mathiasbynens/1143845
|
||
Comment 6•11 months ago
|
||
The severity field is not set for this bug.
:robwu, could you have a look please?
For more information, please visit BugBot documentation.
|
|
||
Comment 7•11 months ago
|
||
We should document this difference at https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Chrome_incompatibilities#content_script_environment (and link to this bug). Comment 5 explains the differences between Chrome and Firefox, and explains the historical reasons for the difference.
@robwu thank you for the detailed explanation. Even though I do not understand all of the technicalities behind the design decision, as a developer who is maintaining both Firefox and Chrome extensions using the same code base, it has been a pain point for us. We ended up spending a lot of time to refactor everything to use a combination of removeEventListener() and addEventListener() to work around this issue.
It would still be nice for Firefox behavior to be consistent with Chrome so that the DX is more predictable. 🤞
Description
•