311385 - Update Later button should not bring up an alert box

Status: RESOLVED DUPLICATE of bug 391598

(Toolkit :: Application Update, defect)

Description

[Lorenzo Colitti]

When an app update has been manually downloaded, the dialog box has three
buttons, Later, Back (greyed out) and Restart Firefox now.

If you click later, the dialog box doesn't close immediately but brings up
another window saying "The update you just downloaded will be installed the next
time you start Firefox."

This window should not come up: clicking Later should just silently dismiss the
dialog.

Steps to reproduce:

1. Wait for an update to be available (if you have the nightly channel, just
wait one day)
2. Click Help->Check for Updates
3. Click Download and Install now
4. Click Later

Expected results: the window disappears

Actual results: another window appears saying "The update you just downloaded
will be installed the next time you start Firefox"

Comment 1

[Ria Klaassen (not reading all bugmail)]

Maybe another button "Cancel update" would be fine, for I've heard more similar
complaints.

Comment 2

[Darin Fisher]

ben: can you comment on this and either confirm or wontfix this bug?

Comment 3

[Mike Connor [:mconnor]]

*** Bug 321611 has been marked as a duplicate of this bug. ***

Comment 4

[:Gavin Sharp [email: [e-mail]]]

*** Bug 339362 has been marked as a duplicate of this bug. ***

Comment 5

[Scott MacGregor]

Ben, can you comment on if you'd be ok with removing this alert? I can put together a patch.

Comment 6

[Andreas Morawietz]

Any news about it? 

Comment 7

[:deanis74]

Attachment: screenshot

Here's a screenshot clarifying how little value the confirmation dialog adds.

Comment 9

[Martijn Wargers (dead)]

Attachment: patch

I guess this would fix it, but I haven't tested it.

Comment 10

[:deanis74]

Would it make sense to change the buttons to be worded similarly?  To me, that would be "Restart Now" and "Restart Later".

Comment 11

[Martijn Wargers (dead)]

Yes, that would make sense to me. Perhaps, that should be file as a new bug, though.

I'm not sure how to test my patch, Check for updates... doesn't find any update in my debug build, it seems.

Comment 12

[Martijn Wargers (dead)]

Comment on attachment 292842 [details] [diff] [review]
patch

It seems obvious to me that this should work, but I haven't been able to test it.
Gavin, you have an idea of how to test this?

Comment 13

[Martijn Wargers (dead)]

Thanks to Nick on IRC, I could test it:
cf_nthomas>	hi mw22, for that update test you can save https://aus2.mozilla.org/update/1/Firefox/3.0b3pre/2007121505/WINNT_x86-msvc/en-US/nightly/update.xml to your hard drive, then make a pref app.update.url.override and put the file://.. to what you saved. Just be ever so careful about blowing away the updates dir before restarting your build

I tested the patch now, and it works as expected.

Comment 14

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 292842 [details] [diff] [review]
patch

Get ui+r on this, but yeah, seems obvious given Dean's screenshot. Wouldn't hurt to also change the button title while you're at it, as Dean suggests, your ui-reviewer might have thoughts on that.

Comment 15

[Martijn Wargers (dead)]

Comment on attachment 292842 [details] [diff] [review]
patch

Mike, also see comment 10 and comment 14 on renaming the buttons to something else.
What is your decision on that?

Comment 16

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 292842 [details] [diff] [review]
patch

Let's get this in as-is. I'd support a follow on patch that changes the buttons to "Restart Now" and "Restart Later" if we can get it in before code freeze. I'll file a follow-up.

Comment 17

[Mike Beltzner [:beltzner, not reading bugmail]]

Reed: can you check this in, and as it lands switch the buttons to say "Restart Now" and "Restart Later"? ui-r+a=beltzner on those changes.

Comment 18

[Reed Loden [:reed]]

Bug 391598 took care of this bug already, so I'm marking this as FIXED even though no patch was actually landed from this bug.

(In reply to comment #17)
> Reed: can you check this in, and as it lands switch the buttons to say "Restart
> Now" and "Restart Later"? ui-r+a=beltzner on those changes.

I didn't do this, as the text has changed since this screenshot was taken. See bug 391598. If you want to change the text, file another bug?

Comment 20

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #18)
> I didn't do this, as the text has changed since this screenshot was taken. See
> bug 391598. If you want to change the text, file another bug?

Heh, I *thought* I'd seen a bug go by to change the dialog to say "Not Now". No, I think the text is fine as bug 391598 made it.