Closed Bug 551283 Opened 15 years ago Closed 15 years ago

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

Categories

(Toolkit :: Application Update, defect)

Product:
Toolkit
Component:
Application Update
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
status1.9.2 --- .9-fixed
status1.9.1 --- wontfix

People

(Reporter: mstange, Assigned: robert.strong.bugs)

References

Details

Attachments

(2 files, 2 obsolete files)

simple patch rev2
(deleted), patch
Dolske
: review+
Details | Diff | Splinter Review
1.9.2 patch
(deleted), patch
robert.strong.bugs
: review+
christian
: approval1.9.2.7-
Details | Diff | Splinter Review
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
Attached patch simple patch (obsolete) (deleted) — — Splinter Review
Attachment #431574 - Flags: review?(dolske)
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.
Attached patch simple patch rev2 (obsolete) (deleted) — — Splinter Review
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)
Attached patch simple patch rev2 (deleted) — — Splinter Review
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
Closed: 15 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
Attached patch 1.9.2 patch (deleted) — — Splinter Review
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 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?
Depends on: 576939
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.

Attachment

General

Creator:
Created:
Updated:
Size: