541571 - Show uninstallation progress

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Alex Pakhotin (:alexp)]

Current version of uninstaller does not have any UI, it just displays a message box to confirm the uninstallation, and reports with another message box at the end. There is no feedback to the user on what's happening after he/she agreed to remove Fennec and until it's done. We need to display at least some message during that process to let user know what's going on, or better a progress of uninstallation.

Comment 1

[Alex Pakhotin (:alexp)]

Attachment: Uninstaller UI

Added a simple dialog box showing uninstallation progress.

Comment 2

[Alex Pakhotin (:alexp)]

Attachment: Uninstaller UI v2

Improved interaction between the Installer and Uninstaller

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 424375 [details] [diff] [review]
Uninstaller UI v2

>+#define WM_DIALOGCREATED (WM_USER+1)

nit, spaces around the + (in both places... would be nice if this were in a header)

>+BOOL OnDialogCreated(HWND)
>+{
>+  if (!ShutdownFastStartService(g_sInstallPath))
>+  {
>+    //TODO: May need to handle this situation
>+
>+    //MessageBoxW(hWnd, L"Unable to shut down Fennec. Try to reset your device.",
>+    //            Strings.GetString(StrID_UninstallCaption), MB_OK|MB_ICONWARNING);
>+    //return FALSE;
>+  }
>+
>+  return TRUE;
>+}

remove the commented out MessageBoxW call and return

>+const int c_nMaxDirs = 25; // approximate number of dirs

this isn't ideal, but I guess we can live with it for now.  Please file a follow up bug so we can see how costly getting an actual count is

>   SendMessageToControl(IDC_CMB_PATH, CB_SETCURSEL, 0, 0 );
and
>+  PostMessage(m_hDlg, WM_DIALOGCREATED, 0, 0 );
and
>   if (nPathIndex >=0 )
and
>+  PostMessage(hDlg, WM_DIALOGCREATED, 0, 0 );

please fix the space before the close paran

>   SendMessageToControl( IDC_PROGRESS, PBM_SETPOS, (WPARAM)n, 0);
and after the open paran here

> #ifdef SHELL_AYGSHELL
you test for SHELL_AYGSHELL in a few places. Does this code even work if that's not defined? I suspect it doesn't in which case, please remove these ifdefs

Comment 4

[Alex Pakhotin (:alexp)]

Attachment: Patch v3

Code review changes.

Comment 5

[Alex Pakhotin (:alexp)]

Please note - this patch has to be applied after bug 533581, as it changes the
same parts of code.

Comment 6

[Brad Lassey [:blassey] (use needinfo?)]

pushed http://hg.mozilla.org/mozilla-central/rev/86df4904332f and http://hg.mozilla.org/releases/mozilla-1.9.2/rev/3ef047a84b70 and followed up with http://hg.mozilla.org/releases/mozilla-1.9.2/rev/c7a1da591f26 to fix 1.9.2 because bug 533542 hasn't landed there yet.  We should land that on trunk as well to keep them in sync.

Comment 7

[Aakash Desai [:aakashd]]

verified FIXED on builds:

Mozilla/5.0 (Windows; U; WindowsCE 5.2; en-US; rv:1.9.2.2pre) Gecko/20100208 Namoroka/3.6.2pre Fennec/1.0a4pre

Comment 8

[Marco Bonardo [:mak]]

what is the checkin-needed about?

Comment 9

[Tony Chung [:tchung]]

fix was for winmo. in-litmus-