442167 - [SeaMonkey] XPI install misses "installation complete" dialog after application restart

Status: RESOLVED FIXED

(SeaMonkey :: Startup & Profiles, defect)

Description

[Serge Gautherie (:sgautherie)]

Today, I installed 'Leak Monitor Extension' v0.4.2:

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.0.1pre) Gecko/2008061806 GranParadiso/3.0.1pre] (nightly) (W2Ksp4)

After restart, I got a new dialog reporting "installation complete".

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.0.1pre) Gecko/2008062602 SeaMonkey/2.0a1pre] (nightly) (W2Ksp4)

No such dialog after restart.

I don't know if this is because I had to modify the .xpi: I doubt it;
or because SeaMonkey misses this extra step: I guess.

Comment 1

[Tony Mechelynck [:tonymec]]

I don't remember ever seeing that kind if behaviour in Fx2 or Sm2, unless specifically provided by the extension, so I suppose it's some kind of Fx3 "novelty".

Now I'm not sure this kind of "enhancement" is desirable on SeaMonkey (which goes for the "no-nonsense" look & feel as opposed to Fx/Tb's "flashiness"). KaiRo, what do you think?

Comment 2

[Serge Gautherie (:sgautherie)]

In this case, I see this more as a "security" reminder: no "unexpected/forgotten" install.
Of course, we could always want a (hidden) preference to disable it.

Comment 3

[Robert Kaiser]

This is a feature of the toolkit's extension (add-ons) manager, we should have it automatically, don't we?

Comment 4

[Serge Gautherie (:sgautherie)]

I would think so ... but I have not (yet) searched for the involved code.
Maybe I'll do some more tests too...
(Maybe SeaMonkey misses a startup hook or something to trigger this ?)

Comment 5

[Robert Kaiser]

Serge, yes, it's possible we miss a startup hook or such, you'll probably find out in the bug that introduced the functionality (or you can ask Mossop on IRC).

Comment 6

[Serge Gautherie (:sgautherie)]

Confirmed:
FF startup hook is in </browser/components/nsBrowserGlue.js>.
SM needs a port to its </suite/common/src/nsSuiteGlue.js>.
(TB bug is bug 422029.)

Comment 7

[Serge Gautherie (:sgautherie)]

FF bug 408115 depended on FF bug 411207;
but SM actually misses the whole "session restore" feature: bug 36810.

Comment 8

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) <nsSuiteGlue.js>

This patch ports bug 408118 FF code to SM.
(Waiting for bug 36810 before looking for reviews.)

We may want to the port CSS rules too:
<http://mxr.mozilla.org/mozilla/search?string=newAddon&case=on&find=%2Fextensions%5C.css%24&filter=%5E%5B%5E%5C0%5D*%24&tree=mozilla>
<https://bugzilla.mozilla.org/attachment.cgi?id=308041&action=diff>
<https://bugzilla.mozilla.org/attachment.cgi?id=308101&action=diff>

Comment 9

[Robert Kaiser]

Please don't reintroduce Ci/Cc, we have carefully worked to remove those shorthands from that file.

Comment 10

[Robert Kaiser]

oh, and the CSS rules are in toolkit, they don't concern us, we are using *stripe for toolkit theming anyway in the default theme (and Modern currently falls back to that for mozapps as it doesn't define any of it right now).

Comment 11

[[e-mail]]

Why can't we do this as part of final-ui-startup?

Comment 12

[Robert Kaiser]

Serge, can you revive this patch in a way that addresses the last few comments? Would surely be nice to have this in now.

Comment 13

[Robert Kaiser]

This is nothing I'd hold off the release for, but I think we should still take it if the patch can be revived.

Serge?

Comment 14

[Serge Gautherie (:sgautherie)]

Attachment: (Av1a) Port bug 408115

Av1, with comment 9 suggestion(s).

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.3a1pre) Gecko/20090919 SeaMonkey/2.1a1pre] (nightly) (W2Ksp4)

(In reply to comment #11)
> Why can't we do this as part of final-ui-startup?

I tried: the dialog is opened too, but before (= in the background) the browser shows up :-/

***

Fwiw, maybe we could remove this (2) observer when the event(s) is received instead of (or in addition to) at shutdown?

Comment 15

[[e-mail]]

(In reply to comment #14)
> (In reply to comment #11)
> > Why can't we do this as part of final-ui-startup?
> I tried: the dialog is opened too, but before (= in the background) the browser
> shows up :-/
That's assuming that we're opening the browser... we might not be!

Comment 16

[[e-mail]]

Comment on attachment 401678 [details] [diff] [review]
(Av1a) Port bug 408115

>+      case "sessionstore-windows-restored":
>+        this._onBrowserStartup();
>+        break;
I just tried and this has no effect until you open a browser window.

>+      const args = Components.classes["@mozilla.org/supports-array;1"]
>+                             .createInstance(Components.interfaces.nsISupportsArray);
Please use an nsIMutableArray instead.

>+      str.data = "";
Isn't an empty string the default anyway?

>+      const EMFEATURES = "chrome,menubar,extra-chrome,toolbar,dialog=no,resizable";
Please use the same EMFEATURES as tasksOverlay.xul does.

Comment 17

[Serge Gautherie (:sgautherie)]

(In reply to comment #16)

> >+      const args = Components.classes["@mozilla.org/supports-array;1"]
> >+                             .createInstance(Components.interfaces.nsISupportsArray);
> Please use an nsIMutableArray instead.

I tried with
{
      const args = Components.classes["@mozilla.org/array;1"]
                             .createInstance(Components.interfaces.nsIMutableArray);
+
      args.AppendElement(str, false);
}
but it stops working and I don't know (how to find out) why.

> >+      str.data = "";
> Isn't an empty string the default anyway?

It looks like so.  At least, I confirmed it still works without this line.

Comment 18

[Serge Gautherie (:sgautherie)]

(In reply to comment #17)
> {
>       args.AppendElement(str, false);
> }
> but it stops working and I don't know (how to find out) why.

Bah: it needs s/A/a/ !

Comment 19

[Serge Gautherie (:sgautherie)]

Attachment: (Av2) Port bug 408115, but use final-ui-startup + alwaysRaised [Checkin: See comment 21]

Av1a, with comment 16 suggestion(s).

That's what I found: not perfect, but hopefully acceptable until bug 447570.

Comment 20

[[e-mail]]

Comment on attachment 402738 [details] [diff] [review]
(Av2) Port bug 408115, but use final-ui-startup + alwaysRaised
[Checkin: See comment 21]

Heh, this is going to annoy anyone who's already installed extensions into SeaMonkey, because EM has been silently tracking them all ;-)

>+  // If new add-ons were installed during startup, open the add-ons manager.
>+  _openEM: function()
I can see why you might not want to include this code in onProfileStartup, but in that case you should call it something more memorable e.g. _checkForNewAddons:

>+    // 'alwaysRaised' makes sure it stays in the foreground (though unfocused)
I don't think that works on Linux, but I guess they'll just lose out.

Comment 21

[Serge Gautherie (:sgautherie)]

Comment on attachment 402738 [details] [diff] [review]
(Av2) Port bug 408115, but use final-ui-startup + alwaysRaised
[Checkin: See comment 21]


http://hg.mozilla.org/comm-central/rev/b22c572189c9
Av2, with comment 20 suggestion(s).

(In reply to comment #20)

> Heh, this is going to annoy anyone who's already installed extensions into
> SeaMonkey, because EM has been silently tracking them all ;-)

Indeed.  KaiRo: we may want to 'relnote' this?

> >+    // 'alwaysRaised' makes sure it stays in the foreground (though unfocused)
> I don't think that works on Linux, but I guess they'll just lose out.

It looks like you're right: bug 453274.

Comment 22

[Robert Kaiser]

(In reply to comment #21)
> (From update of attachment 402738 [details] [diff] [review])
> > Heh, this is going to annoy anyone who's already installed extensions into
> > SeaMonkey, because EM has been silently tracking them all ;-)
> 
> Indeed.  KaiRo: we may want to 'relnote' this?

Let's see how large the outcry of nightly testers is ;-)

I don't think we need to actually relnote it, as it will only appear the first time they launch the newly installed SeaMonkey 2.0 version - and it will only affect those who tested Alpha or Beta versions, not the normal users coming over from 1.x.