Suspicious and possibly unused code in updates.js updateListener.onProgress

RESOLVED FIXED

Status

()

Toolkit
Application Update
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: mstange, Assigned: rstrong)

Tracking

Trunk
Points:
---
Bug Flags:
in-testsuite -
in-litmus -

Firefox Tracking Flags

(status1.9.2 .9-fixed, status1.9.1 wontfix)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

8 years ago
http://hg.mozilla.org/mozilla-central/file/5f4acba26bdc/toolkit/mozapps/update/content/updates.js#l565

That function has this code:

  var pm = document.getElementById("checkingProgress");
  checkingProgress.setAttribute("mode", "normal");

checkingProgress is not defined in this function and pm is not used. Is this code called at all?
Thanks for catching that!
Assignee: nobody → robert.bugzilla
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Created attachment 431574 [details] [diff] [review]
simple patch
Attachment #431574 - Flags: review?(dolske)
(Reporter)

Comment 4

8 years ago
It might not matter much, but I'd prefer pm.mode = ... and pm.value = ...
Setting the value property will send an accessibility event, setting the attribute won't.

Were you able to trigger a "checkingProgress is undefined" kind of error?
Will do... I wasn't able to trigger it likely due to it completing the download before the event and I don't have the time to investigate this thoroughly atm with the Lorentz and Quarter 1 goals work.
Created attachment 431601 [details] [diff] [review]
simple patch rev2
Attachment #431574 - Attachment is obsolete: true
Attachment #431601 - Flags: review?(dolske)
Attachment #431574 - Flags: review?(dolske)
Attachment #431601 - Attachment is obsolete: true
Attachment #431601 - Flags: review?(dolske)
Created attachment 431602 [details] [diff] [review]
simple patch rev2
Attachment #431602 - Flags: review?(dolske)
Attachment #431602 - Flags: review?(dolske) → review+
Pushed to mozilla-central
http://hg.mozilla.org/mozilla-central/rev/6960c848f6e1
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
Created attachment 449083 [details] [diff] [review]
1.9.2 patch

Simple patch that I'd like to get this for Firefox 3.6.5.
Attachment #449083 - Flags: review+
Attachment #449083 - Flags: approval1.9.2.5?

Comment 10

8 years ago
Comment on attachment 449083 [details] [diff] [review]
1.9.2 patch

We will not be taking this for 3.6.6. Moving approval request forward.

If you disagree, send me an email.
Attachment #449083 - Flags: approval1.9.2.7?
Attachment #449083 - Flags: approval1.9.2.6-
Attachment #449083 - Flags: approval1.9.2.5?
Comment on attachment 449083 [details] [diff] [review]
1.9.2 patch

Resetting request so I get the approval email if this gets approved
Attachment #449083 - Flags: approval1.9.2.7?
Attachment #449083 - Flags: approval1.9.2.7?
Comment on attachment 449083 [details] [diff] [review]
1.9.2 patch

I'm going to add this to a rollup patch in Bug 576939 for 1.9.2
Attachment #449083 - Flags: approval1.9.2.8?
Fixed on 1.9.2 for Firefox 3.6.9 by bug 576939.
status1.9.1: --- → wontfix
status1.9.2: --- → .9-fixed
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.