Opened 9 years ago
Closed 8 years ago
#17535 closed defect (fixed)
[patch][cla] dojox/html/styles throws SecurityError
Reported by: | Wouter Hager | Owned by: | dylan |
---|---|---|---|
Priority: | low | Milestone: | 1.10 |
Component: | Dojox | Version: | 1.9.1 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description
A same origin violation error is thrown when dojox/html/styles encounters a stylesheet from another domain.
dojox/data/CssRuleStore has the same problem, see #16096
Attachments (1)
Change History (21)
comment:1 Changed 8 years ago by
Component: | General → Dojox |
---|---|
Owner: | set to Adam Peller |
Changed 8 years ago by
Attachment: | if-remote-cssRules-fails.patch added |
---|
comment:2 Changed 8 years ago by
comment:3 Changed 8 years ago by
FYI, patches should be submitted as pull requests, see https://github.com/dojo/dojo/blob/master/CONTRIBUTING.md#submitting-a-pull-request.
In this case though I don't see the point of this change. All you are doing is silently surpressing an error message. The stylesheet load still doesn't work.
comment:4 Changed 8 years ago by
Apologies, I would be happy to create PRs as it's less of a hassle, but I thought I had to provide a patch. And apologies again, I had a working fix, but made a small edit without testing. I'll try again later today.
comment:6 Changed 8 years ago by
Hi Bill, I'm not quite sure what you expect. Catching the silent error means that the script will just continue loading other stylesheets, won't it?
comment:7 Changed 8 years ago by
That's what I would guess. Do you think that's a good thing? Isn't it better to tell the app developer that they have a problem and they need to fix it?
comment:8 Changed 8 years ago by
I'd warn about it in the console. That way the developer at least gets some info. A solution would be to add a CORS attribute to the <link> element, but that's HTML5, so I can't put that in the warning.
I think files loaded from CDN should be ignored anyway.
So all in all I think it's a good enough idea.
comment:9 Changed 8 years ago by
Milestone: | tbd → 1.10 |
---|---|
Summary: | dojox/html/styles throws SecurityError → [patch][cla] dojox/html/styles throws SecurityError |
comment:10 Changed 8 years ago by
Hm well maybe you're right. But the error shouldn't be silent. I'll try to find out why it is.
comment:11 Changed 8 years ago by
I definitely agree that the error shouldn't be silent. I'm not familiar with the code you are changing; perhaps the error occurs in a separate thread, asynchronously?
comment:12 Changed 8 years ago by
My mistake, it's not silent. Sorry, don't know why I thought it was.
comment:13 Changed 8 years ago by
Ah I understand, the error is silent in dojox/data/css.js. see #16096
That's probably why I filed this in the first place, I must have mixed up the issues.
comment:14 Changed 8 years ago by
OK, so this ticket (and associated pull request) should be closed?
comment:15 Changed 8 years ago by
Well I stil think a warning (and more verbose) would be better. Not allowed to read one css doesn't mean all should fail. This is a convenience class to begin with. But that is my opinion, so I leave it up to you.
comment:16 Changed 8 years ago by
Owner: | changed from Adam Peller to dylan |
---|---|
Priority: | undecided → low |
Status: | new → assigned |
@wshager, at this point, I think you know this code as much as anyone that's active, so if you have an alternative patch to propose, we'll consider it. Thanks!
comment:17 Changed 8 years ago by
I've added a warning. I think this convenience should be convenient.
comment:18 Changed 8 years ago by
Please note that CSS from CDN (ajax.googleapis.com/ajax/libs/dojo) prevents mimetype sniffing. That probably causes the issue in my code.
comment:19 Changed 8 years ago by
I'm fine with any changes, you can change it to a warning if you like. I guess throwing exceptions doesn't work if you are in a separate thread (i.e. an asynchronous callback).
comment:20 Changed 8 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
please review attached patch