Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 357295 - [Compatibility] PerspectiveSwitcher does not respect Open-New-Perspective in New Window setting
Summary: [Compatibility] PerspectiveSwitcher does not respect Open-New-Perspective in ...
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 M3   Edit
Assignee: Remy Suen CLA
QA Contact: Remy Suen CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-09 20:18 EDT by Brian de Alwis CLA
Modified: 2011-10-26 08:36 EDT (History)
1 user (show)

See Also:


Attachments
Patch PerspectiveSwitcher to use IWorkbench#showPerspective() (2.76 KB, patch)
2011-09-09 20:18 EDT, Brian de Alwis CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brian de Alwis CLA 2011-09-09 20:18:27 EDT
Created attachment 203098 [details]
Patch PerspectiveSwitcher to use IWorkbench#showPerspective()

The Perspective Switcher doesn't respect the user's "Open New Perspective In New Window" setting.  I don't see that there should be a difference in behaviour between using Window > Open Perspective and the Perspective Switcher.

Steps to repeat:

1. Set "Preferences > General >Perspectives > Open a new perspective > In a new window".  
2. Use the Perspective Switcher to open a new perspective.  

See the new perspective open in the current window.

Patch to simply use Workbench#showPerspective() attached.  (It also includes a tiny is-null test in closePerspective() to avoid an NPE I was experienced when trying to close a perspective.)
Comment 1 Brian de Alwis CLA 2011-09-09 20:36:30 EDT
Hmm, this patch worked in my testing, but now breaks.  This code in Workbench#activate() and in Workbench#showPerspective() around like 2544 seem a bit suspect:

	public IWorkbenchPage showPerspective(String perspectiveId, IWorkbenchWindow targetWindow,
			IAdaptable input) throws WorkbenchException {
//...
		if (targetWindow != null) {  /// line 2539
			IWorkbenchPage page = targetWindow.getActivePage();
			if (activate(perspectiveId, page, input, false)) {
				return page;
			}
			IPreferenceStore store = WorkbenchPlugin.getDefault().getPreferenceStore();
			int mode = store.getInt(IPreferenceConstants.OPEN_PERSP_MODE);

			if (IPreferenceConstants.OPM_NEW_WINDOW != mode) {
//....
        }

	private boolean activate(String perspectiveId, IWorkbenchPage page, IAdaptable input,
			boolean checkPerspective) {
		if (page != null) {
			for (IPerspectiveDescriptor openedPerspective : page.getOpenPerspectives()) {
				if (!checkPerspective || openedPerspective.getId().equals(perspectiveId)) {
					if (page.getInput() == input) {
						WorkbenchWindow wwindow = (WorkbenchWindow) page.getWorkbenchWindow();
						MWindow model = wwindow.getModel();
						application.setSelectedElement(model);
						return true;
					}
				}
			}
		}
		return false;
	}

In this case, the call to activate() at line 2541 succeeds as input is null and the page input is null too.
Comment 2 Brian de Alwis CLA 2011-09-09 21:08:30 EDT
(In reply to comment #1)
> Hmm, this patch worked in my testing, but now breaks.

I'm a bit puzzled by this — removing the deltas.xml seemingly makes things right.  I think it's nearly time to call it a night.
Comment 3 Remy Suen CLA 2011-09-12 08:31:59 EDT
Thanks for the bug report, Brian. We're in a milestone week right now so I'll take a look at this next week.
Comment 4 Remy Suen CLA 2011-09-20 14:45:10 EDT
I've fixed the problem by just changing the menu item to invoke the 'Show Perspective' handler. The 3.x handler already has all the logic there and if it gets screwed up we only need to fix it in one place. :)
http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?h=R4_development&id=beb08644857e62ab03cdb0f58b79a3f1927b34c3

Thanks for the bug report, Brian!
Comment 5 Remy Suen CLA 2011-10-26 08:36:44 EDT
Verified with I20111025-2000 on Windows XP. Thanks for the bug report, Brian!