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

RESOLVED FIXED in seamonkey2.0

Status

SeaMonkey
Startup & Profiles
--
trivial
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: sgautherie, Assigned: sgautherie)

Tracking

(Depends on: 1 bug, {fixed-seamonkey2.0, polish})

Trunk
seamonkey2.0
fixed-seamonkey2.0, polish
Dependency tree / graph
Bug Flags:
wanted-seamonkey2.0 +
blocking-seamonkey2.0 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

9 years ago
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.
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?
OS: Windows 2000 → All
Hardware: PC → All
(Assignee)

Comment 2

9 years ago
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

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

Comment 4

9 years ago
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

9 years ago
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).
(Assignee)

Comment 6

9 years ago
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.)
Severity: enhancement → trivial
Status: UNCONFIRMED → NEW
Depends on: 408115
Ever confirmed: true
Flags: blocking-seamonkey2?
(Assignee)

Comment 7

9 years ago
FF bug 408115 depended on FF bug 411207;
but SM actually misses the whole "session restore" feature: bug 36810.
Depends on: 36810
Whiteboard: [Needs bug 36810]
(Assignee)

Comment 8

9 years ago
Created attachment 327327 [details] [diff] [review]
(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>
Assignee: general → sgautherie.bz
Status: NEW → ASSIGNED
(Assignee)

Updated

9 years ago
Depends on: 421635

Comment 9

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

Comment 10

9 years ago
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).
Why can't we do this as part of final-ui-startup?

Comment 12

8 years ago
Serge, can you revive this patch in a way that addresses the last few comments? Would surely be nice to have this in now.
Keywords: polish
Whiteboard: [Needs bug 36810]

Comment 13

8 years ago
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?
Flags: wanted-seamonkey2.0+
Flags: blocking-seamonkey2.0?
Flags: blocking-seamonkey2.0-
(Assignee)

Comment 14

8 years ago
Created attachment 401678 [details] [diff] [review]
(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?
Attachment #327327 - Attachment is obsolete: true
Attachment #401678 - Flags: superreview?(neil)
Attachment #401678 - Flags: review?(neil)
(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 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.
Attachment #401678 - Flags: superreview?(neil)
Attachment #401678 - Flags: superreview-
Attachment #401678 - Flags: review?(neil)
(Assignee)

Comment 17

8 years ago
(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.
(Assignee)

Comment 18

8 years ago
(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/ !
(Assignee)

Comment 19

8 years ago
Created attachment 402738 [details] [diff] [review]
(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.
Attachment #401678 - Attachment is obsolete: true
Attachment #402738 - Flags: superreview?(neil)
Attachment #402738 - Flags: review?(neil)
(Assignee)

Updated

8 years ago
Summary: XPI install misses "installation complete" dialog after application restart → [SeaMonkey] XPI install misses "installation complete" dialog after application restart
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.
Attachment #402738 - Flags: superreview?(neil)
Attachment #402738 - Flags: superreview+
Attachment #402738 - Flags: review?(neil)
Attachment #402738 - Flags: review+
(Assignee)

Comment 21

8 years ago
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.
Attachment #402738 - Attachment description: (Av2) Port bug 408115, but use final-ui-startup + alwaysRaised → (Av2) Port bug 408115, but use final-ui-startup + alwaysRaised [Checkin: See comment 21]
(Assignee)

Updated

8 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Component: General → Startup & Profiles
Keywords: fixed-seamonkey2.0
QA Contact: general → profile-manager
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.0
(Assignee)

Updated

8 years ago
Depends on: 453274

Comment 22

8 years ago
(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.
(Assignee)

Updated

8 years ago
Blocks: 521621
You need to log in before you can comment on or make changes to this bug.