Bug 357295 – [Compatibility] PerspectiveSwitcher does not respect Open-New-Perspective in New Window setting

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

Product:	Platform
Classification:	Eclipse
Component:	UI
Version:	4.1
Hardware:	All All

Importance:	P3 normal (vote)
Target Milestone:	4.2 M3
Assigned To:	Remy Suen
QA Contact:	Remy Suen

URL:
Whiteboard:
Keywords:

Depends on:
Blocks:
	Show dependency tree

Reported:	2011-09-09 20:18 EDT by Brian de Alwis
Modified:	2011-10-26 08:36 EDT (History)
CC List:	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	no flags	Details \| Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Brian de Alwis

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

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

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

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

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

2011-10-26 08:36:44 EDT

Verified with I20111025-2000 on Windows XP. Thanks for the bug report, Brian!