-
Notifications
You must be signed in to change notification settings - Fork 286
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
Prevent errors when document.documentElement.dataset is not present. #475
Prevent errors when document.documentElement.dataset is not present. #475
Conversation
@teddyzeenny - Should be ready for review... |
(document.documentElement && !document.documentElement.dataset) || | ||
|
||
// when the content script is loaded, this flag is set to `1` | ||
document.documentElement.dataset.emberExtensions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code wouldn't execute if it's not an Ember app.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so you are saying that we do not need to guard here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
1be5d0b
to
a0dc523
Compare
// let ember-debug know that content script has executed | ||
document.documentElement.dataset.emberExtension = 1; | ||
// document.documentElement.dataset is not present for SVG elements | ||
// this guard (and a corresponding one in ember-debug/adapters/basic) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you update the comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
if `document.documentElement` does not have a `dataset` property we will never mark the content script as loaded (and it cannot be an Ember app) this happens in some scenarios with Chrome 45+ and SVG elements.
a0dc523
to
89c9c86
Compare
…oring Prevent errors when document.documentElement.dataset is not present.
Thank you!! |
If
document.documentElement
does not have adataset
property we will never mark the content script as loaded (and it cannot be an Ember app) this happens in some scenarios with Chrome 45+ and SVG elements.Fixes #463
Closes #473