Skip to content

When EventTarget.prototype exists but is not in the prototype chain of window, polyfill installation breaks.#416

Merged
bicknellr merged 2 commits intomasterfrom
window-eventtarget
Jan 19, 2021
Merged

When EventTarget.prototype exists but is not in the prototype chain of window, polyfill installation breaks.#416
bicknellr merged 2 commits intomasterfrom
window-eventtarget

Conversation

@bicknellr
Copy link
Copy Markdown
Collaborator

@bicknellr bicknellr commented Jan 13, 2021

Firefox 33 has EventTarget, but EventTarget.prototype is not in the prototype chain of window. Currently, if EventTarget exists, then the __shady_native_-prefixed functions are only added to EventTarget.prototype. In Firefox 33, this causes the polyfills to throw when a call to one of these functions on window is attempted later during startup.

Tested as http://cl/351475102 .

… of `window`, polyfill installation breaks.
@bicknellr bicknellr marked this pull request as ready for review January 13, 2021 20:55
@bicknellr bicknellr requested a review from aomarks as a code owner January 13, 2021 20:55
@bicknellr bicknellr requested review from kevinpschaaf and sorvell and removed request for aomarks January 13, 2021 20:55
// In Firefox 33, `EventTarget` exists, but `EventTarget.prototype` is not
// in the prototype chain of `window` and, strangely,
// `window instanceof EventTarget` still returns true.
if (window[NATIVE_PREFIX + 'addEventListener'] === undefined) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does EventTarget exist in Node.prototype? (the else clause also deals with patching Node up)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, Node seems to be set up correctly:

Screen Shot 2021-01-13 at 14 44 33

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ok, thanks for checking.

Copy link
Copy Markdown
Collaborator

@kevinpschaaf kevinpschaaf left a comment

Choose a reason for hiding this comment

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

LGTM

@bicknellr bicknellr merged commit 1463479 into master Jan 19, 2021
@bicknellr bicknellr deleted the window-eventtarget branch January 19, 2021 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants