Skip to content

Fix for FireFox "SecurityError: The operation is insecure." if an external CSS ref is present#4

Merged
flesler merged 3 commits intoflesler:masterfrom
dorival:master
Oct 14, 2016
Merged

Fix for FireFox "SecurityError: The operation is insecure." if an external CSS ref is present#4
flesler merged 3 commits intoflesler:masterfrom
dorival:master

Conversation

@dorival
Copy link
Contributor

@dorival dorival commented Oct 13, 2016

Only in FireFox, if we try to access the "cssRule" attribute that belongs to an external resource, let's say, a Google Font CSS ref, FireFox will throw an exception and stop execution of the plugin. This fix follows a discussion on StackOverflow around the same problem.

So far, Internet Explorer, Safari (on iOS) and Chrome did not had problems with this exception, and returns "null". The simple fact of accessing this object property was enough to cause the exception on FireFox.

This pull fixes this problem by adding a try/catch around. The problem went away and tests did not identify any change on the plugin behaviour.

(There were more commits then it should be as I made a mistake on my git commands)

SecurityError is thrown if executing on FireFox, this is because the
code is attempting to access the cssRules attribute, which by default
returns "null" if you are not allowed to access, but FireFox throws an
exception. This fix aims to try/catch this exception and assign the
expected "null" to the variable. This issue only happens if there is an
external CSS referred in your HTML Dom
SecurityError is thrown if executing on FireFox, this is because the
code is attempting to access the cssRules attribute, which by default
returns "null" if you are not allowed to access, but FireFox throws an
exception. This fix aims to try/catch this exception and assign the
expected "null" to the variable. This issue only happens if there is an
external CSS referred in your HTML Dom
@flesler
Copy link
Owner

flesler commented Oct 13, 2016

Hi @dorival, thanks for contributing. I don't have the time to test this well honestly, have you tested on most modern browsers?

I can release this bumping the minor version or maybe as a beta for some time.
Also, if you don't mind, I'd like to squash the commits into a single one to keep master clean, (you don't need to do nothing on your side).

@dorival
Copy link
Contributor Author

dorival commented Oct 14, 2016

Hello @flesler, I have gone through a battery of smoke tests using Microsoft Edge, IE11, FireFox, Chrome, Safari (Yosemite) and Safari (iOS 9 and 10). A QA also stressed the system that makes use of your plugin on FireFox and Chrome for iOS and Android. So far, nothing raised concerns.

It would be a great addition to add some unit tests, perhaps I can factory a few of those in the near future, since our project uses your plugin quite heavily.

@flesler
Copy link
Owner

flesler commented Oct 14, 2016

Hi @dorival that'd be great, to be honest, this project is kinda old and not too popular so I don't put time into it. Whatever you can provided will be appreciated, I'll merge the PR

@flesler flesler merged commit a089b0b into flesler:master Oct 14, 2016
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

Successfully merging this pull request may close these issues.

2 participants