743666 - Figure out what to do about XHR.onuploadprogress

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

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 [:bzbarsky]]

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]

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]

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 [:bzbarsky]]

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]]

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)]

Peter, can you look into this one? We need this for current Aurora.

Comment 7

[Lukas Blakk [:lsblakk] use ?needinfo]

[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 (Exited; not receiving bugmail, old account, do not use)]

Yeah, we need to fix this, preferably before we ship a beta release.

Comment 9

[Peter Van der Beken [:peterv]]

Attachment: v1

Need to add a test.

Comment 10

[Peter Van der Beken [:peterv]]

Attachment: v1

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 629772 [details] [diff] [review]
v1

r=me

We should file a followup for removing this.

Comment 12

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Filed bug 761278

Comment 13

[Lukas Blakk [:lsblakk] use ?needinfo]

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 14

[Boris Zbarsky [:bzbarsky]]

http://hg.mozilla.org/releases/mozilla-beta/rev/ee6af303bd9f

Comment 15

[Boris Zbarsky [:bzbarsky]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/e0b462a2ad9f

Comment 16

[Boris Zbarsky [:bzbarsky]]

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]

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]]

(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 [:Pike]]

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)]

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

Comment 21

[Boris Zbarsky [:bzbarsky]]

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 [:Pike]]

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 [:bzbarsky]]

OK, that works.  Thanks!  And sorry for all the trouble.  :(

Comment 24

[Boris Zbarsky [:bzbarsky]]

http://hg.mozilla.org/releases/mozilla-aurora/rev/fcf10dd798a1

Comment 25

[Ioana (away)]

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.