1304733 - insertRule on cross domain style sheet throws an exception in FF but not Chrome, Edge, IE

Status: RESOLVED WONTFIX

(Core :: DOM: CSS Object Model, defect, P3)

Description

[Jeff Muizelaar [:jrmuizel]]

I believe this is the reason the galleries here: http://jasminestar.pixieset.com/foundinspirationshoot/ are broken in Firefox.

It looks like our behaviour matches the spec, but perhaps the spec needs updating to reflect reality.

Comment 1

[Cameron McCormack (:heycam)]

There were some discussions in bug 1248444 about this issue, which was WORKSFORMEd since the site was fixed.  Mike, should we pick up the investigation about what to do here again?

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

I think we should file a bug to Chromium first, and see if they agree to align their behavior with the spec.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #2)
> I think we should file a bug to Chromium first, and see if they agree to
> align their behavior with the spec.

It looks like Microsoft switched to behaviour between IE 9 and IE 10. I suspect they would be reluctant to switch back. It seems like it would be much easier for us to switch than convince Microsoft, Apple and Google all to switch behaviour. Is there any reason to believe that our behaviour is better and we should try to push it on the browser vendors?

Comment 4

[Mike Taylor [:miketaylr]]

I talked with bz and annevk about this at TPAC. One question we had was would it fix pages if we didn't throw, but rather just no-oped?

Also, need to check if Blink/WebKit/Edge:
only allow this for insertRule? 
Is it always an append, or can you give it an arbitrary index? 
Does cssText allow setting?

(leaving ni? to come back and test).

Comment 5

[Xidorn Quan [:xidorn] UTC+10]

Because throwing exception is the normal way to report failure for accessing cross origin information. If we want to change the spec to match other browsers, we need to convince the working group as well. I don't think either way is easier, and I think the current spec (and our current implementation) is more consistent to similiar cases.

Comment 6

[Xidorn Quan [:xidorn] UTC+10]

Filed an issue to Chromium: https://bugs.chromium.org/p/chromium/issues/detail?id=650534

Comment 7

[Mike Taylor [:miketaylr]]

> One question we had was would it fix pages if we didn't throw, but rather just no-oped?

This was referring specifically to insertRule on a crossdomain stylesheet. Agreed that it should always fail for reads.

Comment 8

[Mike Taylor [:miketaylr]]

(s/fail/throw)

Comment 9

[Mike Taylor [:miketaylr]]

Here's the relevant bit of script from http://static.pixieset.com/release/js/client.combined.min.js?4d09664031ff9f71bde70ca8704088c6 (starting with "void 0")

if (document.createElementNS && document.createElementNS("http://www.w3.org/2000/svg", "svg").createSVGRect)
        if (c = function(c, d) {
            for (var e = 2 * d.width + d.space, f = e + d.length + Math.ceil(d.width / 2) + 1, g = b().width(2 * f).height(2 * f), h = b("g", {
                "stroke-width": d.width,
                "stroke-linecap": "round",
                stroke: d.color
            }).appendTo(b("g", {
                transform: "translate(" + f + "," + f + ")"
            }).appendTo(g)), i = 0; i < d.segments; i++)
                h.append(b("line", {
                    x1: 0,
                    y1: e,
                    x2: 0,
                    y2: e + d.length,
                    transform: "rotate(" + 360 / d.segments * i + ", 0, 0)",
                    opacity: a.fn.activity.getOpacity(d, i)
                }));
            return a("<div>").append(g).width(2 * f).height(2 * f)
        }
        ,
        void 0 !== document.createElement("div").style.WebkitAnimationName) {
            var e = {};
            d = function(a, b, c) {
                if (!e[b]) {
                    for (var d = "spin" + b, f = "@-webkit-keyframes " + d + " {", g = 0; b > g; g++) {
                        var h = Math.round(1e5 / b * g) / 1e3
                          , i = Math.round(1e5 / b * (g + 1) - 1) / 1e3
                          , j = "% { -webkit-transform:rotate(" + Math.round(360 / b * g) + "deg); }\n";
                        f += h + j + i + j
                    }
                    f += "100% { -webkit-transform:rotate(100deg); }\n}";
                    try {
                        document.styleSheets[0].insertRule(f, 0)
                    } catch (k) {
                        if ("SecurityError" !== k.name)
                            throw k
                    }
                    e[b] = d
                }
                a.css("-webkit-animation", e[b] + " " + c + "s linear infinite")
            }
        }

document.styleSheets[0].href points to "http://cdnjs.cloudflare.com/ajax/libs/font-awesome/4.6.3/css/font-awesome.min.css".

It's possible at some point (or on their staging servers) this was same origin, but maybe they moved assets to cloudfare and never noticed.

In this specific instance, no-oping instead of throwing wouldn't break the page -- I think (assuming that bit of animation is just eye candy).

Jeff, can you provide STR to get it to throw? I'm clicking around the galleries but it seems to WFM.

Comment 10

[Xidorn Quan [:xidorn] UTC+10]

(In reply to Mike Taylor [:miketaylr] from comment #7)
> > One question we had was would it fix pages if we didn't throw, but rather just no-oped?
> 
> This was referring specifically to insertRule on a crossdomain stylesheet.
> Agreed that it should always fail for reads.

It should always fail for writes as well, and they should in general be consistent on what would happen.

Comment 11

[Jeff Muizelaar [:jrmuizel]]

I can't reproduce the failure any more. I believe the try/catch around insertRule is new and was added after I filed this bug. I don't remember seeing it there when I was investigating.

Comment 12

[Mike Taylor [:miketaylr]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #11)
> I can't reproduce the failure any more. I believe the try/catch around
> insertRule is new and was added after I filed this bug. I don't remember
> seeing it there when I was investigating.

Ah, OK. I suspect they tested in Firefox 49 and added that as a quick workaround.

(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #10)
> (In reply to Mike Taylor [:miketaylr] from comment #7)
> > > One question we had was would it fix pages if we didn't throw, but rather just no-oped?
> > 
> > This was referring specifically to insertRule on a crossdomain stylesheet.
> > Agreed that it should always fail for reads.
> 
> It should always fail for writes as well, and they should in general be
> consistent on what would happen.

Well, in principle I agree (and per spec). Problem is Blink/WebKit/Edge (unsure about IE?) don't and we have evidence that some sites rely on it. 

(But, both sites we know about have been fixed, apparently)

Comment 13

[Mike Taylor [:miketaylr]]

Bug 1332281 breaks for the same reason (See https://bugzilla.mozilla.org/show_bug.cgi?id=1332281#c4 which hints at the jQuery plugin origin code).

Comment 14

[Brian Birtles (:birtles)]

Looks like the Chrome issue has moved to: https://bugs.chromium.org/p/chromium/issues/detail?id=775525

Comment 15

[Firefox Bug Husbandry Bot]

Migrating Webcompat whiteboard priorities to project flags. See bug 1547409.

Comment 16

[Firefox Bug Husbandry Bot]

See bug 1547409. Migrating whiteboard priority tags to program flags.

Comment 17

[Mike Taylor [:miketaylr]]

Given that Chrome has aligned with the spec, let's close here.