Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: dom.execute() #678

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rdcronin
Copy link
Collaborator

This adds a proposal for a new dom.execute() method, which will allow synchronous execution in another "world" from a content script.

This adds a proposal for a new dom.execute() method, which will allow
synchronous execution in another "world" from a content script.
Copy link
Contributor

@carlosjeurissen carlosjeurissen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rdcronin @Rob--W
Thanks for putting this together and continue where we left off during the San Diego meeting!

proposals/dom_execute.md Outdated Show resolved Hide resolved
proposals/dom_execute.md Show resolved Hide resolved
This API is a necessary first step before introducing inter-world communication
channels. We plan to do that alongside the implementation for this API.

### Specifying a target world
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which means not specifying the target world in the future would default to the main world. If that is a downside or if explicitly specifying a target world right from the start is preferred specifying the target world right now world be good.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct. I think defaulting to the main world makes sense. However, I don't feel strongly here, so if folks would prefer to have a target world specified in all cases, I'm okay with that. @Rob--W or others, any preference?

proposals/dom_execute.md Outdated Show resolved Hide resolved
proposals/dom_execute.md Show resolved Hide resolved
@@ -0,0 +1,174 @@
# Proposal: browser.dom.execute()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the motivation to use the dom namespace instead of the scripting namespace? Considering it's similarities with scripting.executeScript and the seemingly lack of connection with the DOM. Considering execute(), createPort() and also openOrClosedShadowRoot(), would it not make sense to use browser.document instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the separation between browser.dom and browser.scripting is valuable -- even though they have similar surfaces and serve conceptually-similar purposes, their use cases, restrictions, and implementations are quite different. One is used from the extension's primary contexts (e.g., background script) to configure scripts that will be running in completely different contexts (e.g., tabs), and the other is used to interact with the same document the extension is currently running in. IMO, this distinction more easily separates these APIs from developers, and also allows a cleaner isolation of APIs for browsers: scripting.execute() is not allowed in isolated worlds at all, since it is a privileged API. By contrast, this is, and it is (kinda) just adding a script to the DOM (though it's closer to eval than adding a child element), and so is available in these (relatively) untrusted contexts.

Considering execute(), createPort() and also openOrClosedShadowRoot(), would it not make sense to use browser.document instead?

I don't have strong preferences for browser.dom vs browser.document except that we already have browser.dom : ) It doesn't seem worth the churn to migrate to browser.document at this stage to me, given the impact on the ecosystem and the delay it would entail to introduce new APIs (since I do think we'd then want to migrate fully away from dom before moving to document to avoid just having two APIs to maintain).

proposals/dom_execute.md Show resolved Hide resolved
proposals/dom_execute.md Show resolved Hide resolved
proposals/dom_execute.md Show resolved Hide resolved
proposals/dom_execute.md Show resolved Hide resolved

Arguments are serialized and deserialized using the Structured Cloning
algorithm. This allows for more flexibility than simply JSON conversion. In
the future, some arguments may have custom serialization.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If dom.execute doesn't support DOM nodes, it will cause problems for extension developers because dealing with DOM nodes is what content scripts usually do and chrome.dom namespace strongly implies that DOM is supported.

To be precise, it should be EventTarget, the ancestor of Node, which additionally allows transferring things like iframeElem.contentWindow, window.parent or event.source from a postMessage listener.

Although technically this is already possible by sending a MouseEvent with relatedTarget set to the EventTarget instance, but this trick is largely unknown. World isolation is maintained automatically: the event keeps only the spec'd part of the object i.e. without this JS world's expando properties.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants