695811 - Checking for updates logs two warnings about deprecated XMLHttpRequest attributes

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Whenever I open Nightly's About page (which then triggers an automatic check for application updates), two warnings get logged to the error console:

Warning: Use of XMLHttpRequest's progress events' position attribute is deprecated.
Source File: chrome://browser/content/aboutDialog.xul
Line: 0

Warning: Use of XMLHttpRequest's progress events' totalSize attribute is deprecated.
Source File: chrome://browser/content/aboutDialog.xul
Line: 0


Probably shouldn't be using deprecated attributes if it can be avoided.

Comment 1

[Robert Strong (they/them - no direct email)]

References: bug 615507 and bug 616672

Comment 2

[Robert Strong (they/them - no direct email)]

Attachment: patch rev1

I'll get someone from B2G to review this as well after this review.

Comment 3

[Brian R. Bondy [:bbondy]]

Comment on attachment 708451 [details] [diff] [review]
patch rev1

Review of attachment 708451 [details] [diff] [review]:
-----------------------------------------------------------------

Other than the below question everything looks good.

::: toolkit/mozapps/update/content/updates.js
@@ -617,5 @@
>       */
> -    onProgress: function(request, position, totalSize) {
> -      var pm = document.getElementById("checkingProgress");
> -      pm.mode = "normal";
> -      pm.value = Math.floor(100 * (position / totalSize));

Does this not need to be updated?
http://dxr.mozilla.org/mozilla-central/toolkit/mozapps/update/content/updates.xul.html#l45

Comment 4

[Robert Strong (they/them - no direct email)]

(In reply to Brian R. Bondy [:bbondy] from comment #3)
> Comment on attachment 708451 [details] [diff] [review]
> patch rev1
> 
> Review of attachment 708451 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Other than the below question everything looks good.
> 
> ::: toolkit/mozapps/update/content/updates.js
> @@ -617,5 @@
> >       */
> > -    onProgress: function(request, position, totalSize) {
> > -      var pm = document.getElementById("checkingProgress");
> > -      pm.mode = "normal";
> > -      pm.value = Math.floor(100 * (position / totalSize));
> 
> Does this not need to be updated?
> http://dxr.mozilla.org/mozilla-central/toolkit/mozapps/update/content/
> updates.xul.html#l45
No. Before this patch it would change the progressmeter to a normaal progressmeter and with the patch it just stays as an undetermined progressmeter. With the xml file being so small this won't make a noticeable difference to the user since it only changes after the download has started and then after the download is finished it moves to the next page.

Comment 5

[Brian R. Bondy [:bbondy]]

Comment on attachment 708451 [details] [diff] [review]
patch rev1

Review of attachment 708451 [details] [diff] [review]:
-----------------------------------------------------------------

r+ given that keeping it indeterminate is ok.

Comment 6

[Robert Strong (they/them - no direct email)]

Comment on attachment 708451 [details] [diff] [review]
patch rev1

This removes onprogress from the update check interface since it isn't needed and causes a deprecated warning. Could you review the B2G changes?

Comment 7

[Dave Hylands [:dhylands]]

(In reply to Robert Strong [:rstrong] (do not email) from comment #6)
> Comment on attachment 708451 [details] [diff] [review]
> patch rev1
> 
> This removes onprogress from the update check interface since it isn't
> needed and causes a deprecated warning. Could you review the B2G changes?

Be happy to. I probably won't get to it until tomorrow though.

Comment 8

[Dave Hylands [:dhylands]]

Comment on attachment 708451 [details] [diff] [review]
patch rev1

Review of attachment 708451 [details] [diff] [review]:
-----------------------------------------------------------------

Looks fine to me.

Comment 9

[Robert Strong (they/them - no direct email)]

Pushed to mozilla-inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/a1716a79f81b

Comment 10

[[e-mail]]

[Had you really wanted to show progress you could have switched to loaded and total instead.]

Comment 11

[Phil Ringnalda (:philor)]

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