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.

Sign up for GitHub

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

Jump to bottom

Document composition of content script global #24215

Merged
merged 1 commit into from
Feb 14, 2023
Merged

Document composition of content script global #24215

merged 1 commit into from
Feb 14, 2023

Conversation

Rob--W
Copy link
Member

@Rob--W Rob--W commented Feb 6, 2023

Description

Documents the composition of the content script global.

Motivation

Content scripts in extensions behave somewhat consistently across browsers, except when they don't. This is currently not clearly documented, and occasionally confuses extension developers.

Additional details

Related bugs:

Examples of bugs showing confusion from extension developers:

Related issues and pull requests

None

@Rob--W Rob--W added the Content:WebExt WebExtensions docs label Feb 6, 2023
@Rob--W Rob--W requested a review from a team as a code owner February 6, 2023 17:45
@Rob--W Rob--W requested review from willdurand and removed request for a team February 6, 2023 17:45
@github-actions github-actions bot added the Content:Other Any docs not covered by another "Content:" label label Feb 6, 2023
@Rob--W Rob--W requested review from zombie and removed request for willdurand February 6, 2023 17:46
@github-actions
Copy link
Contributor

github-actions bot commented Feb 6, 2023 •
edited
Loading

Preview URLs

External URLs (1)

URL: /en-US/docs/Mozilla/Add-ons/WebExtensions/Content_scripts
Title: Content scripts

(comment last updated: 2023-02-14 13:33:18)

@WebReflection
Copy link

... occasionally confuses extension developers

The use case is not about confusion:

// trap it once and fail forever after
const {setTimeout} = window;

// VS

// works because ... developers are confused?
const setTimeout = window.setTimeout.bind(window);

It is not possible to extract once like in any other JS env so it should be clarified why is that, imho.

@Rob--W
Copy link
Member Author

Rob--W commented Feb 7, 2023

... occasionally confuses extension developers

The use case is not about confusion:

// trap it once and fail forever after
const {setTimeout} = window;

// VS

// works because ... developers are confused?
const setTimeout = window.setTimeout.bind(window);

It is not possible to extract once like in any other JS env so it should be clarified why is that, imho.

This proves my point. window is commonly understood to refer to the global scope, but that is not the case. The standard way to get the global (including non-browser environments) is globalThis. Like this:

const setTimeout = globalThis.setTimeout.bind(globalThis);

@WebReflection
Copy link

There should be zero need to bind global utilities though + globalThis is not available to Firefox < 52, as mentioned in the bug. What are the benefits of having a window that doesn't act globally though, or just like globalThis? why is this addressed as "developers confusion" when it's a Firefox only thing? Anyway, we had implemented a workaround but the wording should be more on FF choice rather than developers confusion, imho.

@Rob--W Rob--W mentioned this pull request Feb 10, 2023
Merged
rpl
rpl approved these changes Feb 10, 2023
View reviewed changes
Copy link
Member

@rpl rpl left a comment

Choose a reason for hiding this comment

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

looks good from my perspective, the followings two comments feel just nits, the one about removing the bugzilla issue linked in this patch in also a nit but also something that I'd like to be handled before this is going to be merged).

files/en-us/mozilla/add-ons/webextensions/chrome_incompatibilities/index.md Outdated Show resolved Hide resolved
files/en-us/mozilla/add-ons/webextensions/content_scripts/index.md Outdated Show resolved Hide resolved
files/en-us/mozilla/add-ons/webextensions/chrome_incompatibilities/index.md Outdated Show resolved Hide resolved
@Rob--W Rob--W force-pushed the webext-compat-content-script-xray branch from 4c42f83 to 76e6986 Compare February 10, 2023 19:32
@Rob--W Rob--W force-pushed the webext-compat-content-script-xray branch from 76e6986 to 5eb55e0 Compare February 14, 2023 13:31
@Rob--W Rob--W merged commit 24b79f9 into mdn:main Feb 14, 2023
tophf
tophf reviewed Jul 20, 2024
View reviewed changes
files/en-us/mozilla/add-ons/webextensions/content_scripts/index.md
As noted at ["Content script environment" at Chrome incompatibilities](/en-US/docs/Mozilla/Add-ons/WebExtensions/Chrome_incompatibilities#content_script_environment), the behavior differs across browsers:

- In Firefox, this behavior is called [Xray vision](/en-US/docs/Mozilla/Add-ons/WebExtensions/Sharing_objects_with_page_scripts#xray_vision_in_firefox).
Content scripts may encounter JavaScript objects from its own global scope or Xray-wrapped versions from the web page.
Copy link

Choose a reason for hiding this comment

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

Plural form mismatch, "its" should be "their" or use a singular for "A content script may...".

tophf
tophf reviewed Jul 20, 2024
View reviewed changes
files/en-us/mozilla/add-ons/webextensions/chrome_incompatibilities/index.md
@@ -197,6 +197,16 @@ When calling `tabs.remove()`:
- **In Firefox:** When a content script makes an HTTP(S) request, you _must_ provide absolute URLs.
- **In Chrome:** When a content script makes a request (for example, using [`fetch()`](/en-US/docs/Web/API/Fetch_API/Using_Fetch)) to a relative URL (like `/api`), it will be sent to `https://example.com/api`.

#### Content script environment

- **In Firefox:** The global scope of the [content script environment](/en-US/docs/Mozilla/Add-ons/WebExtensions/Content_scripts#content_script_environment) is not strictly equal to `window` ({{bug(1208775)}}). More specifically, the global scope (`globalThis`) is composed of standard JavaScript features as usual, plus `window` as the prototype of the global scope. Most DOM APIs are inherit from the page through `window`, through [Xray vision](/en-US/docs/Mozilla/Add-ons/WebExtensions/Sharing_objects_with_page_scripts#xray_vision_in_firefox) to shield the content script from modifications by the web page. Content scripts may encounter JavaScript objects from its own global scope or Xray-wrapped versions from the web page.
Copy link

Choose a reason for hiding this comment

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

  • "are inherited" (missing a passive "ed")
  • two "through" in succession is somewhat confusing to me, maybe switch the second one to "using"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tophf tophf tophf left review comments

@rpl rpl rpl approved these changes

@zombie zombie Awaiting requested review from zombie

Assignees
No one assigned
Labels
Content:Other Any docs not covered by another "Content:" label Content:WebExt WebExtensions docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Rob--W @WebReflection @rpl @tophf