Last Comment Bug 734215 - Constructing a typed array with a security-wrapped array buffer produces incorrect result
: Constructing a typed array with a security-wrapped array buffer produces inco...
Status: RESOLVED FIXED
[js:p1:fx16]
: regression
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: 12 Branch
: x86 Mac OS X
: -- normal with 1 vote (vote)
: mozilla16
Assigned To: Steve Fink [:sfink] [:s:]
:
Mentors:
Depends on: 667388 769192
Blocks: cpg 755122
  Show dependency treegraph
 
Reported: 2012-03-08 12:13 PST by Michael Hanson
Modified: 2012-07-18 09:07 PDT (History)
17 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
wontfix
+
fixed


Attachments
An extension that demonstrates the issue (6.38 KB, application/x-tar)
2012-03-08 12:16 PST, Michael Hanson
no flags Details
use .byteLength instead of .length when initializing from a proxy (836 bytes, patch)
2012-04-24 00:30 PDT, Mark Hammond [:markh]
no flags Details | Diff | Review
A test to be run under the js shell (1.22 KB, application/x-javascript)
2012-04-24 06:28 PDT, Mark Hammond [:markh]
no flags Details
unwrap the array buffer + tests (2.94 KB, patch)
2012-04-24 20:38 PDT, Mark Hammond [:markh]
no flags Details | Diff | Review
move the unwrap further up the stack, more tests (3.50 KB, patch)
2012-04-25 00:33 PDT, Mark Hammond [:markh]
no flags Details | Diff | Review
new test for error (1.73 KB, application/x-javascript)
2012-06-08 10:57 PDT, Shane Caraveo (:mixedpuppy) [on leave 5/16-7/16]
no flags Details
Typed array property access should allow proxies (6.49 KB, patch)
2012-06-18 21:53 PDT, Steve Fink [:sfink] [:s:]
luke: review-
Details | Diff | Review
Typed array property access should allow proxies (34.75 KB, patch)
2012-06-25 17:22 PDT, Steve Fink [:sfink] [:s:]
luke: review+
Details | Diff | Review

Description Michael Hanson 2012-03-08 12:13:34 PST
I have constructed a Sandbox into which I have imported the WebSocket constructor.  When I make a connection from this WebSocket to a remote server using binaryType "arraybuffer" and read a message from the socket, I am unable to read the buffer.

Specifically, 

  socket.onmessage = function(msg) {
    console.log("msg.data.byteLength is "+ msg.data.byteLength);
    var array = new Uint8Array(msg.data);
    console.log("array.buffer.byteLength is " + array.buffer.byteLength
  }

shows that msg.data.byteLength is (e.g.) 4.  array.buffer is defined, but array.buffer.byteLength (and array.length) are zero.

A testcase is attached; it constructs the worker and attempts to connect to Facebook's chat server (with a bogus username and password, so the message it receives is an error).  The testcase should not be used for automation; we would need to use a test websocket server.  Any I/O should be sufficient for the test as long as the length was >0.

I note that using binaryType="blob" and a FileReader in the same setup works fine.
Comment 1 Michael Hanson 2012-03-08 12:16:02 PST
Created attachment 604154 [details]
An extension that demonstrates the issue

An extension that demonstrates the issue.  The connection is run immediately on bootstrap startup.
Comment 2 Michael Hanson 2012-03-08 12:16:57 PST
Additional notes from email:

bholley:
Well, sure - that's the "spidermonkey doesn't handle proxies/wrappers properly" case.

In any case though, you may be right. When constructing a typed array with the security-wrapped array buffer, we'll get here:

http://mxr.mozilla.org/mozilla-central/source/js/src/jstypedarray.cpp#1693

This check will fail (since the object is a proxy), and we'll end up in the bailout case below. However, I would think that the bailout case should work (albeit slowly), since, if the object is a proxy, all of the property-getting should work transparently.

sicking:
That's not a "slow path". That's a "We were handed an Array, not an
ArrayBuffer" path. The first thing it does is get the .length property
but ArrayBuffers have no such property, only a .byteLength property.
And even if it did, ArrayBuffers don't have [0], [1], etc accessors to
grab the individual bytes (you have to wrap in a view to get at the
actual data).

In other words, the two paths behave very differently. Intentionally.

bholley:
Ah! I see. Then yeah, that's probably exactly what's happening.

So the fix is probably either to make this code understand proxies (questionably worth the effort), or to just try to unwrap if we hit a wrapper. The latter involves making a security decision, which my patches for bug 667388 let us do. So if Blake signs off on them, fixing this should be pretty straightforward.
Comment 3 Bobby Holley (PTO through June 13) 2012-03-08 12:34:39 PST
Marking this bug as dependent on the structured clone bug, since presumably we'd want to use the same infrastructure here.
Comment 4 Mark Hammond [:markh] 2012-03-28 15:57:17 PDT
This is probably a dupe of bug 737245 - which is where all the cool kids are hanging out and working on a fix.
Comment 5 Mark Hammond [:markh] 2012-04-04 02:53:05 PDT
this is fixed with the patches in bug 737245

*** This bug has been marked as a duplicate of bug 737245 ***
Comment 6 Mark Hammond [:markh] 2012-04-24 00:29:25 PDT
This problem has either come back, or I was mistaken that bug 737245 fixed it (I'm still trying to confirm that).

In the current code, the problem is that in the case of a proxy, the call:

  new Uint8Array(array_buffer_proxy);

end up calling js_GetLengthProperty() on array_buffer_proxy - however, array buffer's don't have a .length, but a .byteLength.  Hence the length of the new array is zero.

If I change the call to js_GetLengthProperty to code that gets .byteLength things work as expected, but I'm not sure this is the correct thing to do or if there is something else I'm missing.  I'll attach the patch anyway to see what people think.
Comment 7 Mark Hammond [:markh] 2012-04-24 00:30:31 PDT
Created attachment 617809 [details] [diff] [review]
use .byteLength instead of .length when initializing from a proxy
Comment 8 Mark Hammond [:markh] 2012-04-24 06:28:33 PDT
Created attachment 617849 [details]
A test to be run under the js shell

Attaching a JS shell test-case which is much simpler to run and debug.  It creates an object, then tests an expression executed both in the same compartment as the object and then in another - the same expression should be true in both compartments.  I've tried to make it the start of something that can end up checked into the tests/ directory.

Unfortunately, it seems to demonstrate the patch I attached isn't complete - of the 3 tests here, 2 fail without the patch - but 1 still fails with the patch.  It's late, so I'll attach it here as I may not get back to it for a couple of days...

Before the patch:
OK: 'let a= new ArrayBuffer(10);' -> 'a.byteLength==10' worked.
FAILED: 'let a = new ArrayBuffer(10);' -> 'new Uint8Array(a).length==10' failed
FAILED: 'let a = new ArrayBuffer(10); new Uint8Array(a)[0] = 123;' -> 'new Uint8Array(a)[0]==123' failed
done - 1 worked, 2 failed

After the patch:
OK: 'let a= new ArrayBuffer(10);' -> 'a.byteLength==10' worked.
OK: 'let a = new ArrayBuffer(10);' -> 'new Uint8Array(a).length==10' worked.
FAILED: 'let a = new ArrayBuffer(10); new Uint8Array(a)[0] = 123;' -> 'new Uint8Array(a)[0]==123' failed
done - 2 worked, 1 failed
Comment 9 Mark Hammond [:markh] 2012-04-24 20:38:14 PDT
Created attachment 618156 [details] [diff] [review]
unwrap the array buffer + tests

The other patch was wrong - the branch it changed is handling the creation of a TypedArray from a regular javascript array so it broke that.  A better fix seems to be to simply unwrap the object before checking it is already an array.  The existing typedarray.js tests all pass and there is a new test file, typedarray_cc.js, which also passes with the patch.
Comment 10 Steve Fink [:sfink] [:s:] 2012-04-24 21:45:40 PDT
markh: sorry, should've talked to you earlier. I'll look at your patch tomorrow, but I'm pretty sure I re-broke this one with bug 711843, because it fixed bug 743000. I'll look tomorrow, but you could check whether your patch is going to re-break 743000.

I have a fix in my patch queue, but it's kind of messy. It might help if you could explain the scenario where you're wanting to create a view on an ArrayBuffer in another compartment.

(I also have a test that looks very similar to yours!)
Comment 11 Mark Hammond [:markh] 2012-04-24 22:40:15 PDT
I'll try and look at that other bug, but a simplified usecase is, from a sandbox:

"""
let buffer = new ArrayBuffer(10);
let reader = new FileReader();
reader.onload = function(event) {
  let a = new Uint8Array(reader.result);
}
let blob = new Blob([buffer], {type: "binary"});
reader.readAsArrayBuffer(blob);
"""

The actual use-case is using WebSockets, where the initial "buffer" came from the websocket's onmessage.  The "real" usecase is in the initial attachment and that minimal case is in the bootstrap.js attachment (both now obsoleted but available)
Comment 12 Mark Hammond [:markh] 2012-04-25 00:33:40 PDT
Created attachment 618181 [details] [diff] [review]
move the unwrap further up the stack, more tests

*sigh* - yes, the first js shell test case from bug 743000 fails with this in place.  Moving the unwrap up the stack fixes this and none of the other js shell tests fail in a debug build - but even then I suspect you to say the UnwrapObject() is still wrong - but that's OK, it's all a learning exercise for me :)

Updating the patch which includes 3 new tests, one of which failed with the previous version of the patch.  All JS tests pass in a debug build with this change (well - except for one strange test that times out, but that happens without the patch applied for me...)
Comment 13 Steve Fink [:sfink] [:s:] 2012-04-25 15:56:17 PDT
So the basic problem is that if you just unwrap to allow new TypedArray(foreignArrayBuffer) to work, then you've created a bit of a monstrosity: a typed array in one compartment with a slot directly holding an array buffer from another compartment. I think cross-compartment wrappers are about the only thing that does things like that, and they need special handling by the garbage collector.

One way to fix this would be to detect this case and copy the ArrayBuffer from its origin compartment into the created typed array's compartment, but then changes made through a different view in the origin compartment would not be visible. If you knew that nothing would ever touch that ArrayBuffer in the origin compartment, you could create a new ArrayBuffer but steal the actual data from the original ArrayBuffer (and neuter it so nothing can reach the data through it). I have a patch for that in bug 720949, and I think that's probably what you'd actually want in the use case you gave in comment 11.

The general fix is to do the opposite -- detect when you're crossing compartments, and create the typed array in the *origin* compartment. Then you'd wrap it for the calling compartment and return the wrapper. I have a slightly outdated patch for that in bug 741041, but it's a little messy and Luke was wondering whether this case would actually show up in practice. I'm still not entirely sure if it does, given that it seems like transferable ArrayBuffers are probably better for the example you gave. I'll upload the latest patches there, anyway.

And the 3rd possibility is that I'm missing something obvious and there's an easy fix. :)
Comment 14 Luke Wagner [:luke] 2012-05-11 14:33:44 PDT
I agree with the option of "fix it on the JS engine side": this is a bug that is made visible to plain web content with compartment-per-global, so we should just fix it.
Comment 15 Boris Zbarsky [:bz] 2012-05-11 14:36:47 PDT
Requesting tracking for web-visible regression from cpg here.
Comment 16 Steve Fink [:sfink] [:s:] 2012-05-16 12:42:11 PDT
I have a fix for this in bug 741041, which already has r+ but I just need to rebase it on top of the RootedVar/Handle changes. Should be landing shortly.

*** This bug has been marked as a duplicate of bug 741041 ***
Comment 17 Steve Fink [:sfink] [:s:] 2012-06-07 17:32:56 PDT
According to bug 741040 comment 9, this was in fact *not* fixed by bug 741041. That is bad.

Lemme see how badly I can mess up the tracking flags...
Comment 18 Shane Caraveo (:mixedpuppy) [on leave 5/16-7/16] 2012-06-07 17:46:12 PDT
(In reply to Steve Fink [:sfink] from comment #17)
> According to bug 741040 comment 9, this was in fact *not* fixed by bug
> 741041. That is bad.

To let you know, I got a "permission denied to access object" exception when doing "new Uint8Array()"
Comment 19 Shane Caraveo (:mixedpuppy) [on leave 5/16-7/16] 2012-06-08 10:57:53 PDT
Created attachment 631464 [details]
new test for error

mark was able to get a test together for this last night, I tweeked it a bit this morning.  this tests our use case in both content iframes and xul iframes, as well is differentiating the hidden window to make sure that is not an issue.  The content test currently passes, the xul tests fail.
Comment 20 Mark Hammond [:markh] 2012-06-10 22:25:07 PDT
Comment on attachment 631464 [details]
new test for error

This test relies on some test infrastructure from the social project.  It might be better if the test can be changed back to not relying on this so the test could be run by the platform/content guys, and even dropped directly next to other FileReader tests in the tree.
Comment 21 Alex Keybl [:akeybl] 2012-06-18 16:16:19 PDT
Assigning to dmandelin temporarily so that we can find an assignee for this regression.
Comment 22 Steve Fink [:sfink] [:s:] 2012-06-18 21:53:31 PDT
Created attachment 634293 [details] [diff] [review]
Typed array property access should allow proxies

Luke: turns out I left a big gap in my test cases using the cross-compartment array buffers and their views -- I only tested function calls, not property accesses.

---

When constructing a view on an ArrayBuffer in a different compartment, a proxy is returned for a typed array in that other compartment, and that typed array's prototype is itself a proxy for the prototype in the original compartment.

When accessing properties on the resulting object, the correct routine will be used, but the proxy will be passed as the target object. This proxy must be unwrapped to get the properties' values.
Comment 23 Steve Fink [:sfink] [:s:] 2012-06-18 21:57:09 PDT
The patch I attached will fix one of the problems I saw in a test case; I'm not sure if it fixes everything. In particular, I would expect an error message referring to TypedArray.prototype.something, not "permission denied".

Oh, and this is mine unless I can track it down to XPConnect or something. Maybe the wrapping solution I explained badly above (should've said "*target*" instead of "*origin*" compartment; see below) is having trouble when one of the compartments is not accessible from the other. The test case I came up with to check that did not reproduce it, though.
Comment 24 Luke Wagner [:luke] 2012-06-19 12:09:54 PDT
Comment on attachment 634293 [details] [diff] [review]
Typed array property access should allow proxies

The "don't sprinkle random UnwrapObjects around b/c it makes it really easy to have cross-compartment violations" principle indicates that this patch isn't the way to go and, lo and behold,

>     JS_SET_RVAL(cx, vp, ObjectValue(*TypedArray::getBuffer(tarray)));

a cross-compartment violation (getBuffer(tarray)->compartment() != cx->compartment)!

Waldo points out that these properties are now defined to be accessors so we should make them accessors which means turning them into JSNatives which would allow us to use NonGenericMethodGuard which will, I believe, solve these problems cleanly.
Comment 25 Steve Fink [:sfink] [:s:] 2012-06-25 15:34:21 PDT
(In reply to Luke Wagner [:luke] from comment #24)
> Comment on attachment 634293 [details] [diff] [review]
> Typed array property access should allow proxies
> 
> Waldo points out that these properties are now defined to be accessors so we
> should make them accessors which means turning them into JSNatives which
> would allow us to use NonGenericMethodGuard which will, I believe, solve
> these problems cleanly.

That made sense to me, so I implemented it. How can I do it without reopening bug 565604?

The problem is that if I have an object o whose prototype is a Float32Array, then presumably o.length should retrieve the length of the array. But o is passed as |this|, so although the correct getter will be invoked, it will be invoked with an Object |this|, so NonGenericMethodGuard will yell about it being called with the wrong clasp.

I could search up the proto chain for the typed array, but then I would have to reconstruct a CallArgs with the |this| value swapped in order to use NonGenericMethodGuard.

On the other hand, my reading of section 4.4.6 of the WebIDL spec seems to indicate that this shouldn't work in the first place.
Comment 26 Luke Wagner [:luke] 2012-06-25 15:44:00 PDT
Perhaps Waldo can comment on this?
Comment 27 Jeff Walden [:Waldo] (remove +bmo to email) 2012-06-25 15:52:24 PDT
It shouldn't work in the first place.

I tend to think typed arrays, array buffers, and all that should have actual data properties for this stuff, not accessor properties.  This is easily fixed if typed arrays were specified in ECMAScript spec style.  It's not something you can fix using WebIDL.  Typed arrays really push the boundaries of WebIDL, and I think we'd be better off if they didn't use it.  But since typed arrays do use WebIDL now, throwing on accessing length via prototype chain is correct.

Note that implementing these properties is hard in the cross-compartment case now because of how special typed arrays are (really, exactly the same way dense arrays are special now).  But it'd be easy in the new object/property representation.
Comment 28 Jeff Walden [:Waldo] (remove +bmo to email) 2012-06-25 15:53:56 PDT
(In reply to Jeff Walden [:Waldo] (busy, try to prefer other reviewers if possible) from comment #27)
> Note that implementing these properties is hard in the cross-compartment
> case now because of how special typed arrays are (really, exactly the same
> way dense arrays are special now).

...if they're to be visible as data properties, I should say.
Comment 29 Dave Herman [:dherman] 2012-06-25 16:00:16 PDT
> It shouldn't work in the first place.

That's correct according to my reading of WebIDL.

> I tend to think typed arrays, array buffers, and all that should have actual
> data properties for this stuff, not accessor properties.

When you say "this stuff" do you just mean for the immutable .length property? Or also for the indexed properties?

Anyway, I'm not sure I agree. First, a typed array is a view that aliases the data of a buffer, so its values need to be in sync with the buffer. And I've failed to convince Kenneth Russell to give up on the awful .length-zeroing semantics for transferring buffers, so even that is a property whose value can change and must be kept in sync with the buffer.

> This is easily
> fixed if typed arrays were specified in ECMAScript spec style.  It's not
> something you can fix using WebIDL.  Typed arrays really push the boundaries
> of WebIDL, and I think we'd be better off if they didn't use it.

I do agree with this, and ES6 will specify typed arrays in the usual ES spec style. There's really no point in doing it via WebIDL. It's an unnecessary layer of spec indirection when the only point to typed arrays is to be an ECMAScript feature.

Dave
Comment 30 Jeff Walden [:Waldo] (remove +bmo to email) 2012-06-25 16:19:29 PDT
(In reply to Dave Herman [:dherman] from comment #29)
> > I tend to think typed arrays, array buffers, and all that should have actual
> > data properties for this stuff, not accessor properties.
> 
> When you say "this stuff" do you just mean for the immutable .length
> property? Or also for the indexed properties?

Since ECMAScript only gives you [[GetOwnProperty]] and [[DefineOwnProperty]] rope, I think you'd have to handle every property algorithmically, indexed properties included.  I think it's a bad idea to have elements exposed with odd getter/setter functions (especially since there'd have to be a different one for every element), so they'd probably have to be exposed as data properties.  (Although they wouldn't be implemented as normal data properties by any engine that cares about performance at all.  And possibly not even by ones that don't, for the aliasing concern.)  They'd be non-configurable fromt he start so they can't be deleted, which would be shenanigans.  [[DefineOwnProperty]] would prevent things like marking them non-writable.

> Anyway, I'm not sure I agree. First, a typed array is a view that aliases
> the data of a buffer, so its values need to be in sync with the buffer. And
> I've failed to convince Kenneth Russell to give up on the awful
> .length-zeroing semantics for transferring buffers, so even that is a
> property whose value can change and must be kept in sync with the buffer.

If you have length-zeroing, and that presumably nulls out buffer for consistency and sets byteOffset to zero as well, then yeah, the named properties have to be accessors.  I'd forgotten about that additional idea.

> I do agree with this, and ES6 will specify typed arrays in the usual ES spec
> style. There's really no point in doing it via WebIDL. It's an unnecessary
> layer of spec indirection when the only point to typed arrays is to be an
> ECMAScript feature.

Good stuff!
Comment 31 Steve Fink [:sfink] [:s:] 2012-06-25 17:22:41 PDT
Created attachment 636558 [details] [diff] [review]
Typed array property access should allow proxies

My apologies for having 3 completely different ways of defining the accessors. I couldn't come up with a template scheme that covered all of them.

Note that I also avoided failing a test by nuking it entirely. Waldo said on IRC that it's ok.
Comment 32 Luke Wagner [:luke] 2012-06-26 11:35:12 PDT
Comment on attachment 636558 [details] [diff] [review]
Typed array property access should allow proxies

Review of attachment 636558 [details] [diff] [review]:
-----------------------------------------------------------------

Excellent, thanks!

::: js/src/jstypedarray.cpp
@@ +1385,5 @@
>          obj->setLastPropertyInfallible(empty);
>  
>          DebugOnly<uint32_t> bufferByteLength = buffer.byteLength();
> +        DebugOnly<uint32_t> arrayByteLength = static_cast<uint32_t>(byteLengthValue(obj).toInt32());
> +        DebugOnly<uint32_t> arrayByteOffset = static_cast<uint32_t>(byteOffsetValue(obj).toInt32());

Could you use the non-Value-returning accessors instead of the *Value(obj).toInt32()?  (Here and maybe 5 other times below.)

@@ +1392,2 @@
>          JS_ASSERT(buffer.dataPointer() <= getDataOffset(obj));
>          JS_ASSERT(getDataOffset(obj) <= offsetData(obj, bufferByteLength));

I think it would look nicer just wrap the whole thing in #ifdef DEBUG.  It would also avoid the possibility that the compiler doesn't throw everything away.

@@ +1516,5 @@
> +    {
> +        RootedId id(cx, NameToId(name));
> +        unsigned flags = JSPROP_PERMANENT | JSPROP_READONLY | JSPROP_SHARED | JSPROP_GETTER;
> +
> +        JSObject *getter = js_NewFunction(cx, NULL, Getter<ValueGetter>, 0, 0, global, NULL);

You can now use cx->compartment->global() instead of passing a handle to the global.

::: js/src/jstypedarrayinlines.h
@@ +67,3 @@
>  inline uint32_t
>  TypedArray::getLength(JSObject *obj) {
> +    return lengthValue(obj).toInt32();

Since you are tweaking these anyways, could you rename getX() to x() for X = {Length, ByteOffset, ByteLength, ...}, since these are all infallible getters?
Comment 33 Steve Fink [:sfink] [:s:] 2012-06-27 16:49:51 PDT
(In reply to Luke Wagner [:luke] from comment #32)
> ::: js/src/jstypedarray.cpp
> @@ +1392,2 @@
> >          JS_ASSERT(buffer.dataPointer() <= getDataOffset(obj));
> >          JS_ASSERT(getDataOffset(obj) <= offsetData(obj, bufferByteLength));
> 
> I think it would look nicer just wrap the whole thing in #ifdef DEBUG.  It
> would also avoid the possibility that the compiler doesn't throw everything
> away.

Fixed.

> @@ +1516,5 @@
> > +    {
> > +        RootedId id(cx, NameToId(name));
> > +        unsigned flags = JSPROP_PERMANENT | JSPROP_READONLY | JSPROP_SHARED | JSPROP_GETTER;
> > +
> > +        JSObject *getter = js_NewFunction(cx, NULL, Getter<ValueGetter>, 0, 0, global, NULL);
> 
> You can now use cx->compartment->global() instead of passing a handle to the
> global.

Ok, though I may have done it just in time for njn to make it fallible. I guess he gets to switch it to getGlobal() for me.

> ::: js/src/jstypedarrayinlines.h
> @@ +67,3 @@
> >  inline uint32_t
> >  TypedArray::getLength(JSObject *obj) {
> > +    return lengthValue(obj).toInt32();
> 
> Since you are tweaking these anyways, could you rename getX() to x() for X =
> {Length, ByteOffset, ByteLength, ...}, since these are all infallible
> getters?

Done, though this propagated to a number of different files.

Also note that I had to modify some regression tests that depended on eg Object.create(new Uint32Array(1)).byteLength being accessible through the proto chain.

https://hg.mozilla.org/integration/mozilla-inbound/rev/d861452b261c
Comment 35 Steve Fink [:sfink] [:s:] 2012-06-27 17:18:24 PDT
Should've built after rebasing. My tree was a day old.

https://hg.mozilla.org/integration/mozilla-inbound/rev/c408dd243e7a
Comment 36 Ed Morley [:emorley] 2012-06-28 01:08:40 PDT
https://hg.mozilla.org/mozilla-central/rev/c408dd243e7a
Comment 37 Alex Keybl [:akeybl] 2012-07-16 07:49:07 PDT
Should this web regression fix be considered for uplift to FF15? What's the risk profile? Please nominate for approval if low risk and/or high reward.
Comment 38 David Mandelin [:dmandelin] 2012-07-17 16:18:28 PDT
(In reply to Alex Keybl [:akeybl] from comment #37)
> Should this web regression fix be considered for uplift to FF15? What's the
> risk profile? Please nominate for approval if low risk and/or high reward.

The bug's kind of old by now, so if it affected the web I'd expect we'd have heard by now. I think this was about something in a sandbox for social API or other chrome code not working. Somebody correct me if I'm wrong.
Comment 39 Mark Hammond [:markh] 2012-07-17 17:24:21 PDT
(In reply to David Mandelin [:dmandelin] from comment #38)
> (In reply to Alex Keybl [:akeybl] from comment #37)
> > Should this web regression fix be considered for uplift to FF15? What's the
> > risk profile? Please nominate for approval if low risk and/or high reward.
> 
> The bug's kind of old by now, so if it affected the web I'd expect we'd have
> heard by now. I think this was about something in a sandbox for social API
> or other chrome code not working. Somebody correct me if I'm wrong.

That is correct, and the feature this is being used for (socialapi) is being targeted at FF16 so this bug isn't blocking that.
Comment 40 Lukas Blakk [:lsblakk] use ?needinfo 2012-07-18 09:07:28 PDT
[Triage comment]

Given comment 39 I'm going to untrack this for 15 then and mark as wontfix. It can ride the trains.

Note You need to log in before you can comment on or make changes to this bug.