Last Comment Bug 743666 - Figure out what to do about XHR.onuploadprogress
: Figure out what to do about XHR.onuploadprogress
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla16
Assigned To: Peter Van der Beken [:peterv]
:
Mentors:
Depends on: 761278
Blocks: 740069
  Show dependency treegraph
 
Reported: 2012-04-09 07:14 PDT by :Ms2ger (⌚ UTC+1/+2)
Modified: 2013-04-04 13:53 PDT (History)
12 users (show)
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed
+
verified


Attachments
v1 (5.57 KB, patch)
2012-06-03 12:43 PDT, Peter Van der Beken [:peterv]
no flags Details | Diff | Splinter Review
v1 (7.43 KB, patch)
2012-06-04 07:30 PDT, Peter Van der Beken [:peterv]
bzbarsky: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description :Ms2ger (⌚ UTC+1/+2) 2012-04-09 07:14:35 PDT
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.
Comment 1 Boris Zbarsky [:bz] 2012-04-09 07:15:42 PDT
Requesting tracking for the moment; we don't want to accidentally ship "something" here without making a considered decision.
Comment 2 Jonas Sicking (:sicking) No longer reading bugmail consistently 2012-04-09 12:36:43 PDT
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.
Comment 3 Jonas Sicking (:sicking) No longer reading bugmail consistently 2012-04-09 12:37:20 PDT
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.
Comment 4 Boris Zbarsky [:bz] 2012-04-09 13:01:00 PDT
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.
Comment 5 Alex Keybl [:akeybl] 2012-05-02 15:45:24 PDT
I'm not sure who's on point for fixing this in FF14 - sending over to jst to help find an assignee.
Comment 6 Johnny Stenback (:jst, jst@mozilla.com) 2012-05-02 16:51:58 PDT
Peter, can you look into this one? We need this for current Aurora.
Comment 7 Lukas Blakk [:lsblakk] use ?needinfo 2012-06-03 10:50:13 PDT
[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?
Comment 8 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-06-03 10:59:11 PDT
Yeah, we need to fix this, preferably before we ship a beta release.
Comment 9 Peter Van der Beken [:peterv] 2012-06-03 12:43:47 PDT
Created attachment 629635 [details] [diff] [review]
v1

Need to add a test.
Comment 10 Peter Van der Beken [:peterv] 2012-06-04 07:30:07 PDT
Created attachment 629772 [details] [diff] [review]
v1
Comment 11 Boris Zbarsky [:bz] 2012-06-04 08:38:00 PDT
Comment on attachment 629772 [details] [diff] [review]
v1

r=me

We should file a followup for removing this.
Comment 12 :Ms2ger (⌚ UTC+1/+2) 2012-06-04 12:43:48 PDT
Filed bug 761278
Comment 13 Lukas Blakk [:lsblakk] use ?needinfo 2012-06-04 15:29:50 PDT
Comment on attachment 629772 [details] [diff] [review]
v1

[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.
Comment 16 Boris Zbarsky [:bz] 2012-06-04 18:32:28 PDT
Comment on attachment 629772 [details] [diff] [review]
v1

[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.
Comment 17 Lukas Blakk [:lsblakk] use ?needinfo 2012-06-04 19:49:02 PDT
Comment on attachment 629772 [details] [diff] [review]
v1

Approving low risk fix, and it's already green (for the most part) on mozilla-beta too.
Comment 18 Jonathan Kew (:jfkthame) 2012-06-05 01:54:12 PDT
(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 Axel Hecht [pto-Aug-30][:Pike] 2012-06-05 03:59:00 PDT
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?
Comment 20 Geoff Lankow (:darktrojan) 2012-06-05 06:22:42 PDT
https://hg.mozilla.org/mozilla-central/rev/e0b462a2ad9f
Comment 21 Boris Zbarsky [:bz] 2012-06-05 08:15:23 PDT
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 Axel Hecht [pto-Aug-30][:Pike] 2012-06-05 08:19:36 PDT
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.
Comment 23 Boris Zbarsky [:bz] 2012-06-05 08:21:04 PDT
OK, that works.  Thanks!  And sorry for all the trouble.  :(
Comment 25 Ioana (away) 2012-08-07 06:27:01 PDT
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.

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