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

Check for Element type? #11

Open
cdukes opened this issue Dec 16, 2016 · 4 comments
Open

Check for Element type? #11

cdukes opened this issue Dec 16, 2016 · 4 comments
Labels
enhancement

Comments

@cdukes
Copy link

cdukes commented Dec 16, 2016

Hi,

We log JS errors from our users, and very infrequently, this line in include-media.js causes some errors:

if (window.getComputedStyle && (window.getComputedStyle(element, '::after').content !== ''))

For instance, Googlebot encountered Failed to execute 'getComputedStyle' on 'Window': parameter 1 is not of type 'Element'. today

And 48 Firefox users (versions 47-50) encountered TypeError: window.getComputedStyle(...) is null over the past couple months

I can't pin down why this sometimes happens and have never been able to reproduce it myself, but I think it would help clear out some edge cases to check that window.getComputedStyle is a function and element is an Element before trying to use them?

Thanks!
@eduardoboucas
Copy link
Owner

I was able to replicate that error by including the script in the <head>, where the DOM isn't available yet. Could this be the case with your errors too?

In any case, it sounds like a good idea to add a safeguard around those expressions.

Thanks for raising the issue!

@mpdude
Copy link

mpdude commented May 11, 2017

Maybe #12 solves this as well?

@eduardoboucas eduardoboucas mentioned this issue May 15, 2017
Merged
@mpdude
Copy link

mpdude commented May 15, 2017

Re-read the problem description – #12 probably does not help: The exceptions occur outside the current try/catch block.

Would it make sense to have a much broader exception handling in that method, catching all sorts of errors and reverting the breakpoints variable to false?

@eduardoboucas
Copy link
Owner

Would it make sense to have a much broader exception handling in that method, catching all sorts of errors and reverting the breakpoints variable to false?

Yes, that sounds sensible. I don't have the capacity to look into this right now, so I'll leave it open for when I or someone else has the chance to make it happen.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mpdude @cdukes @eduardoboucas