fix: prevent "document is not defined" error in non-DOM environments - #428
Open
ZayanKhan-12 wants to merge 2 commits into
Open
fix: prevent "document is not defined" error in non-DOM environments#428ZayanKhan-12 wants to merge 2 commits into
ZayanKhan-12 wants to merge 2 commits into
Conversation
The MetaMaskInpageProvider constructor reads document.readyState (and registers a DOMContentLoaded listener on window) whenever shouldSendMetadata is enabled. In non-DOM environments, such as extension background pages and service workers, the document global does not exist, so constructing the provider throws a "document is not defined" ReferenceError. Guard the document access with a typeof check and treat a missing document as "already loaded", sending the site metadata immediately. Also guard the DOM reads in siteMetadata.ts so that the metadata request falls back to the hostname (and a null icon) instead of failing when no document is available. Behavior in normal window contexts is unchanged. Fixes MetaMask#191 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Constructing
MetaMaskInpageProviderwithshouldSendMetadataenabled throws adocument is not definedReferenceErrorin non-DOM environments (e.g. Chrome extension background scripts / service workers), because the constructor unconditionally readsdocument.readyStateand registers aDOMContentLoadedlistener onwindow.This PR guards the DOM access:
MetaMaskInpageProvider: iftypeof document === 'undefined', the missing document is treated as "already loaded" and the site metadata is sent immediately, instead of readingdocument.readyState/ waiting forDOMContentLoaded.siteMetadata.ts: the site name/icon extraction now receivesglobalThisand falls back tolocation.hostname(and anullicon) when nodocumentis available, so the metadata request no longer fails with an unhandledReferenceErrorlogged as an error. Also fixes a small inconsistency wheregetSiteNameread the globalwindowinstead of itswindowObjectparameter for the hostname fallback.Behavior in normal window contexts is unchanged.
Fixes #191
Testing
yarn jest— all 9 suites / 135 tests pass, including new tests:DOMContentLoadedwhen the document is still loadingdocumentisundefined(simulated by mocking thedocumentgetter in jsdom)sendSiteMetadatasends DOM-derived metadata, and falls back to the hostname whendocumentisundefinedyarn lint:eslint— passes (the two touched test files were added to the existing mixed-environmentno-restricted-globalsoverride, following the pattern used forEIP6963.test.tset al.)yarn lint:misc --check— passesyarn build— passes🤖 Generated with Claude Code
Note
Low Risk
Small defensive guards on optional metadata sending; normal in-page behavior is preserved and covered by new tests.
Overview
Fixes
document is not definedwhenMetaMaskInpageProvideris created withshouldSendMetadatain environments without a DOM (extension backgrounds, service workers).The constructor now treats a missing
documentlike a fully loaded page and callssendSiteMetadataimmediately instead of readingdocument.readyStateor waiting onDOMContentLoaded.siteMetadataextraction usesglobalThisand, whendocumentis absent, falls back tolocation.hostnameand anullicon (and uses the passed object for the hostname fallback instead of the globalwindow).Browser behavior is unchanged; tests cover DOM, loading, and no-document paths.
Reviewed by Cursor Bugbot for commit 9638968. Bugbot is set up for automated code reviews on this repo. Configure here.