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

Add support for preload scripts #292

Merged
merged 6 commits into from Dec 22, 2022
Merged

Add support for preload scripts #292

merged 6 commits into from Dec 22, 2022

Conversation

jgraham
Copy link
Member

@jgraham jgraham commented Sep 20, 2022

These are scripts which are run when a realm (initially only for realms with a Window global) is created. They allow a test harness to inject functionality that's guaranteed to be available for any content scripts that are subsequently loaded, and before any later scripts that WebDriver injects into the context.

Registering a preload script happens with the script.addPreloadScript command. This returns a handle to the added script.

Load scripts can later be removed by passing the handle to script.removePreloadScript.

Load scripts can either be run directly in the newly created Window, or in a named sandbox.


Preview | Diff

@jgraham
Copy link
Member Author

jgraham commented Sep 20, 2022

This is a very initial cut at #65

I changed the name from "bootstrap scripts" to "load scripts" because I thought "bootstrap" might be a bit obscure, but maybe "load" is suggestive of the wrong point in the lifecycle.

Compared to the original propsal, this only supports window-type globals and doesn't have the URL pattern matching. URL pattern matching can be implemented in the script itself (with some care; navigating from the initial about:bank reuses the same Window). This featureset currently matches what's in CDP which suggests it's enough to enable existing client features.

@jgraham
Copy link
Member Author

jgraham commented Sep 20, 2022

whatwg/html#8300 for the HTML side of this change.

Copy link
Contributor

@whimboo whimboo left a comment

Choose a reason for hiding this comment

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

Thanks James! As I can see the PR only contains the details for the two methods but not the trigger for run WebDriver BiDi load scripts. Do I miss something or did you forget to add that to the browsing context / realm created sections?

Also I agree that we should have a discussion for a proper name.

index.bs Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
Copy link
Member Author

@jgraham jgraham left a comment

Choose a reason for hiding this comment

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

As I can see the PR only contains the details for the two methods but not the trigger for run WebDriver BiDi load scripts

That happens in the linked HTML change. Since there's no event here there's no ordering question between contextCreated and running the scripts (although maybe things will be more complex once we have custom events).

index.bs Outdated Show resolved Hide resolved
@css-meeting-bot
Copy link
Member

The Browser Testing and Tools Working Group just discussed load scripts.

The full IRC log of that discussion <jgraham> Topic: load scripts
<jgraham> github: https://github.com//pull/292
<sadym> q+
<jgraham> jgraham: Mader a PR for load scripts (aka bootstrap scripts) following TPAC. Just addLoadScript/removeLoadScript, most of the implementation complexity is calling the scripts at the right time
<jgraham> ack sadym
<jgraham> sadym: When you have several load scripts for the same realm, which order do they run in?
<whimboo> q+
<jgraham> jgraham: Yes I think so.
<jgraham> whimboo: so there's no way to add at the beginning without removing all existing scripts?
<jgraham> jgraham: That's correct. I don't think other protocols allow you to adjust the order. It's not clear how that would work in the face of multiple sessions all trying to add different load scripts (or even multiple components using the same session)
<jgraham> ack whimboo
<jgraham> whimboo: Could we add a priority field? Then the client could specify the order within a given session.
<sadym> q+
<jgraham> jgraham: Would be good to know what use cases require that; maybe add them to the issue
<jgraham> sadym: If we guarantee that we run scripts before client scripts that should be enough.
<jgraham> RRSAgent: make minutes
<RRSAgent> I have made the request to generate https://www.w3.org/2022/10/12-webdriver-minutes.html jgraham
<jgraham> Zakim, bye
<Zakim> leaving. As of this point the attendees have been foolip, sadym, jgraham, whimboo, cb, PujaJagani, JimEvans
<jgraham> RRSAgent: bye
<RRSAgent> I see no action items

@jimevans
Copy link
Collaborator

jimevans commented Dec 7, 2022

@jgraham Nitpicky comment: If we think "load script" might be confusing because of the cognitive issue surrounding when "load" occurs, and "bootstrap" being obscure, is there any mileage in the term "preload scripts?"

@jgraham
Copy link
Member Author

jgraham commented Dec 7, 2022

I like "preload scripts". Unless people prefer "bootstrap scripts" I'll change to that for the next revision.

jgraham and others added 5 commits December 21, 2022 21:07
These are scripts which are run when a realm (initially only for
realms with a Window global) is created. They allow a test harness to
inject functionality that's guaranteed to be available for any content
scripts that are subsequently loaded, and before any later scripts
that WebDriver injects into the context.

Registering a load script happens with the script.addLoadScript
command. This returns a handle to the added script.

Load scripts can later be removed by passing the handle to
script.removeLoadScript.

Load scripts can either be run directly in the newly created Window,
or in a named sandbox.
Co-authored-by: Henrik Skupin <mail@hskupin.info>
@jgraham jgraham changed the title Add support for "load scripts" Add support for preload scripts Dec 21, 2022
Copy link
Contributor

@sadym-chromium sadym-chromium left a comment

Choose a reason for hiding this comment

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

I like the name "PreloadScript".

index.bs Show resolved Hide resolved
@jgraham
Copy link
Member Author

jgraham commented Dec 22, 2022

I'm going to merge this on the assumption that the HTML PR will merge without requiring substantive changes on the WebDriver side.

@jgraham jgraham merged commit d73975c into master Dec 22, 2022
@jgraham jgraham deleted the load_scripts branch December 22, 2022 13:00
github-actions bot added a commit that referenced this pull request Dec 22, 2022
SHA: d73975c
Reason: push, by jgraham

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
whimboo added a commit to whimboo/webdriver-bidi that referenced this pull request Feb 3, 2023
* Add support for "preload scripts"

These are scripts which are run when a realm (initially only for
realms with a Window global) is created. They allow a test harness to
inject functionality that's guaranteed to be available for any content
scripts that are subsequently loaded, and before any later scripts
that WebDriver injects into the context.

Registering a load script happens with the script.addPreloadScript
command. This returns a handle to the added script.

Load scripts can later be removed by passing the handle to
script.removePreloadScript.

Preload scripts can either be run directly in the newly created Window,
or in a named sandbox.

Co-authored-by: Henrik Skupin <mail@hskupin.info>
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.

None yet

5 participants