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.

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

Some Chrome extensions styles causes a crash. #6

Closed
AlmeroSteyn opened this issue Feb 7, 2019 · 13 comments
Closed

Some Chrome extensions styles causes a crash. #6

AlmeroSteyn opened this issue Feb 7, 2019 · 13 comments

Comments

@AlmeroSteyn
Copy link

AlmeroSteyn commented Feb 7, 2019

I was looking at a site that uses this library today (https://marcysutton.com/) and through blind luck I noticed that, when extensions load stylesheets, it causes this component to crash in Chrome.

I would submit a PR but would take a lot of time to set up an enviroment where I could test this as I do not yet use it myself, so here are my findings.

If you go to the site above, and load this extension: https://chrome.google.com/webstore/detail/wave-evaluation-tool/jbbplnpkjmmeebjpijfedlgcdilocofh you could try it.

The error happens when the extension has been activated and I attempt a navigation.

In BackgroundUtils.js on line 11 the following error occurs:

SecurityError: Failed to read the 'rules' property from 'CSSStyleSheet': Cannot access rules

From what I could find out it appears that Chrome is the only browser that treats this as an error. From (odoo/odoo#22517 (comment)) it appears that adding the following code fixes this issue:

if (sheets[i].hasOwnProperty('rules')) {

Once again, I would have loved to do a PR just don't have the time today to get the test environment up to confirm the fix before submitting :-(

@timhagn
Copy link
Owner

timhagn commented Feb 7, 2019

Hi @AlmeroSteyn,

would be nice to know which version of gatsby-background-image is used on the page mentioned
cause after PR #2 this should be already fixed in 0.2.2.

The only possibility I see might be wrapping it in a try ... catch block, cause as I mentioned in
my comment and changes after merging the PR, hasOwnProperty() seems to break some styles
completely...

A testing environment may be found at gbitest by the way,
but I'm gonna test it on my own page as soon as I find the time ; ).

@AlmeroSteyn
Copy link
Author

The page is using version 0.2.2.

I did not think to check in the closed things for this as it was breaking, hehe.

BUT, I can confirm that in gbitest it still fails. I confirmed that 0.2.2 was indeed loaded in npm_modules.

I hard edited BackgroundUtils.js in node_modules and a try .. catch indeed solves the issue. The page then renders fine with the extension.

@timhagn
Copy link
Owner

timhagn commented Feb 7, 2019

Thanks for checking : )! Gonna try to integrate it later today (and as I just saw, update gbitest to use the next version, too % ).

@AlmeroSteyn
Copy link
Author

Yeah forgot to mention. I did bump before I tested though :-)

Thanks!

timhagn added a commit that referenced this issue Feb 7, 2019
@timhagn
Copy link
Owner

timhagn commented Feb 7, 2019

And down the rabbit-hole we went oO:
Seems like this behaviour is inherent in Chrome since version 64+, see this SO thread and concerns a changed spec in the CSS Object Model, see w3.org on this.

Fixed it with the aforementioned try ... catch block and at seems to work with WAVE:

WAVE screenshot

Updated the dependencies in gbitest as well (and might you give it a go ; ). Thanks again @AlmeroSteyn!

@AlmeroSteyn
Copy link
Author

Hey it's a pleasure!

I found the bit about 64 but not the change in the spec. Nice sleuthing!!!

I dit a test with 2.0.3 and it works like a charm! Thanks for fixing so fast :-)

@timhagn
Copy link
Owner

timhagn commented Feb 8, 2019

Happy it's working - and you're welcome and thanked - might have bitten me sooner or later ^^.

Didn't know about these changes, either, but I'm quite liking the specs (helped me a few times beforehand, too, like in this case % ).

And thanks for a new word for my vocabulary (as German is my native tongue),
hadn't heard about "sleuthing", that's kewl : ).

@AlmeroSteyn
Copy link
Author

Hehe cool! Kinda word that is never used aside from in whodunit novels and github issues hahaha.

@timhagn
Copy link
Owner

timhagn commented Feb 8, 2019

Yip, "neighs, quoth in yesteryear though nevermore" ; ).
And issue tracking is a kind of detective work, isn't it ^^?
So be thanked for both : ).

@EricSSartorius
Copy link

Not sure if I should re-open a new issue or not but I am currently using GBI v 0.2.4 and still have similar issues. In pages that both use GBI and also require 3rd party css style sheets (google fonts and yotpo widget in my case), I am getting something that reads:

Unable to read stylesheet rules for https://fonts.googleapis.com/css?family=Roboto:+400,500,700 DOMException: Failed to read the 'rules' property from 'CSSStyleSheet': Cannot access rules...

A similar error for the yotpo widget as well.

Is there a way around this? Not exactly sure how to handle it.

@timhagn
Copy link
Owner

timhagn commented Mar 6, 2019

@EricSSartorius Thought this was solved oO. Gonna look into it.
Edit:
Just looked into my own source and this seems to be completely normal,
as in BackgroundUtils.js this is the try .. catch block responsible:

    try {
      classes = typeof styleSheets[i].rules !== 'undefined' ? styleSheets[i].rules :
          typeof styleSheets[i].cssRules !== 'undefined' ? styleSheets[i].cssRules : ''
    } catch (e) {
      console.debug(`Unable to read stylesheet rules for ${ styleSheets[i].href }`, e)
    }

So it should only be visible in dev environment and not really break anything.

@timhagn
Copy link
Owner

timhagn commented Mar 8, 2019

Changed it to only console.debug with e.message, so one won't get overthrown by the stack trace ; ).

@EricSSartorius
Copy link

Ah ok, Yeah I didn't notice any crazy behavior only that I had a ton of messages in the console and it took me by surprise I guess.

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

No branches or pull requests

3 participants