552617 - A paused update can't be resumed when closing the window using a window manager control

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

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

We should handle this case the same as when hiding the window

Comment 1

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

Attachment: patch rev1

The button positioning prevents the code from using the cancel button and setting its label to Hide so I just added onWizardCancel... simple enough.

Comment 2

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

Hey Alex, it might be better to prompt the user as to whether they want to cancel instead when they close the window. Can I get your input on this?

Comment 3

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

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

Holding off on review until I get UX feedback

Comment 4

[Alex Faaborg [:faaborg] (Firefox UX)]

It makes sense to prompt the user on very large downloads that can't be resumed on exit (they might have forgotten that they had started the download, and don't really want to exit).  However in this case we are initiating the update download (right?), so we shouldn't warn the user about something that they aren't involved with.  If we started the update download the best behavior would be to fail silently and try again next time.

Comment 5

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

Talked with Alex about this and he suggested we cancel the download of the update without prompting if the user closes the window using the window manager control. This will delete the existing update and the process will start again during the next update check, etc.

Comment 6

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

Attachment: patch rev1

Comment 7

[Justin Dolske [:Dolske]]

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

>+    aus.pauseDownload();
>+    um.saveUpdates();

These (existing) APIs seem unfortunately named; pauseDownload() seems to really want to be cancelDownload(), similarly for saveUpdates() being clearUpdates(). Oh well.

Comment 8

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

agreed wholeheartedly about pauseDownload but saveUpdates does save the update metadata to disk... I'm just adding cleanup of the updates dir when the updates metadata states there isn't an active update.

Comment 9

[Justin Dolske [:Dolske]]

Ah, fair point.

Comment 11

[Emil Ivanov]

My Firefox 3.6 got stuck with the update to 3.6.2. Is there any way to workaround it or I have to download Firefox and install it manually?

Comment 12

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

Emil, with a default install

for Windows Vista and above type the following in the Start Menu search box and press return.
%LOCALAPPDATA%
Then navigate to \Mozilla\Firefox\Mozilla Firefox

for Windows prior to Windows Vista type the following in the Start Menu -> run box and press return.
%USERPROFILE%
Then navigate to \Local Settings\Application Data\Mozilla\Firefox\Mozilla Firefox

With Firefox closed delete all of the files inside the Mozilla Firefox directory.

Start Firefox and you should be able to check for / install updates

Comment 13

[Emil Ivanov]

(In reply to comment #12)
> With Firefox closed delete all of the files inside the Mozilla Firefox
> directory.
> 
> Start Firefox and you should be able to check for / install updates

I only changed the filename of active-update.xml and it worked.
Firefox recreated another file.
Im with 3.6.2 now, thanks a lot.

Comment 14

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

Attachment: patch as checked in

I talked this over with beltzner and he wanted it to just continue in the state it is in. I also discussed it with dolske and since it is just a subset of the original patch he gave me a verbal r+.

Comment 15

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

Pushed to mozilla-central
http://hg.mozilla.org/mozilla-central/rev/44a74653c7c5

I believe I can create tests for this but I am going to hold off on doing so until after the end of the quarter. Filed Bug 555132 for tracking the tests so they aren't forgotten

Comment 16

[Steffen Wilberg]

Comment on attachment 435092 [details] [diff] [review]
patch as checked in

>+    // Set _hiding to false to prevent onWizardCancel from cancelling the update
>+    // that is in progress.
>+    this._hiding = true;
This "false" looks pretty "true" to me...

Comment 17

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

Quite right and thanks for catching the comment mistake... I'll fix it sometime in the next couple of days

Comment 18

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

Pushed typo only fix
http://hg.mozilla.org/mozilla-central/rev/8722c089c3e6

Comment 19

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

Attachment: 1.9.2 patch

Simple patch that I'd like to get in Firefox 3.6.5.

Comment 20

[LegNeato]

Comment on attachment 449085 [details] [diff] [review]
1.9.2 patch

a=LegNeato for 1.9.2.6. Please land it on mozilla-1.9.2 default, not the relbranch.

Code freeze is tomorrow @ 11:59 pm PST.

Comment 21

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

Pushed to mozilla-1.9.2
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/7dca7be78aad