419600 - Prism: Add tray icon support (nsINotificationArea) for GTK2

Status: RESOLVED WONTFIX

(Mozilla Labs :: Prism, enhancement)

Description

[Fredrik Larsson (:nossralf)]

This is some desktop integration lovin' for Prism on Linux. Adds support for tray icons (though the GTK nomenclature calls it "status icons"). It's got feature parity with the Windows code that's in the tree right now.

Comment 1

[Fredrik Larsson (:nossralf)]

Attachment: a work-in-progress patch

This is a patch that builds and works great for me. I still call it a WIP because there are probably things like license texts that may need some massaging, it's possible the makefiles include unnecessary stuff, etc. Also, there are probably UI changes that fall under this bug, to enable tray icons when creating a webapp, etc. (Or they're follow-ups at least.)

Note that there's code commented out in _processConfig in the patch which corresponds to changes suggested in bug 418904.

(It's a patch generated by "hg diff", so apply with "patch -p1".)

Comment 2

[Reed Loden [:reed]]

Please use "unix" instead of "linux".

Comment 3

[Reed Loden [:reed]]

well, the summary can definitely say Linux, but I just meant that you should use "unix" for the directory name, etc.

Comment 4

[Fredrik Larsson (:nossralf)]

Ah, OK. Gotcha. Heh, I'm new at this.

Comment 5

[Fredrik Larsson (:nossralf)]

Attachment: same WIP with renames of files and directories

Dirs are now unix/ instead of linux/, files end with GTK instead of Linux.

Comment 6

[Matthew Gertner]

Comment on attachment 305727 [details] [diff] [review]
same WIP with renames of files and directories

Great work! Thanks a lot. A few comments:

>diff -r 014e0666cf43 client/chrome/content/webrunner.js
>--- a/client/chrome/content/webrunner.js	Mon Feb 25 13:51:19 2008 +0100
>+++ b/client/chrome/content/webrunner.js	Tue Feb 26 12:49:10 2008 +0100
>@@ -190,10 +190,10 @@ var WebRunner = {
> 
>     document.title = WebAppProperties.name;
> 
>-    if (WebAppProperties.trayicon) {
>-      var desktop = Cc["@mozilla.org/desktop-environment;1"].getService(Ci.nsIDesktopEnvironment);
>-      desktop.QueryInterface(Ci.nsINotificationArea).setTitle(WebAppProperties.id, document.title);
>-    }
>+    //    if (WebAppProperties.trayicon) {
>+    //      var desktop = Cc["@mozilla.org/desktop-environment;1"].getService(Ci.nsIDesktopEnvironment);
>+    //      desktop.QueryInterface(Ci.nsINotificationArea).setTitle(WebAppProperties.id, document.title);
>+    //    }

I removed this anyway in bug 419640.

>     if (WebAppProperties.uri)
>         this._getBrowser().loadURI(WebAppProperties.uri, null, null);
>@@ -568,7 +568,7 @@ var WebRunner = {
>     appIcon.append(WebAppProperties.id);
>     appIcon.append("icons");
>     appIcon.append("default");
>-    appIcon.append(WebAppProperties.icon + ".ico");
>+    appIcon.append(WebAppProperties.icon + ".png");

We should probably use the text preprocessor for this so we can use the right file extension for the platform (see http://developer.mozilla.org/en/docs/Build:Text_Preprocessor for general info and install-shortcut.js for an example).

>--- a/components/public/Makefile.in	Mon Feb 25 13:51:19 2008 +0100
>+++ b/components/public/Makefile.in	Tue Feb 26 12:49:10 2008 +0100
>@@ -54,6 +54,10 @@ DIRS = mac
> DIRS = mac
> endif
> 
>+ifeq ($(MOZ_WIDGET_TOOLKIT), gtk2)
>+DIRS = unix
>+endif
>+
> EXPORTS = \
> 		$(NULL)

We should move nsINotificationArea.idl into the top-level public directory if we're going to share it across multiple operating systems.

>diff -r 014e0666cf43 components/public/unix/Makefile.in
>--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
>+++ b/components/public/unix/Makefile.in	Tue Feb 26 12:49:10 2008 +0100

See previous comment.

>diff -r 014e0666cf43 components/public/unix/nsINotificationArea.idl
>--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
>+++ b/components/public/unix/nsINotificationArea.idl	Tue Feb 26 12:49:10 2008 +0100

See previous comment.

>+/*
>+ This little struct is used to maintain the relationship between
>+ tray icons and their listeners. It is necessary to do so
>+ in order to properly release the listener object once the
>+ icon is removed.
>+ */
>+
>+typedef struct _IconListenerMapping {
>+  GtkStatusIcon* icon;
>+  nsINotificationAreaListener* listener;
>+} IconListenerMapping;

Okay for now but we'll probably want to do something generic that works across all platforms.

>+static const nsModuleComponentInfo components[] =
>+{
>+  { "Windows desktop environment",

"GTK desktop environment"

Comment 7

[Fredrik Larsson (:nossralf)]

Attachment: same WIP with required changes

This addresses everything in comment 6. (Except the IconListenerMapping thing.) 

I can't verify the slight changes for the Windows makefile (but as they follow the same logic that works on Linux I think it's fine). This patch builds on Linux and works as before.

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

nossralf, do you have any time to look at matching this to matt's current work on windows and mac?

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Prism isn't maintained anymore. Mass closing of the bugs.