Skip to content

Content script load order clarification #39298

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

Merged
merged 6 commits into from
May 21, 2025

Conversation

rebloor
Copy link
Contributor

@rebloor rebloor commented Apr 28, 2025

Description

As a follow-up to Bug-1792685 Content scripts and styles guaranteed execution order release note #39245, add details of the load order of items specified in the content_scripts manifest key.

@rebloor rebloor added the Content:WebExt WebExtensions docs label Apr 28, 2025
@rebloor rebloor requested review from dotproto and Rob--W April 28, 2025 03:01
@rebloor rebloor self-assigned this Apr 28, 2025
@rebloor rebloor requested a review from a team as a code owner April 28, 2025 03:01
@github-actions github-actions bot added the size/m [PR only] 51-500 LoC changed label Apr 28, 2025
Copy link
Contributor

github-actions bot commented Apr 28, 2025

Preview URLs

(comment last updated: 2025-05-16 04:28:36)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Collaborator

@dotproto dotproto left a comment

Choose a reason for hiding this comment

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

Suggested a couple of small changes in this PR and a couple of bigger ones that we can tackle either here or in a follow-on PR. I'm good with either approach.

@@ -324,6 +306,46 @@ Details of all the keys you can include are given in the table below.
</tbody>
</table>

## Load order

When a webpage load loads, matching key objects are processed in this order:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
When a webpage load loads, matching key objects are processed in this order:
When a webpage loads, matching key objects are processed in this order:


When a webpage load loads, matching key objects are processed in this order:

- in accordance with the `run_at` directive (`"document_start"` then `"document_end"` then `"document_idle"`). For each object with the same directive:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we link to run_at rather than include possible values here? Having it inline is both more info for the reader to parse and more content to maintain should the potential values change.

@@ -324,6 +306,46 @@ Details of all the keys you can include are given in the table below.
</tbody>
</table>

## Load order
Copy link
Collaborator

Choose a reason for hiding this comment

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

As a reader, I find the current framing a little confusing. The first sentence in this section starts by talking about the order in which "matching key objects are processed," but it's not immediately clear to me why that matters.

It feels like the current copy is too focused on the implementation details of parsing an array of objects rather than the thing I want to understand as a developer: what (if any) guarantees are there about the order in which my content scripts will be added to (and executed on) a web page.

For the moment, I think just adding an intro statement would help clarify this content. For example:

Files declared in content scripts are injected into web pages in a well defined order. When a webpage loads, matching key objects are processed in this order:

Longer term, I think we should consider moving this to the content scripts page and building it out to cover both the static content scripts definitions from this page and the dynamic content scripts declrations from the runtime API.

Comment on lines 341 to 347
The files are loaded like this when a mozilla.org domain opens:

- `"jquery.js"` - because it's requested to run at `"document_start"`.
- `"my-content-script.js"` - because it's in the first array requesting run at `"document_idle"`.
- `"my-css.css"` - because an object's CSS is loaded before its JavaScript.
- `"another-content-script.js"` - because it's the first item in the `js` property.
- `"yet-another-content-script.js"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

I really like helping concertize this information with a grounded example. I think we can improve on this explanation, though, by adding notes about things like when the website's own scripts run, when DOM events occur (e.g. DOMContentLoaded), and referencing which content script object is being processed at a given moment.

@rebloor rebloor requested a review from dotproto April 29, 2025 04:13
@rebloor
Copy link
Contributor Author

rebloor commented Apr 29, 2025

@dotproto I've addressed your specific feedback. However, given that this was to scratch a particular documentation itch, for wider-ranging comments, would you consider raising an issue?

@@ -324,6 +306,46 @@ Details of all the keys you can include are given in the table below.
</tbody>
</table>

## Load order

Files declared in `content_scripts` are injected into web pages in a defined order. When a webpage loads, matching key objects are processed in this order:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested edit to address my feedback at https://github.com/mdn/content/pull/39298/files#r2075072868

Suggested change
Files declared in `content_scripts` are injected into web pages in a defined order. When a webpage loads, matching key objects are processed in this order:
Files declared in `content_scripts` are injected into web pages in a defined order: At the time specified by `run_at`, in the order as given in the `css` / `js` arrays.
When a webpage loads, matching key objects are processed in this order:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Rob--W I'm unclear how this could help the reader: they are now presented with two slightly different statements of the processing order. And, as I commented previously, this statement doesn't include what I understood to be the whole reason for the change, i.e., the statement about the object ordering the key array.

Copy link
Member

Choose a reason for hiding this comment

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

@Rob--W I'm unclear how this could help the reader: they are now presented with two slightly different statements of the processing order.

I think that a concise one-sentence frame helps the reader with putting your following explanation in the right context. The list of steps below could result in the appearance that there is some complex rule (that the reader must brace for), while the rule is relatively simple: run_at, and then the order as given. If someone already understands this one sentence rule, they don't need to read the rest.

And, as I commented previously, this statement doesn't include what I understood to be the whole reason for the change, i.e., the statement about the object ordering the key array.

Your fuller explanation covers that. The sentence here is mainly to get the rule of thumb. And as phrased, it could also include the order of js/css arrays across all objects (but it could also mean just within the object). It is not needed to make the sentence longer to address that ambiguity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Rob--W For clarity, we should state the rule only once.

Copy link
Member

Choose a reason for hiding this comment

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

I'd be fine with stating the rule only once. Can we choose a phrasing that fits more naturally with how it actually works? The current text below describes some ordering, without the time aspect of run_at (they are evaluated separately at different times!). The mentions of "each object", "object", "key value" and "each object" again is getting me confused, requiring me to think what exactly it is referring to.

Here is an example of text that conveys the concepts more naturally, at least to me.

At each point in time specified by `run_at`
(first `document_start`, then`document_end`, and finally `document_idle`),
the registered objects in `content_scripts` are run as follows:

- In the order as specified in the `content_scripts` array,
  for each object with the matching `run_at` value:
  - CSS is applied in the order as specified in its `css` array, if any.
  - JavaScript code is executed in the order as specified in its `js` array, if any.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Rob--W a slight irony here in that my original draft included detailing the sequence of run_at directives, but @dotproto suggested removing them to avoid duplication and redundancy. Please check out my update.

Copy link
Collaborator

Choose a reason for hiding this comment

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

For reference, here's my comment. At the time I just read it as a list of possible run_at values, not as commentary about their execution order. That was an oversight on my part.

},
{
"matches": ["*://*.mozilla.org/*"],
"js": ["jquery.js"],
Copy link
Member

Choose a reason for hiding this comment

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

May I suggest putting jquery.js back in the original list of two scripts, and use run-first.js here? jquery can be viewed as a dependency of the other scripts, and if they are indeed dependent, then it makes much more sense to put them in the same declaration.

Something called "run-first.js" already conveys from its name what it is supposed to do.

@rebloor rebloor requested a review from Rob--W May 12, 2025 11:25
@@ -324,6 +306,46 @@ Details of all the keys you can include are given in the table below.
</tbody>
</table>

## Load order

Files declared in `content_scripts` are injected into web pages in a defined order. When a webpage loads, matching key objects are processed in this order:
Copy link
Member

Choose a reason for hiding this comment

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

I'd be fine with stating the rule only once. Can we choose a phrasing that fits more naturally with how it actually works? The current text below describes some ordering, without the time aspect of run_at (they are evaluated separately at different times!). The mentions of "each object", "object", "key value" and "each object" again is getting me confused, requiring me to think what exactly it is referring to.

Here is an example of text that conveys the concepts more naturally, at least to me.

At each point in time specified by `run_at`
(first `document_start`, then`document_end`, and finally `document_idle`),
the registered objects in `content_scripts` are run as follows:

- In the order as specified in the `content_scripts` array,
  for each object with the matching `run_at` value:
  - CSS is applied in the order as specified in its `css` array, if any.
  - JavaScript code is executed in the order as specified in its `js` array, if any.

rebloor and others added 2 commits May 16, 2025 15:09
@rebloor rebloor requested a review from Rob--W May 16, 2025 04:27
Copy link
Member

@Rob--W Rob--W left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Collaborator

@dotproto dotproto left a comment

Choose a reason for hiding this comment

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

Thanks for working through this, @rebloor!

@rebloor rebloor merged commit 3cc3f79 into mdn:main May 21, 2025
8 checks passed
@rebloor rebloor deleted the Content-script-load-order branch May 21, 2025 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:WebExt WebExtensions docs size/m [PR only] 51-500 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants