Bug 335011

Comment 1 Markus Keller 2011-01-21 09:12:30 EST

I think SWT should automatically remove task items on shutdown ([1]), unless a menu item has been specially treated so that it also works without a running process ([2]).

AFAICs, TaskBar#createShellLink(MenuItem, String) hardcodes the arguments to
"--launcher.openFile /SWTINTERNAL_ID108" etc. where "108" is the id of the MenuItem in the currently running instance of the application. This can only work reliably in the same process. When the application is terminated (and possibly replaced by an other version), the id can very well be different in the new process.

[1] e.g. via Runtime#addShutdownHook(..)

[2] You could add an API like TaskItem#PERSISTENT_TASK_COMMAND and have clients call menuItem.setData(PERSISTENT_TASK_COMMAND, "<command line to execute when application is not running>". On shutdown, set a new list of tasks that only contains the special items with the given command lines.

To make jump lists work even in non-RCP apps, you could even add TaskItem#TASK_COMMAND that doesn't rely on the magic "--launcher.openFile " argument prefix.

Comment 2 Felipe Heidrich 2011-01-21 10:34:35 EST

I believe that if the application call Display#setAppName() if a unique name only the app by that appName would get the menu.

Applications do not remove the menu when they close, the application can be pinned to the taskbar and menu is available even when the app is not running (try Chrome for example).

If you want the menu to be removed, TaskItem.setMenu(null) should work (have you tried?). You can call that when the workbench is closing.


I also do not like the ID magic, see Bug 306039.

Comment 3 Markus Keller 2011-01-21 13:24:01 EST

> If you want the menu to be removed, TaskItem.setMenu(null) should work (have
> you tried?). You can call that when the workbench is closing.

Yes, that works, but I'm speaking as a user here. And as a user, the leftover items from Mylyn are garbage that I cannot remove.


> Applications do not remove the menu when they close, the application can be
> pinned to the taskbar and menu is available even when the app is not running
> (try Chrome for example).

Yes, I understand that. That's why I proposed to only remove those items which are only working by chance across relaunches.

> I also do not like the ID magic, see Bug 306039.

I didn't find that bug before, thanks. The ideas there are very similar to my proposed TaskItem#PERSISTENT_TASK_COMMAND. The only difference is that I expect only those task items to stay in the menu which specify an explicit executable and arguments.

The strategy via Runtime#addShutdownHook(..) also gives at least a working story in RCP applications that can dynamically add/remove bundles. E.g. Mylyn could be removed from an install or a user could just remove an Eclipse install and replace it with a fresh install that doesn't contain Mylyn (but uses the same app name). SWT will never know about that and cannot easily remove stale menu items.

But with my solution, the items will be gone after the next shutdown, even if TaskItem#setMenu(..) is never called any more.

Comment 4 Markus Keller 2011-02-08 11:41:43 EST

This will probably require API changes, so it should be scheduled for M6.

Comment 5 Felipe Heidrich 2011-02-09 10:27:40 EST

I'm afraid I don't understand the problem entirely.

Doesn't Eclipse always call TaskItem#setMenu(newMenu) as part of the start-up ?
Doesn't newMenu include only the "good" items in it ?

I think the part I don't understand is:
Since eclipse always set a new menu everytime it runs, why do you need to remove the menu when eclipse closes ?

and if you indeed need that, why can't you use this code:

display.addListener(SWT.Dispose, new Listener() {
	public void handleEvent(Event event) {
		TaskBar bar = display.getSystemTaskBar();
		if (bar != null) {
			TaskItem item = bar.getItem(null);
			if (item != null) item.setMenu(null);
		}
	}
});

Why do you need SWT to remove it for you ? 
It seems to me that you have all the API to do what you need.

What am I missing ?

on a separate note, I do not think Mylyn should be calling TaskItem#setMenu(newMenu) directly, I would expect jump list items to be contribute to the UI by extension points.

Comment 6 Markus Keller 2011-02-09 12:40:13 EST

The Eclipse SDK *never* calls TaskItem#setMenu(newMenu), that's why stale entries stay there after any Eclipse-based application ever set a menu.

I agree that Mylyn should not call this method, but the platform currently doesn't provide API for plug-ins to contribute to a common application task item menu. I've filed bug 336738 for that.

Nevertheless, running the attached SnippetPersistentTaskItems only once is enough to get a task item in every SWT application with the same AppName. This is clearly a lifecycle problem in SWT. Applications that didn't set a menu should not get a menu out of nowhere.

My proposal to prune the menu on shutdown and only keep items that have been set by the current process would solve this issue. Using the display dispose event for this looks good (but that should be done by SWT, not by clients).

Comment 7 Markus Keller 2011-03-23 13:01:34 EDT

Ping. I still think SWT should take care of this lifecycle issue.

Right before the display gets disposed, SWT should check the current system task bar menu. If the menu contains items without the magic "org.eclipse.swt.win32.taskbar.executable" property, SWT should remove these items and set a new system menu that only contains the items that can work across relaunches.

If there was no task bar menu or if none of the menu items has set the property, SWT should still set an empty menu to make sure all obsolete items are also removed in the OS.

Comment 8 Felipe Heidrich 2011-03-24 10:20:58 EDT

(In reply to comment #7)
> Ping. I still think SWT should take care of this lifecycle issue.
> Right before the display gets disposed, SWT should check the current system
> task bar menu. If the menu contains items without the magic
> "org.eclipse.swt.win32.taskbar.executable" property, SWT should remove these
> items and set a new system menu that only contains the items that can work
> across relaunches.

Not true, all items can work across relaunches.


> If there was no task bar menu or if none of the menu items has set the
> property, SWT should still set an empty menu to make sure all obsolete items
> are also removed in the OS.

The best idea I think is to always removes all items during the initialization of the display, since the app (Mylyn) always call setMenu() on every startup things should work. Eclipse does not call setMenu(), so the items will be removed on the first startup of Eclipse.

Are you okay with that ?

Comment 9 Markus Keller 2011-03-26 17:15:11 EDT

(In reply to comment #8)
Sounds good.

Unfortunately, this will clear the menu during startup, but that's a minor problem compared to the stale tasks.

Comment 10 Felipe Heidrich 2011-03-28 12:53:16 EDT

Created attachment 192024 [details]
patch

Comment 11 Felipe Heidrich 2011-03-28 12:54:39 EDT

Created attachment 192025 [details]
patch

Comment 12 Felipe Heidrich 2011-03-28 12:55:24 EDT

Fixed in HEAD

Comment 13 Markus Keller 2011-03-30 10:59:32 EDT

The fix doesn't seem to work reliably. In I20110329-0800, it did remove the stale Mylyn tasks from my Eclipse task menu, but when I run attachment 187281 [details] and then run it again with the first XXX line commented, the task doesn't disappear.


> (In reply to comment #8)
> Unfortunately, this will clear the menu during startup, but that's a minor
> problem compared to the stale tasks.

If anybody from Mylyn has a problem with that, please speak up.

Comment 14 Felipe Heidrich 2011-03-30 13:58:29 EDT

My mistake, I was calling CoCreateInstance() before OleInitialize(), I moved the same code to run after OleInitialize() (still inside the same method Display#init)
I tested using SnippetPersistentTaskItems.

Comment 15 Markus Keller 2011-03-30 14:40:17 EDT

Thx, verified in HEAD.