Closed Bug 342098 Opened 17 years ago Closed 17 years ago

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


(Toolkit :: Add-ons Manager, defect)

1.8 Branch
Not set





(Reporter: robert.strong.bugs, Assigned: mwu)



(Keywords: fixed1.8.1)


(4 files, 1 obsolete file)

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.
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
Assignee: nobody → michael.wu
Patches for other apps' prefs will be necessary.
Attachment #226729 - Flags: review?(robert.bugzilla)
This patch tests fine with multi item xpis.
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
>+++ 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!
Attachment #226729 - Flags: review?(robert.bugzilla) → review-
Pref change needed for EM changes
Attachment #226777 - Flags: review?(mscott)
Attached patch XULRunner patchSplinter Review
Pref change needed for EM changes
Attachment #226778 - Flags: review?(mattwillis)
Comment on attachment 226778 [details] [diff] [review]
XULRunner patch

XULRunner pref change for EM changes
Attachment #226778 - Attachment description: Sunbird patch → SULRunner patch
Attachment #226778 - Flags: review?(mattwillis) → review?(benjamin)
Attachment #226778 - Attachment description: SULRunner patch → XULRunner patch
Attached patch Sunbrd patchSplinter Review
Sunbird pref change for EM changes
Attachment #226779 - Flags: review?(mattwillis)
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)
Attachment #226729 - Attachment is obsolete: true
Attachment #226798 - Flags: review?(robert.bugzilla)
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.
Attachment #226777 - Flags: review?(mscott) → review+
Attachment #226779 - Flags: review?(mattwillis) → review+
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.
Attachment #226798 - Flags: review?(robert.bugzilla) → review+
I'd like to wait on the XULRunner pref patch getting approved before checking in to trunk
Attachment #226778 - Flags: review?(benjamin) → review+
All 4 patches checked in to trunk
Closed: 17 years ago
Resolution: --- → FIXED
(In reply to comment #14)
> cause this ?

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

Attachment #226798 - Flags: approval1.8.1?
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.
Flags: blocking-firefox2?
What does this fix, and how important is it that we take this for FF2?  Thanks!
(In reply to comment #17)
> and fixes bug 342098 as well.

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

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.
Blocks: 340400
Attachment #226798 - Flags: approval1.8.1? → approval1.8.1+
Checked in to MOZILLA_1_8_BRANCH
Keywords: fixed1.8.1
Flags: blocking-firefox2? → blocking-firefox2+
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.