Skip to content

Fix no-op check to work with Safari and support of old browsers#582

Merged
Rob--W merged 1 commit intomozilla:masterfrom
xeenon:master
Mar 28, 2024
Merged

Fix no-op check to work with Safari and support of old browsers#582
Rob--W merged 1 commit intomozilla:masterfrom
xeenon:master

Conversation

@xeenon
Copy link
Contributor

@xeenon xeenon commented Mar 7, 2024

The browser object is not a direct descendant of Object.prototype in Safari. This check should use instanceof for a more compatible check.

@xeenon
Copy link
Contributor Author

xeenon commented Mar 7, 2024

@diox @rpl @willdurand @Rob--W

@Rob--W
Copy link
Member

Rob--W commented Mar 8, 2024

The reason for looking up the immediate prototype is to avoid activating the polyfill when there is an element on the page that has an ID. Here is the PR that added the logic for more context: #153 (originally checking instanceof Node, but changed in response to #153 (comment) ).

What is the actual prototype chain in Safari? Is it expected to be somewhat stable?

@xeenon
Copy link
Contributor Author

xeenon commented Mar 9, 2024

What is the actual prototype chain in Safari? Is it expected to be somewhat stable?

The problem is the prototype is not reachable, so it can't be compared with the result of Object.getPrototypeOf().

I think a better approach would be to do a similar optional chain check looking for !globalThis.browser?.runtime?.id.

Verify the object that might be at `globalThis.browser` is not already
implementing the basic Web Extension APIs. This is symmetric with the
check to verify the poly fill is only included in extension contexts.

Don't use optional-chaining for these checks, since this needs to
continuing working in older browsers.
@xeenon xeenon changed the title Fix no-op check to work with Safari (#364) Fix no-op check to work with Safari and support of old browsers Mar 25, 2024
@Rob--W Rob--W merged commit 871b49d into mozilla:master Mar 28, 2024
@Rob--W
Copy link
Member

Rob--W commented Mar 28, 2024

Thanks for the patch @xeenon! We'll fix our CI and then publish a new version.

@Rob--W
Copy link
Member

Rob--W commented Apr 3, 2024

We still need to publish an update FYI.

rpl added a commit that referenced this pull request Apr 11, 2024
rpl added a commit that referenced this pull request Apr 11, 2024
…config (#584)

* chore(circleci): updated browser-tools orb to 1.4.8 and cimg/node to 20.12

* chore(deps-dev): bump chromedriver from 112.0.0 to 123.0.3

* chore(deps-dev): bump geckodriver from 3.2.0 to 4.3.3

* test: Update unit test to match new expected behaviors with changes applied from PR #582

---------

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@Rob--W
Copy link
Member

Rob--W commented Apr 16, 2024

FYI just released 0.11.0 that includes this patch: https://github.com/mozilla/webextension-polyfill/releases/tag/0.11.0

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.

2 participants