89532 - System tray menu for -turbo should include components

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect, P2)

Description

[Viswanath Ramachandran]

This bug is to implement aspects of the System Tray Icon specification for 
QuickLaunch / turbo mode. We need to have menu items for Nav, Mail, Composer, 
Address Book (Message Compose??) in the system tray menu. In commercial we need 
an IM menu item. 

We also need to fix up exit so that it can be called even if windows are still 
open.

Comment 1

[Viswanath Ramachandran]

we could consider adding this to nsBranch list if we have a patch and we first 
get through the other items on the plate. 

Comment 2

[timeless]

Could we make this show the list of items from the task menu (after windows is 
split) up to the first seperator.

Comment 3

[Peter 'Rufus' Nelson]

If there's going to be this tray icon, and mozilla's going to be running
(loaded) in the background anyway, why not have it check if there's any new mail
and when there is show a small mail-icon instead of the mozilla icon.  Then when
you double-click on the mail icon, it open your mailbox instead of opening a new
browser window.  Just an idea, but one I think a lot of people will like....

Comment 4

[:deanis74]

As I mentioned in Bug 85984, I'd like a way to hide this icon.  It's fine if
it's in the prefs panel, and visible by default, but I don't like clutter in my
tray and I hate apps that throw an icon down there and don't let me remove it. 
So my suggestion is to add a Display Quick Launch Icon in System Tray (prolly
needs to be reworded) checkbox in the Advanced prefs panel that's only enabled
if Enable Quick Launch is checked.

Comment 5

[Peter Lairo]

See also related bug 90466 because the mail notify menu items need to be
sensibly integrated.

Comment 6

[Michael Hendy (Hendikins)]

*** Bug 91639 has been marked as a duplicate of this bug. ***

Comment 7

[RFLazaro]

integrating a simple mail checker would be really nice; possible features would
include checking mail every N minutes and a popup dialog with say a button
saying open mail client and cancel 

Comment 8

[Alex Bishop]

Roderick: That's bug 90466.

Comment 9

[Jaime Rodriguez, Jr.]

PChen - Is Blake still working on this one? Is this a "stop-ship"? Will this one
be completed by today?

---> Pchen, because blaker has finished his internship, and now is blakeross, again.

Comment 10

[Chase Tingley]

*** Bug 98065 has been marked as a duplicate of this bug. ***

Comment 11

[Jaime Rodriguez, Jr.]

Pchen - Are we gonna get to this one for eMojo? If we want this, please mark as
nsbranch+.

Comment 12

[Paul Chen]

This is not a 0.9.4 stopper, pushing to mozilla0.9.5

Comment 13

[Blake Ross]

Taking, I have a patch for this.

Comment 14

[Blake Ross]

Attachment: patch (-w)

Comment 15

[Bill Law]

Comments:

1. I think we need an "appendMenuItem()" utility function that does the
AppendMenuW vs. GetACPString+AppendMenu rigamorole just once (it can remember
which technique is required via some static flag).

2. This technique of adding the components replicates logic that's already in
the tasks menu overlay.  It might be better to add code to navigator.js so that
it simply extracts the appropriate task menu items from the task menu and passes
them back to nsINativeAppSupportWin (e.g., via some new AddTrayIconMenuItem
method).  That would avoid replicating these task menu items in
turboMenu.properties, the check for mail/address-book, and hopefully take care
of the addition of IM in commercial builds (and new tasks that get added in the
future).

Comment 16

[Jatin Billimoria]

Blake, my suggested wording is below. We've separated the term into two words in
previous releases, so we shouldn't make it one this time.

---Suggested Wording---
Quick Launch is now disabled. To enable it again, open the Edit menu and choose
Preferences. Choose the "Advanced" category and check "Enable Quick Launch."
---

Comment 17

[Blake Ross]

> 1. I think we need an "appendMenuItem()" utility function that does the
> AppendMenuW vs. GetACPString+AppendMenu rigamorole just once (it can remember
> which technique is required via some static flag).

What do you mean?  Why is it more than once?  We try it once, and if we get an 
error, we do the other technique.

> 2. This technique of adding the components replicates logic that's already in
> the tasks menu overlay.  It might be better to add code to navigator.js so 
> that it simply extracts the appropriate task menu items from the task menu and 
> passes them back to nsINativeAppSupportWin (e.g., via some new 
> AddTrayIconMenuItem method).  That would avoid replicating these task menu 
> items in turboMenu.properties, the check for mail/address-book, and hopefully 
> take care of the addition of IM in commercial builds (and new tasks that get 
> added in the future).

Unless there's something I don't know, I think this would unfortunately be 
insanely difficult.  (Weren't we talking about this at the beginning of the 
summer?)  I don't think tasks are changing so much that this should be a huge 
area of concern, but I'm open to suggestions.  With regards to Instant 
Messenger, I'm thinking of just adding a menuitem and getting the label and the 
cmdline arg to pass from a properties file.  This would allow us to use IRC Chat 
in Mozilla builds and Instant Messenger in commercial builds.  I just didn't 
feel like doing that this time around.

Also, this patch looks (literally) pretty ugly, doesn't it? :-)  I think it's a 
combination of the wrap-param-in-spaces convention, the four line indentation, 
and the string usage (which string foo master jag says is nearly as good as it 
can get at the moment).

Comment 18

[Bill Law]

There's n calls to ::AppendMenuW and n calls to GetACPString/::AppendMenu. 
There should be 1 call to each (inside a utility function that is called from
elsewhere, n times).  Sorry I wasn't more clear on that.

>Unless there's something I don't know, I think this would unfortunately be 
>insanely difficult.

I'm not sure how one measures the difficulty, or whether we should be measuring
the one-time implementation effort vs. the long-term effort to keep the various
task component lists in synch.  Basically, we've already got the list of
components defined, it seems a shame to have to do it again.

This is a design issue, not an issue with your implementation.  You've added
hard dependencies between this *implementation* file and the *implementation* of
mailnews.  You've re-introduced the dependency on specific per-component command
line switches (i.e., what's technically implementation details of those
components).  Those aren't good things, obviously.  I'd just like to consider
options that might avoid those problems, regardless of whether those options
might seem a little less convenient to implement.

Of course, it is too late for that.  Maybe I should just go open bugs for all
these design flaws and we can fix it properly when we've got the time.  God
knows when that would be.

Comment 19

[Blake Ross]

I agree with you.  It's not like I enjoy checking if mail is installed from, er,
nsNativeAppSupportWin :-).  This is something I've been talking about in other
bugs.  For one, I think turbo needs to be split out from all these files, into
its own service.

I don't think we really have the component list defined, we just have a menu. 
That's not generic enough for our purposes.  As far as I can tell, trying to use
the tasksOverlay for this purpose would require literally parsing the file.  I
don't see this as a good thing either, really.  We probably should just have a
components list in rdf somewhere.

Comment 20

[Blake Ross]

Also,

> I'd just like to consider options that might avoid those problems, regardless of 
> whether those options might seem a little less convenient to implement.

I'm happy to implement less 'convenient' options -- as I said, I definitely
agree with what you're saying -- I'm just literally getting e-mails as we speak
asking about the status of turbo.

Comment 21

[Syd Logan]

r=syd

Comment 22

[Bill Law]

Yes, let's open bugs to try to encourage us to go back and fix it later.

Comment 23

[Blake Ross]

Attachment: new patch with helper function

Comment 24

[sairuh (rarely reading bugmail)]

qa contact to terri for the nonce...

Comment 25

[Blake Ross]

Checked in on trunk.

Comment 26

[Richard Klein]

I have been watching this project and specifically this bug for quite a while,
and I would like to say you guys are doing a great job.  I would like to make
some suggestions though.  I think that the quick launch should be a seperate
Mozilla component, with it's own preferences, that basically brokers launching
and notification services to different components of Mozilla.  I have made a
couple of mock up screen shots of how I think the preferences should look.  

Comment 27

[Richard Klein]

For some reason I cannot get my attachements created.  I have posted them at
http://www.kleinsystems.com/images/pref.gif and
http://www.kleinsystems.com/images/pref2.gif

The first shot is the general settings for the launch center.  The Launch List
items are what appears when right clicking the icon.  The second shot is an
individual set up for mail & newsgroups behaviors when a notification event occurs.

Comment 28

[Peter Trudelle]

Mass moving lower-priority bugs to 0.9.6 (with Blake's pre-consent) to make room
for remaining 0.9.4/eMojo bugs and MachV planning, performance and feature work.
 If anyone disagrees with the new target, please let me know.

Comment 29

[Blake Ross]

Fixed.

Comment 30

[Terri Preston]

Verified fixed W2k build 2001-09-17-05-0.9.4