Ian Bicking (:ianbicking) Reporter
	Description • 12 years ago

On a page that uses jQuery (for instance http://jquery.com/) when I use "$" or "jQuery" I get an object or wrapper that is incorrect/incomplete.  $('selector') works, but for instance $.ajax is undefined.  Autocompletion of these properties does work.

When I enter "$" (or "jQuery") into the console it shows "> function () {[native code]}" so I'm guessing some (broken) wrapper is in effect.

Tested on Nightly, 16.0a1 (2012-07-05)

	Kevin Dangoor
	Comment 1 • 12 years ago

This behavior is not in FF13 but is in 15. This will affect a lot of users.

	Mihai Sucan [:msucan]
	Comment 2 • 12 years ago

This seems to be caused by bug 726949. I pulled a revision before and after that patch landed. Problem shows when it is applied.

bz / bholley: can you please look into this regression? I am not experienced with the code that changed in that bug. Thank you!

	Bobby Holley (:bholley)
	Comment 3 • 12 years ago

Appears that, from the web console, $ and window.$ are different objects.

Object.getPrototypeOf($) === this.Function.prototype
Object.getPrototypeOf(window.$) === window.Function.prototype

	Bobby Holley (:bholley)
	Comment 4 • 12 years ago

Oh right, because it's a bound method.

	Bobby Holley (:bholley)
	Comment 5 • 12 years ago

Ok, so you can reproduce this with a simple testcase outside of the web console:

$ = $.bind(window);

This is exactly analogous to what we're doing under the hood here. So the first step is for someone to figure out why that breaks jquery.

	Mihai Sucan [:msucan]
	Comment 6 • 12 years ago

bholley: thank you for looking into this issue.

This happens with any global function in the page. For example, put a <script>function foobarFn() { return true; }</script> the page.

then in the web console try:

window.foobarFn;
foobarFn;

The former works, the latter doesn't.

Also try:

<script>
function foobarFn() { return true; }

foobarFn.fooObj = { boom: "test" };
</script>

Try to evaluate foobarFn.fooObj. I get undefined. If I evaluate window.foobarFn.fooObj, I get the correct result.

The Web Console doesn't try to do anything specific with the globals. We just put the window object as the sandboxPrototype.

	Bobby Holley (:bholley)
	Comment 7 • 12 years ago

(In reply to Mihai Sucan [:msucan] from comment #6)
> This happens with any global function in the page. For example, put a
> <script>function foobarFn() { return true; }</script> the page.
> 
> then in the web console try:
> 
> window.foobarFn;
> foobarFn;
> 
> The former works, the latter doesn't.

Define 'works'. Do you mean that the function is decompiled? If so, I see what you mean. But both function correctly return true.

> Try to evaluate foobarFn.fooObj. I get undefined. If I evaluate
> window.foobarFn.fooObj, I get the correct result.

Ah! So that's the issue. Right, when we rebind the function, we lose any properties that were defined on that function.
 
> The Web Console doesn't try to do anything specific with the globals. We
> just put the window object as the sandboxPrototype.

Yes, but sandboxPrototype sure does something specific - see bug 726949.

I wonder if we can get away with using the original method as the prototype for the rebound method.

	Bobby Holley (:bholley)
	Comment 8 • 12 years ago

So, as usual, the prototype strategy would _almost_ work. If we prototyped bound methods to their underlying method, then we'd have |Object.getPrototypeOf($) === window.$|. So we could correctly access |$.ajax === window.$.ajax|. But if we invoked it ($.ajax(...)), the method would end up with the bound function ($) as the this-binding rather than the window.$, which may or may not be ok.

I'm starting to think that this whole rebinding thing is a bit dubious. Maybe instead of bound methods we could return a transparent wrapper with an overridden call() trap? That would allow us to get the this-binding right, but not expose us to this whole mess here. Boris, what do you think?

Also, unless I'm mistaken, this regression is in FF14, which is shipping in a week, yeah? I'm pretty sure the code freeze has passed, so it sounds like we're shipping this regression. :-(

	Mihai Sucan [:msucan]
	Comment 9 • 12 years ago

Yep, unfortunately, this is going in fx14.

Can't we write a js proxy object for the window object that we then give as the sandboxPrototype? It sounds like it could work, if I understand this correctly. Any property we'd try to access from the sandboxPrototype could go through it.


(In reply to Bobby Holley (:bholley) from comment #7)
> (In reply to Mihai Sucan [:msucan] from comment #6)
> > This happens with any global function in the page. For example, put a
> > <script>function foobarFn() { return true; }</script> the page.
> > 
> > then in the web console try:
> > 
> > window.foobarFn;
> > foobarFn;
> > 
> > The former works, the latter doesn't.
> 
> Define 'works'. Do you mean that the function is decompiled? If so, I see
> what you mean. But both function correctly return true.

works: the function decompiles, as you said.

	Bobby Holley (:bholley)
	Comment 10 • 12 years ago

(In reply to Mihai Sucan [:msucan] from comment #9)
> Yep, unfortunately, this is going in fx14.
> 
> Can't we write a js proxy object for the window object that we then give as
> the sandboxPrototype? It sounds like it could work, if I understand this
> correctly. Any property we'd try to access from the sandboxPrototype could
> go through it.

Well, we did that, except it was a C++ proxy. The issue here is that we might need a _second_ proxy. The current proxy rebinds all methods it returns to use the window object as the |this| parameter. But this obviously isn't transparent enough.

A scripted proxy _might_ also work (though scripted proxies are less transparent). More importantly though, any other consumers of sandboxPrototype wouldn't get to take advantage of it. So the fix should probably be in C++.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 11 • 12 years ago

Well, ideally we'd only rebind XPConnect and quickstub methods, since those are the ones that can't deal, right?

We could handle non-quickstubbed ones by checking for the native being XPC_WN_CallMethod, but doing that for quickstubs is too much pain, of course.  And similar for DOM bindings.

For methods, I wonder what the obj in the property descriptor will be....  Building now to check.

As far as comment 8, I guess we could do that.  My proxy fu is too weak to tell for sure.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 12 • 12 years ago

OK, so the obj in the property descriptor is, unsurprisingly, a proxy.

	Bobby Holley (:bholley)
	Comment 14 • 12 years ago

I'm probably the best person to review this. Is it ready?

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 15 • 12 years ago

Comment on attachment 640019 [details] [diff] [review]
Instead of using bound functions for the functions we get off the sandbox proto, use a function proxy.  That allows property gets on the functions to get through.

Uh....  I thought I requested review from you when I uploaded it!  Yes, it's ready. Afaict.  Still cycling on try.

	Alex Keybl [:akeybl]
	Comment 16 • 12 years ago

Does a forward fix make the most sense here? Perhaps we should just consider backing out bug 726949 (and bug 622301?) from Beta for sake of risk mitigation.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 17 • 12 years ago

I think backing out bug 622301 might also involve backing out (or at least turning off) all the various DOM bindings stuff that happened in 14.  I suspect that that's riskier than this targeted fix.  For one thing, it affects web-facing code, whereas this patch only affects sandboxes....

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 18 • 12 years ago

Try looks good.

	Bobby Holley (:bholley)
	Comment 19 • 12 years ago

Comment on attachment 640019 [details] [diff] [review]
Instead of using bound functions for the functions we get off the sandbox proto, use a function proxy.  That allows property gets on the functions to get through.


>+// A proxy handler that lets us wrap callables and invoke them with
>+// the correct this object, while forwarding all other operations down
>+// to them directly.
>+class SandboxCallableProxyHandler : public js::AbstractWrapper {

This should inherit IndirectProxyHandler, per a patch that just landed in bug 703537. It could also inherit IndirectWrapper (IndirectProxyHandler + enter/leave traps), but we've recently decided that IndirectWrapper is useless and should be removed.


This might also involve changing a few cases of "wrappedObject" to "GetProxyTarget". The basic idea is that now we have an inheritance chain of 3 classes. BaseProxyHandler is generic, with fundamental traps as pure virtual and derived traps in terms of fundamental traps. IndirectProxyHandler : public BaseProxyHandler implements the fundamental traps to forward to a target object. DirectProxyHandler : public IndirectProxyHandler reimplements the derived traps to forward directly.

We're making this change to better match the ES5 direct proxies spec, which includes the notion of a target object. So now the only unique thing about wrappers are the policy enforcement traps. We might end up renaming them at this point. Sorry this stuff is in flux at the moment. :-(

>+public:
>+    SandboxCallableProxyHandler() : js::AbstractWrapper(0)
>+    {
>+    }
>+
>+    virtual bool call(JSContext *cx, JSObject *proxy, unsigned argc,
>+                      Value *vp);
>+};
>+
>+bool
>+SandboxCallableProxyHandler::call(JSContext *cx, JSObject *proxy, unsigned argc,
>+                                  Value *vp)
>+{
>+    // We forward the call to our underlying callable.  The callable
>+    // to forward to can be gotten via GetProxyCall.  The object it's
>+    // effectively bound to can be gotten via GetProxyExtra(0).
>+    return JS::Call(cx, js::GetProxyExtra(proxy, 0), js::GetProxyCall(proxy),
>+                    argc, JS_ARGV(cx, vp), vp);
>+}

I think we can do even better here. I think we should avoid rebinding |this| if the function was invoked without a |this| object. In fact, I think we should _only_ rebind if |this| is equal to the sandbox global. This seems to me like it would buy us 100% transparency - the objects would behave as usual, _except_ in the precise cases where we want to rebind |this|.


>+
>+static SandboxCallableProxyHandler sandboxCallableProxyHandler;
>+
>+static JSObject*
>+BindCallable(JSContext *cx, JSObject *callable, JSObject *targetObj)

Given my suggestion to bind conditionally, this isn't a perfect function name. But it's probably fine. Also, |targetObj| might be better as |thisObj| or something.

>+{
>+    MOZ_ASSERT(JS_ObjectIsCallable(cx, callable));
>+    // Our proxy is wrapping the callable.  So we need to use the
>+    // callable as the private.  We also use it as the parent (XXXbz
>+    // is there a better choice?)

Maybe the SandboxProxyHandler? I don't think it really matters.

and we need to pass it in as the
>+    // "call" so we get a function proxy.
>+    //
>+    // The object we're bound to goes in the proxy extra slot.
>+    JSObject* proxy =  js::NewProxyObject(cx, &sandboxCallableProxyHandler,
>+                                          ObjectValue(*callable), nsnull,
>+                                          callable, callable);
>+    if (!proxy) {
>+        return nsnull;
>+    }
>+
>+    js::SetProxyExtra(proxy, 0, ObjectValue(*targetObj));
>+    return proxy;
>+}
>+
> template<typename Op>
> bool BindPropertyOp(JSContext *cx, JSObject *targetObj, Op& op,
>                     PropertyDescriptor *desc, jsid id, unsigned attrFlag)
> {
>     if (!op) {
>         return true;
>     }
> 
>@@ -3074,17 +3121,17 @@ bool BindPropertyOp(JSContext *cx, JSObj
>     } else {
>         // We have an actual property op.  For getters, we use 0
>         // args, for setters we use 1 arg.
>         uint32_t args = (attrFlag == JSPROP_GETTER) ? 0 : 1;
>         func = GeneratePropertyOp(cx, desc->obj, id, args, op);
>         if (!func)
>             return false;
>     }
>-    func = JS_BindCallable(cx, func, targetObj);
>+    func = BindCallable(cx, func, targetObj);
>     if (!func)
>         return false;
>     op = JS_DATA_TO_FUNC_PTR(Op, func);
>     desc->attrs |= attrFlag;
>     return true;
> }
> 
> extern JSBool
>@@ -3127,17 +3174,17 @@ xpc::SandboxProxyHandler::getPropertyDes
>         return false;
>     if (desc->setter != xpc::holder_set &&
>         desc->setter != XPC_WN_Helper_SetProperty &&
>         !BindPropertyOp(cx, obj, desc->setter, desc, id, JSPROP_SETTER))
>         return false;
>     if (desc->value.isObject()) {
>         JSObject* val = &desc->value.toObject();
>         if (JS_ObjectIsCallable(cx, val)) {
>-            val = JS_BindCallable(cx, val, obj);
>+            val = BindCallable(cx, val, obj);
>             if (!val)
>                 return false;
>             desc->value = ObjectValue(*val);
>         }
>     }
> 
>     return true;
> }

>diff --git a/js/xpconnect/tests/chrome/test_bug771429.xul b/js/xpconnect/tests/chrome/test_bug771429.xul


>+      function f() {}
>+      f.x = 2;
>+      var Cu = Components.utils;
>+      var s = new Cu.Sandbox(window, { sandboxPrototype: window } );
>+      try {
>+        var t = Cu.evalInSandbox('f.x', s);
>+        is(t, 2, "Should have gotten the right thing back");
>+      } catch (e) {
>+        ok(false, "Should not get an exception: " + e);
>+      }

Can you also add |f.g = function() { return this; }| and |is(Cu.evalInSandbox('f.g()', s), f)| to make sure things work properly apropos comment 8?

And also, assuming we go with the more fine-grained rebinding approach I suggested, some test coverage for that would be good as well.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 20 • 12 years ago

> per a patch that just landed in bug 703537.

That doesn't help for aurora and beta, does it?  I would like to focus on a patch that works there, in addition to something for m-c.

> In fact, I think we should _only_ rebind if |this| is equal to the sandbox global.

I'll take a look.  Time is kinda tight for getting this into beta in the next two hours here.

> Maybe the SandboxProxyHandler?

As the parent?  It's totally the wrong sort of object, no?  Like "not a JSObject".  Unless you meant the sandboxproxy itself, not its handler?

	Bobby Holley (:bholley)
	Comment 21 • 12 years ago

(In reply to Boris Zbarsky (:bz) from comment #20)
> > per a patch that just landed in bug 703537.
> 
> That doesn't help for aurora and beta, does it?  I would like to focus on a
> patch that works there, in addition to something for m-c.

Yeah, sure. This patch won't apply to m-c though.

> > In fact, I think we should _only_ rebind if |this| is equal to the sandbox global.
> 
> I'll take a look.  Time is kinda tight for getting this into beta in the
> next two hours here.

Oh, I thought the ship had already sailed there. If not, by all means, r=bholley on anything that improves the current situation. Though this should be a pretty easy change.


> > Maybe the SandboxProxyHandler?
> 
> As the parent?  It's totally the wrong sort of object, no?  Like "not a
> JSObject".  Unless you meant the sandboxproxy itself, not its handler?

Er, yes, sandboxproxy. But either was is fine. AFAICT. I wouldn't bother with it.

	Bobby Holley (:bholley)
	Comment 23 • 12 years ago

Comment on attachment 640236 [details] [diff] [review]
Patch for beta


>+class SandboxCallableProxyHandler : public js::AbstractWrapper {

As noted on IRC, we should just be using js::Wrapper here.

>+bool
>+SandboxCallableProxyHandler::call(JSContext *cx, JSObject *proxy, unsigned argc,
>+                                  Value *vp)
>+{
>+    // We forward the call to our underlying callable. The callable to forward
>+    // to can be gotten via GetReservedSlot(proxy, JSSLOT_PROXY_CALL).  If our
>+    // this object is hanging out in GetProxyExtra(0), we rebind to the object
>+    // in GetProxyExtra(1).
>+    if (JS_THIS(cx, vp) == js::GetProxyExtra(proxy, 0)) {
>+        return JS::Call(cx, js::GetProxyExtra(proxy, 1),
>+                        js::GetReservedSlot(proxy, JSSLOT_PROXY_CALL),
>+                        argc, JS_ARGV(cx, vp), vp);
>+    }
>+
>+    // Just forward the call directly
>+    return JS::Call(cx, JS_THIS_VALUE(cx, vp),
>+                    js::GetReservedSlot(proxy, JSSLOT_PROXY_CALL), argc,
>+                    JS_ARGV(cx, vp), vp);

Just store |thisVal| as a local variable and avoid duplicating the call to JS::Call?

>+}
>+
>+static SandboxCallableProxyHandler sandboxCallableProxyHandler;
>+
>+// Wrap a callable such that if we're called with oldThisObj as the
>+// "this" we will instead call it with newThisObj as the this.
>+static JSObject*
>+WrapCallable(JSContext *cx, JSObject *callable, JSObject *oldThisObj,
>+             JSObject *newThisObj, JSObject *parentObj)
>+{
>+    MOZ_ASSERT(JS_ObjectIsCallable(cx, callable));
>+    // Our proxy is wrapping the callable.  So we need to use the
>+    // callable as the private.  We use the given parentObj as the
>+    // parent.
>+    //
>+    // We need to pass the given callable in as the "call" so we get a
>+    // function proxy.
>+    //
>+    // The oldThisObj object goes in the 0th proxy extra slot.
>+    //
>+    // The newThisObj goes in the 1st proxy extra slot.
>+    JSObject* proxy =  js::NewProxyObject(cx, &sandboxCallableProxyHandler,
>+                                          ObjectValue(*callable), nsnull,
>+                                          parentObj, callable);

Should we also pass the |callable| as a construct hook here rather than leaving it null?

>+    if (!proxy) {
>+        return nsnull;
>+    }
>+
>+    js::SetProxyExtra(proxy, 0, ObjectValue(*oldThisObj));
>+    js::SetProxyExtra(proxy, 1, ObjectValue(*newThisObj));
>+    return proxy;
>+}
>+
>+bool BindPropertyOp(JSContext *cx, JSObject *oldThisObj, JSObject*newThisObj,

Spacing?

> extern JSBool
>@@ -3123,28 +3188,34 @@ xpc::SandboxProxyHandler::getPropertyDes
>     // the "vp is prefilled with the value in the slot" behavior that property
>     // ops can in theory rely on, but our property op forwarder doesn't know how
>     // to make that happen.  Since we really only need to rebind the DOM methods
>     // here, not rebindings holder_get and holder_set is OK.
>     //
>     // Similarly, don't mess with XPC_WN_Helper_GetProperty and
>     // XPC_WN_Helper_SetProperty, for the same reasons: that could confuse our
>     // access to expandos when we're not doing Xrays.
>+    //
>+    // Note: the proxy's parent is the sandbox global, which is exactly what we
>+    // want for oldThisObj for WrapCallable and BindPropertyOp.  We want |obj|
>+    // as the newThisObj.
>     if (desc->getter != xpc::holder_get &&
>         desc->getter != XPC_WN_Helper_GetProperty &&
>-        !BindPropertyOp(cx, obj, desc->getter, desc, id, JSPROP_GETTER))
>+        !BindPropertyOp(cx, JS_GetParent(proxy), obj, desc->getter, desc, id,
>+                        JSPROP_GETTER, proxy))
>         return false;
>     if (desc->setter != xpc::holder_set &&
>         desc->setter != XPC_WN_Helper_SetProperty &&
>-        !BindPropertyOp(cx, obj, desc->setter, desc, id, JSPROP_SETTER))
>+        !BindPropertyOp(cx, JS_GetParent(proxy), obj, desc->setter, desc, id,
>+                        JSPROP_SETTER, proxy))
>         return false;
>     if (desc->value.isObject()) {
>         JSObject* val = &desc->value.toObject();
>         if (JS_ObjectIsCallable(cx, val)) {
>-            val = JS_BindCallable(cx, val, obj);
>+            val = WrapCallable(cx, val, JS_GetParent(proxy), obj, proxy);
>             if (!val)
>                 return false;
>             desc->value = ObjectValue(*val);

I would prefer to store oldThisObject and newThisObject directly as locals. This would also give us an opportunity to assert that oldThisObject has the sandbox global class.

r=bholley for m-a and m-b with those comments addressed.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 24 • 12 years ago

> Should we also pass the |callable| as a construct hook here rather than leaving it null?

I tried it both ways, but could not see a difference.  I guess I can pass it.

Applied the other changes.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 26 • 12 years ago

Comment on attachment 640246 [details] [diff] [review]
Beta patch updated to comments

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 726949 
User impact if declined: Incorrect evaluation of some expressions in web console
Testing completed (on m-c, etc.): Passes xpconnect tests.
Risk to taking this patch (and alternatives if risky): Risk should be low.  The
   other option is to just leave this unfixed for Firefox 14.
String or UUID changes made by this patch:

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 29 • 12 years ago

Note to self: for m-c Bobby would like me to get oldThisObj and newThisObj off the function proxy's parent (which is the sandbox proxy).  One is its parent, the other is its wrapped object.

	Alex Keybl [:akeybl]
	Comment 30 • 12 years ago

Comment on attachment 640246 [details] [diff] [review]
Beta patch updated to comments

[Triage Comment]
Approving based upon test coverage, the low risk profile, and developer impact.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 31 • 12 years ago

http://hg.mozilla.org/releases/mozilla-aurora/rev/b55b4ebc1326
http://hg.mozilla.org/releases/mozilla-beta/rev/e6a74fae7914

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 32 • 12 years ago

Eddy, Bobby,

On m-c trying to inherit from Wrapper gets me:

  note: unimplemented pure virtual method 'toBaseProxyHandler' in
  'SandboxCallableProxyHandler'

which makes sense given the jswrapper.h bits.  What _should_ I actually be inheriting from?

	Bobby Holley (:bholley)
	Comment 33 • 12 years ago

(In reply to Boris Zbarsky (:bz) from comment #32)
> which makes sense given the jswrapper.h bits.  What _should_ I actually be
> inheriting from?

DirectWrapper. We'll be fixing this up again shortly over in bug 772138 (sorry for all the confusion, we decided that we'd made things to complicated), but in the mean time DirectWrapper is the equivalent of the old Wrapper.

	Bobby Holley (:bholley)
	Comment 35 • 12 years ago

Comment on attachment 640607 [details] [diff] [review]
Instead of using bound functions for the functions we get off the sandbox proto, use a function proxy.  That allows property gets on the functions to get through.

>+    JSObject *thisObj = JS_THIS_OBJECT(cx, vp);
>+    if (!thisObj) {
>+        return false;
>+    }

It's valid to have an undefined this binding in ES5 strict mode:

"use strict"; (function() { alert(this); })()

Looks good otherwise.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 36 • 12 years ago

OK, then we have a problem: I don't know how to do the comparison we want here.

In particular, JS_THIS_VALUE will return JSVAL_VOID for global function calls, but JS_THIS and JS_THIS_OBJECT will do non-strict boxing.  And if non-strict boxing returns null that means it threw, and we have to bail out.

Any ideas?  It sounds like what we really want is to pass through stuff without any rebinding if callee is a strict-mode scripted function.  Or something.

	Bobby Holley (:bholley)
	Comment 37 • 12 years ago

Shouldn't we just do JS_THIS_VALUE and compare it with ObjectValue(*oldTarget)? That seems like it would implicitly handle the JSVAL_VOID case (in which I think we should forward along JSVAL_VOID as the this-binding).

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 38 • 12 years ago

Hmm.  That sorta happens to work because the first place JS_THIS_OBJECT is called is the quickstub, and at that point the callee global is the window.  I guess we could do that and add a test can call that good enough....

	Bobby Holley (:bholley)
	Comment 39 • 12 years ago

Doesn't the same case apply when we call a regular dom method without a |this| binding in strict-mode? it seems to me like we shouldn't have to be doing any special thinking about this in the sandbox proxy case - we should just be remapping the one thing we care about, and passing everything else as-is.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 42 • 12 years ago

> Doesn't the same case apply when we call a regular dom method without a |this| binding in
> strict-mode?

Normally this happens without crossing globals.  Here it involves a proxy that moves the call across globals without ever computing this, which is why it ends up working.

	Bobby Holley (:bholley)
	Comment 43 • 12 years ago

Comment on attachment 641188 [details] [diff] [review]
Instead of using bound functions for the functions we get off the sandbox proto, use a function proxy.  That allows property gets on the functions to get through.

r=bholley. Thanks for the interdiff.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 44 • 12 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/016584d9d91b

	Ryan VanderMeulen [:RyanVM]
	Comment 45 • 12 years ago

https://hg.mozilla.org/mozilla-central/rev/016584d9d91b

	Ioana (away)
	Comment 46 • 12 years ago

Verified as fixed on:
Mozilla/5.0 (X11; Linux x86_64; rv:14.0) Gecko/20100101 Firefox/14.0
Mozilla/5.0 (Windows NT 5.1; rv:14.0) Gecko/20100101 Firefox/14.0
Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20100101 Firefox/15.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:15.0) Gecko/20100101 Firefox/15.0
Mozilla/5.0 (Windows NT 5.1; rv:15.0) Gecko/20100101 Firefox/15.0