723388 - Sites depending on cross domain iframe ajax requests are affected by incorrect Array.isArray() behavior

Status: RESOLVED DUPLICATE of bug 714547

(Core :: JavaScript Engine, defect)

Description

[Neil Sarkar]

Our site, groupme.com, received multiple complaints from users who had been upgraded to Firefox 10 last night that they could no longer see messages on our website.

We traced it back to this issue with Array.isArray()
https://gist.github.com/1721221

As you can see in this minimal case, Array.isArray returns false for arrays created in an iframe served from a subdomain and accessed via a shared document.domain 

The javascript on our page is using Array.isArray to determine how to process our API responses. I imagine we are not the only site affected by this, as many sites rely on cross domain iframes to communicate with their own site's API.

Let me know if you have any questions or need any more details, thanks!

Comment 1

[Ben Vinegar]

As Neil points out, this is a common pattern used in cross-domain JavaScript development. Our site, Disqus (http://disqus.com), is similarly affected. Our recourse right now is to remove any code that uses Array.isArray since we can't trust the result on Firefox 10.

Comment 2

[Jeff Walden [:Waldo]]

Hmm.  An attempt at a shell testcase doesn't fail on trunk, although that might just mean the test wasn't sufficiently similar.  I'll investigate further.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Presumably the issue is isArray on a security wrapper around an actual array, right?  I'm not sure how you'd test that in the shell...

I'm also not sure why the behavior here would have changed for Fx10.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Meant to cc Blake

Comment 6

[Masatoshi Kimura [:emk]]

Attachment: Test case

Comment 7

[Luke Wagner [:luke]]

I think this is a dup of bug 714547 which is fixed on nightly and aurora.  Can you confirm this fix?  If so perhaps we can backport the patch to beta so that the fix comes out with FF 11.

Comment 8

[Masatoshi Kimura [:emk]]

Yeah, worked on nightly and aurora but broken on beta and release.
Please nominate beta approval.

Comment 9

[Alex Keybl [:akeybl]]

Not tracking because this is now fixed on all non-release branches in bug 714547.

Comment 10

[John Pignata]

We've fixed this by hardcoding a monkeypatch to the previous buildID. You guys did another release and the site broke again for our customers. We don't want to monkeypatch isArray for all browsers -- is there a chance there will be another release without this bug fix?

Comment 11

[Boris Zbarsky [:bzbarsky]]

There is always a chance that we'll need to ship a release to fix a zero-day security bug.

What exactly does your buildID test look like?

Comment 12

[John Pignata]

Now it's:

if (parseInt(navigator.buildID) >= 20120129021758)
   shim...

Classy.

Comment 13

[Jeff Walden [:Waldo]]

Firefox 11 should fix this, so waiting until then is the default possibility.

The current plan is to not fix Firefox 10, which would mean there'd be an extended-support release with this issue for the next year-ish.  I'm not convinced that's the right decision, given that Array.isArray being broken this way isn't an edge case but rather a fundamental flaw in the implementation.  So I'm appealing that decision now.  Whether that appeal will be successful, who knows.  Keep an eye on bug 714547 for what happens on that front.

Comment 14

[Boris Zbarsky [:bzbarsky]]

John, testing navigator.buildID that way is fundamentally broken.  I'd recommend testing the Gecko version for starting with "10.0" for the moment...

Comment 15

[John Pignata]

K. I'm using navigator.userAgent now instead. Is there a better way to get the Gecko version?

Comment 16

[Boris Zbarsky [:bzbarsky]]

Unfortunately, no.