98797 - "keep this window" in download progress dialog checkbox should be selectable

Status: VERIFIED FIXED

(Core Graveyard :: File Handling, defect)

Description

[sairuh (rarely reading bugmail)]

spun off from bug 91605, basically to undo the change there.

i chatted with benc about this: he didn't realize that the decision of
selecting/deselecting the "keep this window" checkbox would persist [across
sessions, even] in future instances of the download progress dialog.

repeating from 91650:

the current way/workaround to be able to select [turn back on] this checkbox is
to initiate a longish download --so that the progress dialog persists long
enough to allow the user to change it. clicking the Reset button doesn't forget
the decision for the progress dialog, --since Reset only works for remembering
the helper app dialog.

Comment 1

[sairuh (rarely reading bugmail)]

nominating for 0.9.5 --also, prolly add a relnote for 0.9.4.

Comment 2

[sairuh (rarely reading bugmail)]

also noticed that this kills keyboard accessibility --once this item becomes
disabled i cannot even click to bring a widget in focus [which would then allow
me to tab around]. at least on the Mac.

aaronl or bill, would you be able to fix this by backing out the stuff from bug
91605?

Comment 3

[Aaron Leventhal]

-> back to original owner

Comment 4

[Bill Law]

I've included a one-line patch for this bug in an attachment to bug 67803.  
There's also some other stuff done that might related to this bug.  Basically, 
when the download completes, focus switches to the Close button (but this 
checkbox remains enabled).

Comment 5

[sairuh (rarely reading bugmail)]

spam: over to File Handling.

Comment 6

[Bill Law]

fixed (along with bug 67803)

Comment 7

[Scott MacGregor]

re-opening for the branch. Esther and I just discovered the checkbox is disabled
on the branch. Looks like bill just checked the fix into the trunk. This
prevents the user from being able to determine if they want the progress dialog
to stay up after downloading content. This worked in 6.1. I'm not sure I
understand why someone disabled this checkbox. here's the line of JS that needs
to be removed:

In setupPostProgressUI (helperAppDldProgress.js
 keepProgressWindowUpBox.disabled = true;      

It looks like jag added this line when fixing 79889 which was to make sure the
progress dialog gets clipped. I'm not sure I understand why he  added this line
since disabling this widget should have had no effect  on clipping the dialog. 
                          

Comment 8

[Scott MacGregor]

Attachment: Bill's one line fix for the branch

Comment 9

[Bill Law]

See Sarah's initial comment on how it got disabled.

Will the nsbranch keyword change to nsbranch+ at some point?  That's what I'm 
looking for to know what has to be done ASAP.

Comment 10

[Scott MacGregor]

Peter's the lucky guy that can turn it to a nsBranch+ bug. I don't have the power.

Comment 11

[Peter Trudelle]

nsbranch+

behold my power

Comment 12

[Jaime Rodriguez, Jr.]

can we get some reviews, and see if this can get in before monday's build?

Comment 13

[Scott MacGregor]

Comment on attachment 53369 [details] [diff] [review]
Bill's one line fix for the branch

since I go this fix from Bill's checkin, I'll sr this patch.

Comment 14

[Jaime Rodriguez, Jr.]

Gonna PDT+ this one, pending positive review. Pls try and get it in before
Sunday evening, so it is in Monday's build.

Comment 15

[Jaime Rodriguez, Jr.]

Did this get in last night?

Comment 16

[Scott MacGregor]

sadly it didn't...it's still in need of an r=.....

Comment 17

[Jaime Rodriguez, Jr.]

rats! how soon can we get an r=?

Comment 18

[Blake Ross]

Umm...r=blake, it's a little surprising to me though that this is such an 
end-of-the-world stop-ship blocker, given that the checkbox only enables upon 
download completion, and I don't think that the number of users on broadband + 
the numbers downloading files so small they have no chance to check the checkbox 
+ the number of users dying to check the checkbox is so large that it's so 
important. But anyways...

Comment 19

[Blake Ross]

enables -> disables obviously. spam is yummy.

Comment 20

[Jaime Rodriguez, Jr.]

Pls get this in tonight, for tomorrow's build - PDT+, we are respinning for
another bug, assuming you get a positive review.

Comment 21

[lchiang]

Jaime - I believe all the reviews are there (r=blake, sr=mscott)

Comment 22

[Jaime Rodriguez, Jr.]

who can check this in for bill?

Comment 23

[Scott MacGregor]

I checked this into the branch tonight for Bill.

Comment 24

[sairuh (rarely reading bugmail)]

vrfy fixed using 0.9.4-branch comm bits on linux, winNT and mac os 10.1. adding
vtrunk kw.

Comment 25

[sairuh (rarely reading bugmail)]

vrfy fixed using 2001.10.22.1x-trunk comm bits on mac 10.1, winNT and linux rh6.2.