Clarify behavior of userScripts API #739
Conversation
There was a problem hiding this comment.
@EmiliaPaz I've added links to Chromium bugs below in cases where Chrome's behavior does not match the specified behavior yet.
| */ | ||
| js: ScriptSource[]; | ||
| js?: ScriptSource[]; |
There was a problem hiding this comment.
There is consensus among the browser vendors that this should be optional. Devlin also confirmed that on behalf of Google.
I filed an issue for Chromium to change that, at https://issues.chromium.org/issues/383712209
| dictionary RegisteredUserScript { | ||
| boolean? allFrames; | ||
| ScriptSource[] js; | ||
| // js is required in userScripts.register(), optional in userScripts.update(). | ||
| // When specified, must be a non-empty array. |
There was a problem hiding this comment.
Side note: Chrome currently requires js to be a non-empty array, Firefox currently accepts an empty array.
| @@ -157,11 +162,54 @@ As mentioned in requirement A, the user script world can communicate with differ | |||
| - Scripts registered via [`scripting.registerContentScripts()`](https://developer.chrome.com/docs/extensions/reference/scripting/#method-registerContentScripts), following the order they were registered in. Updating a content script doesn't change its registration order. | |||
| - Scripts registered via `userScripts.register()`, following the order they were registered in. Updating a user script doesn’t change its registration order. | |||
| - User scripts are always persisted across sessions, since the opposite behavior would be uncommon. (We may explore providing an option to customize this in the future.) | |||
| - Unlike regular content scripts, `matches` is allowed to be optional when `includeGlobs` is specified. A user script matches a document when its URL matches either `matches` or `includeGlobs`. | |||
There was a problem hiding this comment.
Chrome has currently not implemented this, and this is tracked at https://issues.chromium.org/issues/41483539
| The following fields are required for a valid `RegisteredUserScript`: | ||
|
|
||
| - `js` must be a non-empty array. | ||
| - At least one of `matches` or `includeGlobs` must be a non-empty array. |
There was a problem hiding this comment.
As mentioned before, Chrome currently requires matches, but it should not be. When optional, "obviously" one of the two arrays should be non-empty, as mentioned before at https://issues.chromium.org/issues/41483539#comment16
There was a problem hiding this comment.
Agree that matches should be optional, and with the other comments. However, it's not as clear when reading it on the updated proposal. "The following fields are required for a valid RegisteredUserScript" are only applicable when registering the script, right? Because updating a script doesn't require to provide at least one of matches or includeGlobs, or js.
What about:
The `RegisteredUserScript` type is shared by `userScripts.register()` and
`userScripts.update()`. All fields except `id` are declared as optional, to
allow `userScripts.update()` to update individual properties.
#### Requirements per method:
`userScripts.register()`:
- `js` must be present and a non-empty array.
- At least one of `matches` or `includeGlobs` must be a non-empty array.
`userScripts.update()`
- individual properties may be `null` or omitted to leave the value
unchanged.
- To clear an array, an empty array can be passed.
- The resulting script must be validated to make sure that the updated
script remains a valid script before it replaces a previous script.
#### Example
...
There was a problem hiding this comment.
I have updated the proposal based on your suggestion. This is the difference:
diff --git a/proposals/user-scripts-api.md b/proposals/user-scripts-api.md
index 4dcb1b4..b584136 100644
--- a/proposals/user-scripts-api.md
+++ b/proposals/user-scripts-api.md
@@ -170,19 +170,21 @@ The `RegisteredUserScript` type is shared by `userScripts.register()` and
`userScripts.update()`. All fields except `id` are declared as optional, to
allow `userScripts.update()` to update individual properties.
-The following fields are required for a valid `RegisteredUserScript`:
+#### Requirements per method
-- `js` must be a non-empty array.
+`userScripts.register()`:
+
+- `js` must be present and a non-empty array.
- At least one of `matches` or `includeGlobs` must be a non-empty array.
-The above requirements must be checked when `userScripts.register()` is called.
+`userScripts.update()`:
-When `userScripts.update()` is invoked, individual properties may be `null` or
-omitted to leave the value unchanged. To clear an array, an empty array can be
-passed. The resulting script must be validated to make sure that the updated
-script remains a valid script before it replaces a previous script.
+- Individual properties may be `null` or omitted to leave the value unchanged.
+- To clear an array, an empty array can be passed.
+- The resulting script must be validated to make sure that the updated
+ script remains a valid script before it replaces a previous script.
-Consider the following example:
+#### Example
```javascript
// Valid registration:
proposals/user-scripts-api.md
Outdated
|
|
||
| The above requirements must be checked when `userScripts.register()` is called. | ||
|
|
||
| When `userScripts.update()` is invoked, individual properties may be `null` or |
There was a problem hiding this comment.
Chrome currently resets properties when individual properties are missing, but there is consensus that it should not be changed. For details, see #565 (comment)
... and this is tracked at https://issues.chromium.org/issues/40938749
There was a problem hiding this comment.
Thanks for the updates, Rob! They make sense, just a small comment on how to better phrase one of them.
From Chrome's side, there are 3 main things missing:
userScripts.register()should require either matches or includeGlobs (crbug.com/41483539)userScripts.update()should not reset properties when individual properties are missing. e.g reset the user script world. (crbug.com/40938749)jsin RegisteredUserScript should be optional, only enforcing to be required foruserScripts.register(). (crbug.com/383712209)
And userScripts.execute(), which is still missing implementation
Rob--W
force-pushed
the
userScripts-api-clarifications
branch
from
16f55f0 to
f02f0dc
Compare
- Use empty string instead of unspecified/null as the default worldId, following #565 (comment) - Declare "js" property as optional with remark that it is required in "register", to enable `userScripts.update()` without requiring "js". - Expand on RegisteredUserScript validation, notably on validating matches+includeGlobs after an update. - Update `resetWorldConfiguration()` signature to match Chrome's and Firefox's actual implementation: `worldId` is optional. - Create a new section "World Configurations" and specify world configuration behavior more precisely. In particular, clarify fallback behavior, following #565 (comment) - Mention Firefox's optional-only "userScripts" permission design. - Add `worldId` to userScripts.execute proposal.
Rob--W
force-pushed
the
userScripts-api-clarifications
branch
from
f02f0dc to
04b7ab3
Compare
| // A specific user script world ID to execute in. Only valid if `world` is | ||
| // omitted or is `USER_SCRIPT`. If `worldId` is omitted, the default value is | ||
| // an empty string ("") and the script will execute in the default world. | ||
| worldId?: string, |
There was a problem hiding this comment.
@EmiliaPaz I noticed that userScripts.execute was missing the worldId property, so I decided to include it in the update here.
This PR updates the existing userScripts API proposals to clarify some behavior.
Use empty string instead of unspecified/null as the default worldId, following Support multiple user script worlds in the user scripts API #565 (comment)
Declare "js" property as optional with remark that it is required in "register", to enable
userScripts.update()without requiring "js".Expand on RegisteredUserScript validation, notably on validating matches+includeGlobs after an update.
Update
resetWorldConfiguration()signature to match Chrome's and Firefox's actual implementation:worldIdis optional.Create a new section "World Configurations" and specify world configuration behavior more precisely. In particular, clarify fallback behavior, following Support multiple user script worlds in the user scripts API #565 (comment) (in relation to https://issues.chromium.org/issues/40938749).
Mention Firefox's optional-only "userScripts" permission design.