Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 348920 - [Compatibility] Contribution visibility seems to leak to other workbench windows
Summary: [Compatibility] Contribution visibility seems to leak to other workbench windows
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.1   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 4.2 M4   Edit
Assignee: Remy Suen CLA
QA Contact: Remy Suen CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-09 11:34 EDT by Remy Suen CLA
Modified: 2011-12-06 14:16 EST (History)
3 users (show)

See Also:


Attachments
WorkbenchWindow patch v1 (1.20 KB, patch)
2011-11-24 13:53 EST, Remy Suen CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Remy Suen CLA 2011-06-09 11:34:54 EDT
If you open a Java editor in one workbench window, all other workbench windows will have its contributions show up such as a 'Refactor' menu or tool bar contributions like the breadcrumbs feature.
Comment 1 Remy Suen CLA 2011-06-14 14:28:59 EDT
1. Activate the 'Outline' view.
2. Window > New Window
3. Activate the 'Outline' view.
4. Go back to the first window.
5. Activate the 'Package Explorer' view.
6. Notice that second window now also shows the 'Source' and 'Refactor' menus.
Comment 2 Paul Webster CLA 2011-06-29 07:29:08 EDT
I believe this is an effect of our earlier attempt to push variables to the application context.  Really, we should make sure when our code is evaluating information, it uses takes the model context and uses context.getActiveLeaf().

This is something I feel we should review for 4.1.1 and if the cost is low, include it.

PW
Comment 3 Remy Suen CLA 2011-08-10 10:11:28 EDT
When contexts get activated, their string ids are pushed into the application's context (refer to ContextContextService's activateContext(String)). Since it's in the application's context, the new workbench window ends up retrieving these ids makes the editor action sets visible to the user.
Comment 4 Paul Webster CLA 2011-08-10 10:39:54 EDT
What we want:  contexts are set at each level as they are activated.  The activeIds are the Set of activeIds found by askingContext.getActiveLeaf() and then summing them up
Comment 5 Remy Suen CLA 2011-08-10 13:09:45 EDT
Fix pushed to R4_development.
http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?h=R4_development&id=c21ab3481f6463d26c86c9fb88f51279a83ac968

Technically speaking, the contributions appear in the second workbench window for a few seconds before they get removed.
Comment 6 Remy Suen CLA 2011-08-26 09:14:54 EDT
This is broken again as of M20110817-2001. Since this is not a critical problem that affects the use of the SDK, I will not try to investigate a fix for 4.1.1 GA.
Comment 7 Remy Suen CLA 2011-09-07 09:50:47 EDT
(In reply to comment #4)
> What we want:  contexts are set at each level as they are activated.  The
> activeIds are the Set of activeIds found by askingContext.getActiveLeaf() and
> then summing them up

The problem here is that this process always adds up to the application's context. This means even for something being evaluated in an inactive workbench window, it will take active contexts from the application. Since there is only one context service at the application level that activates and deactivates contexts, an inactive workbench window will seem as if it is actually active.
Comment 8 Brian de Alwis CLA 2011-11-01 11:24:16 EDT
I see this flickering very often: I normally have perspectives in separate windows, and clicking in the stack of the debugger causes continual flicker in other windows.
Comment 9 Remy Suen CLA 2011-11-24 11:45:35 EST
Inserting an additional layer of IContextServices at the window level with a SlaveContextService doesn't seem to fix this problem.

I feel like we need a separate EContextService at every level to locally store what contexts should be active at any given position.

I think ActiveContextsFunction may also need to be changed. As described by comment 7, we always recurse upwards regardless of whether we're the active child or not, so even inactive windows will be affected by the active context chain of the currently active window.
Comment 10 Remy Suen CLA 2011-11-24 13:53:31 EST
Created attachment 207499 [details]
WorkbenchWindow patch v1

Each window already has its own EContextService implementation. They don't have an IContextService implementation though. So if we create a new one for it then it'll have its personal IContextService instead of returning the one for the application.
Comment 11 Remy Suen CLA 2011-11-24 15:45:51 EST
(In reply to comment #10)
> Created attachment 207499 [details]
> WorkbenchWindow patch v1

Bug 364757 has surfaced itself now that the system is actually working as intended.
Comment 13 Remy Suen CLA 2011-12-06 13:26:48 EST
Verified with I20111205-2330 on Windows 7. Brian, please let me know if you spot any problems.
Comment 14 Brian de Alwis CLA 2011-12-06 14:16:48 EST
No problems from me — it's been great. Thanks Remy.