862540 - window.status can no longer be set to anything, ever

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Joseph Brown]

Attachment: test.html

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:20.0) Gecko/20100101 Firefox/20.0
Build ID: 20130329030832

Steps to reproduce:

Nothing, firefox auto-upgraded


Actual results:

select box is not returning a value in jQuery 1.4.2

function onStatusChange(e){
 status = $("#status option:selected").text();
 alert(status);
};



Expected results:

There was a value jQuery javascript, since the last Firefox update, it is no-longer being returned.

Created a test html

Comment 1

[Joseph Brown]

I use Linux, customer uses windows XP, although it's not working on either.

Comment 2

[Josh Matthews [:jdm]]

Comment on attachment 738173 [details]
test.html

It would be more useful to link directly to the released version of jquery 1.4.2 rather than localhost.

Comment 3

[Joseph Brown]

Attachment: using jQuery 191

Added link to http://code.jquery.com/jQuery.1.9.1.min.js
exhibits same behaviour with current jQuery

Comment 4

[Joseph Brown]

This is new behaviour with the latest version of Firefox.

Comment 5

[Boris Zbarsky [:bzbarsky]]

So what I see in Firefox 19 is that the alerts say "undefined", while in Firefox 20 they say "".

And if I make "status" a var, then they say "undefined" in all versions.

So what's happening is that this code is writing (and reading) window.status.

Is the real issue that this stopped being replaceable in bug 823283 and now sets are being ignored?

We should probably allow mStatus to be set but just not use it for anything, when dom.disable_window_status_change is set, I'd think

Comment 6

[Boris Zbarsky [:bzbarsky]]

In particular, do we ever use the mStatus for anything?  If not, seems like letting web pages scribble on it should be fine...

Comment 7

[:Gavin Sharp [email: [e-mail]]]

dom.disable_window_status_change has been true in Firefox since the aviary days (https://bugzilla.mozilla.org/show_bug.cgi?id=22183#attach_158816), and we recently removed support non-default values of that pref in bug 842017.

I think we should just remove all of the relevant nsIWebBrowserChrome2/nsIXULBrowserWindow APIs since they're not actually used for anything.

Comment 8

[Boris Zbarsky [:bzbarsky]]

> dom.disable_window_status_change has been true in Firefox since the aviary days 

Yes, but until bug 823283 assigning to unqualified "status" just created a new global variable instead of trying to write window.status.

Sounds to me like writing to mStatus at least would be fine.  I'll hook that up.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

Yes, understood. For the immediate bug described here, let's do whatever is simplest to fix the change in behavior. But the point I was trying to make was that from my perspective we don't need mStatus or the window.status getter/setter at all - we should do whatever the minimum is for web compat. window.status/defaultStatus don't even seem to be specced in HTML5. I will file another bug for that.

Comment 10

[Boris Zbarsky [:bzbarsky]]

> window.status/defaultStatus don't even seem to be specced in HTML5.

defaultStatus is not.  status is.  It's at http://www.whatwg.org/specs/web-apps/current-work/multipage/browsers.html#the-window-object :

           attribute DOMString status;

Comment 11

[:Gavin Sharp [email: [e-mail]]]

I filed bug 862917 for removing some of that code, fwiw.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: Make window.status actually be settable (but have no effect unless the pref that no one ever sets is set).

Comment 13

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e5d4a350a668

Comment 14

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/e5d4a350a668

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 738887 [details] [diff] [review]
Make window.status actually be settable (but have no effect unless the pref that no one ever sets is set).

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 823283
User impact if declined: Some sites may not work right in Firefox (e.g. see http://stackoverflow.com/questions/16138781/firefox-20-vs-google-charts-getvalue-oddness )
Testing completed (on m-c, etc.): Passes tests and all
Risk to taking this patch (and alternatives if risky):  Risk is low.  This does
   change web-visible behavior, but it should be changing to something every
   other UA implements... 
String or IDL/UUID changes made by this patch:  None.


This is affecting some actual sites, so we should consider backporting this...

Comment 16

[Alex Keybl [:akeybl]]

Comment on attachment 738887 [details] [diff] [review]
Make window.status actually be settable (but have no effect unless the pref that no one ever sets is set).

Approving for Aurora, for sake of correctness given this is the desired path forward from engineering.

If this only helps users with a non-standard pref set, why do we think this needs to be uplifted to Beta s well?

Comment 17

[Boris Zbarsky [:bzbarsky]]

The pref setting is irrelevant to the web-facing behavior.  It just controls what happens in the UI, and never has any effect in Firefox because we have no UI for it.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a35a528a1f2c

Comment 19

[bhavana bajaj [:bajaj]]

Comment on attachment 738887 [details] [diff] [review]
Make window.status actually be settable (but have no effect unless the pref that no one ever sets is set).

Request to land asap for this to get into Fx21 beta4 which is going to build today.

Comment 20

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/releases/mozilla-beta/rev/37dfd55ea007

Comment 21

[u279076]

(In reply to Boris Zbarsky (:bz) from comment #15)
> Risk to taking this patch (and alternatives if risky):  Risk is low.  This
> does change web-visible behavior, but it should be changing to something every
> other UA implements... 

Boris, could you please elaborate on what you mean by web-visible behaviour? Are there any areas of risk not covered by your tests?

Comment 22

[Boris Zbarsky [:bzbarsky]]

> Boris, could you please elaborate on what you mean by web-visible behaviour?

Sure.  Without this patch, this script, in a webpage:

  status = "PASS";
  alert(status);

alerts the empty string.  With this patch it alerts the string "PASS".

The main area of risk is if a web page was somehow depending on setting window.status to not actually change its value from "", and was doing so by explicitly setting window.status (because if it set unqualified "status" and depended on that to not change, it would have been broken in Firefox 19).  Oh, and such a webpage would be broken in other browsers, so it would need to be depending on the behavior in Gecko-specific code...

Basically, the behavior change in this bug is that the old status setter did nothing while the new one just saves the string.  Either way the status getter returns the saved string (which was always empty in Firefox 20), and either way the passed-in value is converted to a string.

Does that help?

Comment 23

[u279076]

So it sounds like this is pretty low risk for regression and that the checked-in tests cover this scenario adequately. We could take a shotgun approach to testing top websites but I don't think that's likely to be a valuable use of our time. Do you think that's a fair and safe assessment?

Comment 24

[Boris Zbarsky [:bzbarsky]]

Yes, sounds right to me.

Comment 25

[u279076]

Thanks Boris. I'm flagging this bug [qa-] for verification given our assessment in the above comments.