342098 - Refactor startup in extensions.js so that showView will only ever be called once

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

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

Spinoff from bug 307358. When updates are displayed during startup the previous view will be generated then the updates view will be generated. The same is true for updatecheck as well as when addDownloads is called during startup though I am unsure if the extra showView can be factored out for addDownloads due to potential timing issues.

Comment 1

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

Forgot to mention that we should also check if there are no updates to be installed and just close the window to cover the edgecase where the extensions datasource disappears out from under us

Comment 2

[Michael Wu [:mwu]]

Attachment: Cleanup code, immediately close notify dialog on no updates

Patches for other apps' prefs will be necessary.

Comment 3

[Michael Wu [:mwu]]

This patch tests fine with multi item xpis.

Comment 4

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

Comment on attachment 226729 [details] [diff] [review]
Cleanup code, immediately close notify dialog on no updates

>--- toolkit/mozapps/extensions/content/extensions.js	20 Jun 2006 05:08:30 -0000	1.72.2.25
>+++ toolkit/mozapps/extensions/content/extensions.js	23 Jun 2006 00:27:26 -0000
...
>@@ -541,38 +534,44 @@ function Startup() 
>   if ("arguments" in window) {
>     try {
>       var params = window.arguments[0].QueryInterface(Components.interfaces.nsIDialogParamBlock);
>+      showView("installs");
>       gDownloadManager.addDownloads(params);
>     }
>     catch (e) {
>-      if (window.arguments[0] == "updatecheck")
>-        checkUpdatesAll();
>-      else if (window.arguments[0] == "updates-only") {
>+      if (window.arguments[0] == "updates-only") {
>         gUpdatesOnly = true;
>         document.getElementById("viewGroup").hidden = true;
>         document.getElementById("extensionsView").setAttribute("norestart", "");
>         showView('updates');
The style in this file is to use double quotes

>         var addonsMsg = document.getElementById("addonsMsg");
>         addonsMsg.showMessage("chrome://mozapps/skin/extensions/question.png",
>                               getExtensionString("newUpdatesAvailableMsg"),
>                               null, null, true, null);
>         document.title = getExtensionString("newUpdateWindowTitle", [getBrandShortName()]);

>       }
>     }
>   }
...
>+  if (gUpdatesOnly && gExtensionsView.children.length == 0)
>+    window.close();
>
>   gPref.setBoolPref(PREF_UPDATE_NOTIFYUSER, false);
window.close has to come after setting the pref or it will do this again on next restart.

This looks good though I want to have a look at the updated patch before +'ing... I am going to test this a bit more and I'll submit patches for the Thunderbird, Calendar, and XULRunner prefs. Thanks Michael!

Comment 5

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

Attachment: Thunderbird patch

Pref change needed for EM changes

Comment 6

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

Attachment: XULRunner patch

Pref change needed for EM changes

Comment 7

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

Comment on attachment 226778 [details] [diff] [review]
XULRunner patch

XULRunner pref change for EM changes

Comment 8

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

Attachment: Sunbrd patch

Sunbird pref change for EM changes

Comment 9

[Michael Wu [:mwu]]

Attachment: Cleanup code, immediately close notify dialog on no updates, v2

Fixed style issue, moved window.close(); to the end of the function for clarity. (in practice, the pref does get toggled off even though we close the window before that line)

Comment 10

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

You are right regarding the pref being reset though while testing I had it appear a second time on startup (e.g. the brief flash) for some reason.

While testing I have opened the mgr during normal operation after this was closed on startup and had it display what I believe is the installs view without the Installations view button and with an empty list. Not sure what is going on there yet.

Comment 11

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

Comment on attachment 226798 [details] [diff] [review]
Cleanup code, immediately close notify dialog on no updates, v2

Thanks Michael. I did a clean build and I was unable to reproduce the blank screen.

Comment 12

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

I'd like to wait on the XULRunner pref patch getting approved before checking in to trunk

Comment 13

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

All 4 patches checked in to trunk

Comment 14

[pal-moz]

cause this ?
http://forums.mozillazine.org/viewtopic.php?p=2341717#2341717

Comment 15

[Michael Wu [:mwu]]

(In reply to comment #14)
> cause this ?
> http://forums.mozillazine.org/viewtopic.php?p=2341717#2341717

Indirectly, due to bug 342813.

Comment 16

[pal-moz]

(In reply to comment #15)
> Indirectly, due to bug 342813.
> 

thanks.

Comment 17

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

Comment on attachment 226798 [details] [diff] [review]
Cleanup code, immediately close notify dialog on no updates, v2

This patch provides a decent amount of cleanup, fixes to correctness, and fixes bug 342098 as well.

Comment 18

[Darin Fisher]

What does this fix, and how important is it that we take this for FF2?  Thanks!

Comment 19

[David Baron :dbaron:]

(In reply to comment #17)
> and fixes bug 342098 as well.

It clearly fixes itself.  What *else* does it fix? :-)

Comment 20

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

Sorry about that. This fixes bug 340400 as well.

This patch does the following:
1) uses a single chrome url for opening the EM ui for all operations.
 * Opens the window with the same dimensions for all views
 * I'd like this specifically for 2.0 so extension authors don't have to
   handle separately the pre 2.0 EM, 2.0 EM, and what will be the 3.0 EM.
2) generates the view only once when opening for an install. Without this patch when opening the ui for an extension install we generate the extensions view and then generate the view for the install. Though the additional time isn't great it is noticeable (around 2.7 seconds) with the 100 extensions I had installed while testing.
3) removes a decent amount of code that is no longer needed.

Comment 21

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

Checked in to MOZILLA_1_8_BRANCH