Figure out what to do about XHR.onuploadprogress

RESOLVED FIXED in Firefox 14



5 years ago
4 years ago


(Reporter: Ms2ger, Assigned: peterv)



Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox14+ fixed, firefox15+ verified)



(1 attachment, 1 obsolete attachment)



5 years ago
This attribute is well hidden on nsIJSXMLHttpRequest; so well hidden, in fact, that it got lost in the Paris Bindings.

Not sure if we want to keep it, but we should make a call one way or the other.
Requesting tracking for the moment; we don't want to accidentally ship "something" here without making a considered decision.
tracking-firefox14: --- → ?
Keywords: regression
I can think of two options:

1. Put the property back and make both onuploadprogress and 
   addEventListener("uploadprogress", ...) work, but make them warn when used.
   phase them out once we feel that we've given enough headsup.
2. Add warnings on aurora and maybe even beta branches *now* so that we have 12
   weeks of warnings before we ship with uploadprogress removed.
And if we choose to do 1, i'm ok with onuploadprogress being exposed in workers as long as we make setting it there throw.
I think #1 is what we should do, per the discussion in today's meeting.

There's been talk of being able to declare things mainthreadonly, so imho we should use that here.


5 years ago
tracking-firefox14: ? → +

Comment 5

5 years ago
I'm not sure who's on point for fixing this in FF14 - sending over to jst to help find an assignee.
Assignee: nobody → jst
Peter, can you look into this one? We need this for current Aurora.
Assignee: jst → peterv
[Triage Comment]

Merge of 14 to the Beta channel is coming up tomorrow. Is there any risk related to this issue if we are shipping 14 to a wider (beta) audience?
Yeah, we need to fix this, preferably before we ship a beta release.

Comment 9

5 years ago
Created attachment 629635 [details] [diff] [review]

Need to add a test.

Comment 10

5 years ago
Created attachment 629772 [details] [diff] [review]
Attachment #629635 - Attachment is obsolete: true
Attachment #629772 - Flags: review?(bzbarsky)
Comment on attachment 629772 [details] [diff] [review]


We should file a followup for removing this.
Attachment #629772 - Flags: review?(bzbarsky) → review+


5 years ago
Depends on: 761278

Comment 12

5 years ago
Filed bug 761278
Comment on attachment 629772 [details] [diff] [review]

[Triage Comment]
Approving for landing on mozilla-beta (now that we have migrated).  Will set tracking 15 on the bug filed for making sure this gets turned off again.
Attachment #629772 - Flags: approval-mozilla-beta+
status-firefox14: --- → fixed
status-firefox15: --- → affected
tracking-firefox15: --- → ?
Flags: in-testsuite+
Target Milestone: --- → mozilla16
Comment on attachment 629772 [details] [diff] [review]

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 740069 
User impact if declined: Possible extension (and less likely site) breakage
Testing completed (on m-c, etc.): Passes automated tests.
Risk to taking this patch (and alternatives if risky): Very low risk: just adds a property with a simple implementation to the WebIDL for XMLHttpRequest.
String or UUID changes made by this patch: None.
Attachment #629772 - Flags: approval-mozilla-aurora?
Comment on attachment 629772 [details] [diff] [review]

Approving low risk fix, and it's already green (for the most part) on mozilla-beta too.
Attachment #629772 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(In reply to Boris Zbarsky (:bz) from comment #16)
> String or UUID changes made by this patch: None.

This wasn't quite accurate, afaics - the patch added a string. And sure enough, this has already triggered questions on the l10n list.... it would have been good to give them a heads-up.

Comment 19

5 years ago
Grmpf, this is soo much easier to message when it can be posted before landing.

I'll allow it, weird edge on the web, string only showing in the error console.

This needs to land on aurora still, too?
Last Resolved: 5 years ago
Resolution: --- → FIXED
Yikes.  That's what I get for doing this stuff while half asleep.  :(

I can just drop all the string stuff from the patch for Aurora and back it out on Beta if preferred.  It's not essential to the patch; it would just delay the warning by a cycle (or two if we drop it from Aurora).  Axel, what would you prefer here?

Comment 22

5 years ago
Leave it as is, better to have the ability to localize it than not. Also, flod already localized it on beta, so we're not helping with a revert at this point.

Also, land with the string on aurora.
OK, that works.  Thanks!  And sorry for all the trouble.  :(
status-firefox15: affected → fixed
tracking-firefox15: ? → +

Comment 25

5 years ago
Verified as fixed on:
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:15.0) Gecko/20100101 Firefox/15.0
Mozilla/5.0 (Windows NT 6.1; rv:15.0) Gecko/20100101 Firefox/15.0
Build identifier: Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20100101 Firefox/15.0

XHR.onuploadprogress still works and the user gets a warning in the Error console about it being deprecated.
status-firefox15: fixed → verified
Component: DOM: Mozilla Extensions → DOM
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.