Bug 540297 - IWorkbenchPage.findView finds view from another perspective
Whenever a view is open in an inactive perspective, if that view is
opened and then closed in the active perspective,
IWorkbenchPage.findView(String) is able to find the view. This should
not be the case.
The undesired behavior is due to IWorkbenchPage.hideView() setting the
"to be rendered flag" on a placeholder which is not asked for the flag
during IWorkbenchPage.findView().
This change builds on the fix for bug 466230 by also asking the
placeholder reported by EModelService.findPlaceholderFor(). Only if both
the current shared reference (asked with fix for bug 466230) and that
placeholder are to be rendered, will the view be used as a result of
IWorkbenchPage.findView().
This change also adds a test which shows bad behavior when a view is
open in another perspective of a workbench window. The test will open a
test view in one of two test perspectives in resp. one of two test
windows. The test then validates the output of IWorkbenchPage.findView()
to ensure it returns non-null only if the view is actually open in that
page.
Change-Id: Ia0151e9a3ade70d541af41963b275f7ca83da8a9
Signed-off-by: Simeon Andreev <simeon.danailov.andreev@gmail.com>
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/WorkbenchPage.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/WorkbenchPage.java
index 6429790..19378a9 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/WorkbenchPage.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/WorkbenchPage.java
@@ -2578,11 +2578,10 @@
for (ViewReference reference : viewReferences) {
MPart model = reference.getModel();
// The part may be linked in either directly or via a
- // placeholder. In the latter case we can look directly
+ // placeholder. In the latter case we can look
// at the part's curSharedRef since we're only considering
// parts visible in the current perspective
- if (parts.contains(model) && model.isToBeRendered()
- && (model.getCurSharedRef() == null || model.getCurSharedRef().isToBeRendered())) {
+ if (parts.contains(model) && !shouldNotRenderPart(model)) {
// only rendered placeholders are valid view references
visibleReferences.add(reference);
}
@@ -2593,6 +2592,28 @@
}
/**
+ * @return {@code true} if the part should not be rendered or it has a current
+ * shared reference that is not to be rendered <b>or</b> if a
+ * placeholder for the part (in the current perspective) exists and is
+ * not to be rendered. {@code false} otherwise, i.e. if the placeholders
+ * of the part are to be rendered.
+ */
+ private boolean shouldNotRenderPart(MPart part) {
+ if (!part.isToBeRendered()) {
+ return true;
+ }
+ MPlaceholder curSharedRef = part.getCurSharedRef();
+ if (curSharedRef != null && !curSharedRef.isToBeRendered()) {
+ return true;
+ }
+ MPlaceholder mPlaceholder = modelService.findPlaceholderFor(window, part);
+ if (mPlaceholder != null && !mPlaceholder.isToBeRendered()) {
+ return true;
+ }
+ return false;
+ }
+
+ /**
* See IWorkbenchPage.
*/
@Override
diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/internal/Bug540297WorkbenchPageFindViewTest.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/internal/Bug540297WorkbenchPageFindViewTest.java
new file mode 100644
index 0000000..d0dc00e
--- /dev/null
+++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/internal/Bug540297WorkbenchPageFindViewTest.java
@@ -0,0 +1,248 @@
+/*******************************************************************************
+ * Copyright (c) 2018 Simeon Andreev and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ * Simeon Andreev - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.ui.tests.internal;
+
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Label;
+import org.eclipse.ui.IPageLayout;
+import org.eclipse.ui.IPerspectiveDescriptor;
+import org.eclipse.ui.IPerspectiveFactory;
+import org.eclipse.ui.IViewPart;
+import org.eclipse.ui.IViewReference;
+import org.eclipse.ui.IWorkbenchPage;
+import org.eclipse.ui.IWorkbenchWindow;
+import org.eclipse.ui.part.ViewPart;
+import org.eclipse.ui.tests.harness.util.UITestCase;
+
+/**
+ * Tests for bug 540297. Call {@link IWorkbenchPage#findView(String)} while the
+ * view is open in some perspective and window, and see if the find view method
+ * behaves properly.
+ */
+public class Bug540297WorkbenchPageFindViewTest extends UITestCase {
+
+ public static class MyPerspective implements IPerspectiveFactory {
+ public static String ID1 = "org.eclipse.ui.tests.internal.Bug540297WorkbenchPageFindViewTest.MyPerspective1";
+ public static String ID2 = "org.eclipse.ui.tests.internal.Bug540297WorkbenchPageFindViewTest.MyPerspective2";
+
+ @Override
+ public void createInitialLayout(IPageLayout layout) {
+ // we want an empty perspective in this test
+ }
+ }
+
+ public static class MyViewPart extends ViewPart {
+ public static String ID = "org.eclipse.ui.tests.internal.Bug540297WorkbenchPageFindViewTest.MyViewPart";
+
+ @Override
+ public void createPartControl(Composite parent) {
+ Label label = new Label(parent, SWT.NONE);
+ label.setText(getSite().getId());
+ }
+
+ @Override
+ public void setFocus() {
+ // we don't care about view functionality, so do nothing here
+ }
+ }
+
+ private IWorkbenchWindow firstWindow;
+ private IWorkbenchPage firstWindowActivePage;
+ private IWorkbenchWindow secondWindow;
+ private IWorkbenchPage secondWindowActivePage;
+ private IPerspectiveDescriptor originalPerspective;
+ private IPerspectiveDescriptor activePerspective;
+ private IPerspectiveDescriptor inactivePerspective;
+
+ public Bug540297WorkbenchPageFindViewTest(String testName) {
+ super(testName);
+ }
+
+ @Override
+ protected void doSetUp() throws Exception {
+ super.doSetUp();
+
+ firstWindow = fWorkbench.getActiveWorkbenchWindow();
+ secondWindow = openTestWindow();
+
+ firstWindowActivePage = firstWindow.getActivePage();
+ secondWindowActivePage = secondWindow.getActivePage();
+
+ originalPerspective = firstWindowActivePage.getPerspective();
+
+ activePerspective = getPerspetiveDescriptor(MyPerspective.ID1);
+ inactivePerspective = getPerspetiveDescriptor(MyPerspective.ID2);
+
+ firstWindowActivePage.setPerspective(activePerspective);
+ prepareWorkbenchPageForTest(firstWindowActivePage);
+ prepareWorkbenchPageForTest(secondWindowActivePage);
+
+ processEvents();
+ }
+
+ private void prepareWorkbenchPageForTest(IWorkbenchPage page) {
+ page.setPerspective(inactivePerspective);
+ page.resetPerspective();
+ page.closeAllEditors(false);
+ page.setPerspective(activePerspective);
+ page.resetPerspective();
+ page.closeAllEditors(false);
+ IViewReference[] views = page.getViewReferences();
+ for (IViewReference view : views) {
+ page.hideView(view);
+ }
+ }
+
+ @Override
+ protected void doTearDown() throws Exception {
+ secondWindow.close();
+ firstWindowActivePage.setPerspective(originalPerspective);
+ firstWindowActivePage.resetPerspective();
+ firstWindowActivePage.closePerspective(inactivePerspective, false, false);
+ firstWindowActivePage.closePerspective(activePerspective, false, false);
+ processEvents();
+ super.doTearDown();
+ }
+
+ /**
+ * Tests that if the active perspective of the first window has a view open, the
+ * view can be found while in the active perspective of the first window.
+ *
+ * Also checks that the second window cannot find the view, since the view was
+ * never open in that window.
+ */
+ public void testFindViewInFirstWindowAndActivePerspective() throws Exception {
+ showView(firstWindowActivePage);
+ assertCanFindView(firstWindowActivePage);
+ assertCannotFindView(secondWindowActivePage);
+ }
+
+ /**
+ * Tests that if the inactive perspective of the first window has a view open,
+ * the view cannot be found while in the active perspective of the first window.
+ *
+ * Also checks that the second window cannot find the view, since the view was
+ * never open in that window.
+ */
+ public void testFindViewInFirstWindowAndInactivePerspective() throws Exception {
+ showViewInInactivePerspective(firstWindowActivePage);
+ assertCannotFindView(secondWindowActivePage);
+ assertCannotFindView(firstWindowActivePage);
+ }
+
+ /**
+ * Tests that if the inactive perspective of the first window has a view open,
+ * the view cannot be found while in the active perspective of the first window.
+ * Shows and hides the view in the active perspective before checking.
+ *
+ * Also checks that the second window cannot find the view, since the view was
+ * never open in that window.
+ */
+ public void testFindViewInFirstWindowAndInactivePerspectiveWithOpenAndClose() throws Exception {
+ showViewInInactivePerspective(firstWindowActivePage);
+ showAndHideView(firstWindowActivePage);
+ assertCannotFindView(secondWindowActivePage);
+ assertCannotFindView(firstWindowActivePage);
+ }
+
+ /**
+ * Tests that if the active perspective of the second window has a view open,
+ * the view can be found while in the active perspective of the second window.
+ *
+ * Also checks that the first window cannot find the view, since the view was
+ * never open in that window.
+ */
+ public void testFindViewInSecondWindowAndActivePerspective() throws Exception {
+ showView(secondWindowActivePage);
+ assertCanFindView(secondWindowActivePage);
+ assertCannotFindView(firstWindowActivePage);
+ }
+
+ /**
+ * Tests that if the inactive perspective of the second window has a view open,
+ * the view cannot be found while in the active perspective of the second
+ * window.
+ *
+ * Also checks that the first window cannot find the view, since the view was
+ * never open in that window.
+ */
+ public void testFindViewInSecondWindowAndInactivePerspective() throws Exception {
+ showViewInInactivePerspective(secondWindowActivePage);
+ assertCannotFindView(firstWindowActivePage);
+ assertCannotFindView(secondWindowActivePage);
+ }
+
+ /**
+ * Tests that if the inactive perspective of the second window has a view open,
+ * the view cannot be found while in the active perspective of the second
+ * window. Shows and hides the view in the active perspective before checking.
+ *
+ * Also checks that the first window cannot find the view, since the view was
+ * never open in that window.
+ */
+ public void testFindViewInSecondWindowAndInactivePerspectiveWithOpenAndClose() throws Exception {
+ showViewInInactivePerspective(secondWindowActivePage);
+ showAndHideView(secondWindowActivePage);
+ assertCannotFindView(firstWindowActivePage);
+ assertCannotFindView(secondWindowActivePage);
+ }
+
+ private void showViewInInactivePerspective(IWorkbenchPage pageForTest) throws Exception {
+ setPerspective(pageForTest, inactivePerspective);
+ showView(pageForTest);
+ setPerspective(pageForTest, activePerspective);
+ }
+
+ private static void setPerspective(IWorkbenchPage page, IPerspectiveDescriptor perspective) {
+ page.setPerspective(perspective);
+ page.resetPerspective();
+ processEvents();
+ }
+
+ private static void showAndHideView(IWorkbenchPage page) throws Exception {
+ showView(page);
+ hideView(page);
+ }
+
+ private static void showView(IWorkbenchPage page) throws Exception {
+ page.showView(MyViewPart.ID);
+ processEvents();
+ }
+
+ private static void hideView(IWorkbenchPage page) throws Exception {
+ IViewPart view = page.findView(MyViewPart.ID);
+ page.hideView(view);
+ processEvents();
+ }
+
+ private IPerspectiveDescriptor getPerspetiveDescriptor(String perspectiveId) {
+ return fWorkbench.getPerspectiveRegistry().findPerspectiveWithId(perspectiveId);
+ }
+
+ private static void assertCanFindView(IWorkbenchPage page) throws Exception {
+ assertFindViewResult(page, true);
+ }
+
+ private static void assertCannotFindView(IWorkbenchPage page) throws Exception {
+ assertFindViewResult(page, false);
+ }
+
+ private static void assertFindViewResult(IWorkbenchPage page, boolean expectedFound) throws Exception {
+ IViewPart viewPart = page.findView(MyViewPart.ID);
+ boolean actualFound = viewPart != null;
+ assertEquals("unexpected result from IWorkbenchPage.findView(String): " + viewPart, expectedFound, actualFound);
+ }
+
+}
diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/internal/InternalTestSuite.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/internal/InternalTestSuite.java
index 9f1cb60..a57d558 100644
--- a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/internal/InternalTestSuite.java
+++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/internal/InternalTestSuite.java
@@ -69,5 +69,6 @@
addTest(new TestSuite(WorkbenchSiteProgressServiceModelTagsTest.class));
addTest(new TestSuite(WorkbenchPageTest.class));
addTest(new TestSuite(SaveablesListTest.class));
+ addTest(new TestSuite(Bug540297WorkbenchPageFindViewTest.class));
}
}
diff --git a/tests/org.eclipse.ui.tests/plugin.xml b/tests/org.eclipse.ui.tests/plugin.xml
index 77ce73b..73653f7 100644
--- a/tests/org.eclipse.ui.tests/plugin.xml
+++ b/tests/org.eclipse.ui.tests/plugin.xml
@@ -99,6 +99,14 @@
class="org.eclipse.ui.tests.propertysheet.PropertySheetPerspectiveFactory3"
id="org.eclipse.ui.tests.propertysheet.PropertySheetPerspectiveFactory3"
name="Second UI Test Perspective for hidden Properties view instance"/>
+ <perspective
+ class="org.eclipse.ui.tests.internal.Bug540297WorkbenchPageFindViewTest$MyPerspective"
+ id="org.eclipse.ui.tests.internal.Bug540297WorkbenchPageFindViewTest.MyPerspective1"
+ name="org.eclipse.ui.tests.internal.Bug540297WorkbenchPageFindViewTest.MyPerspective1"/>
+ <perspective
+ class="org.eclipse.ui.tests.internal.Bug540297WorkbenchPageFindViewTest$MyPerspective"
+ id="org.eclipse.ui.tests.internal.Bug540297WorkbenchPageFindViewTest.MyPerspective2"
+ name="org.eclipse.ui.tests.internal.Bug540297WorkbenchPageFindViewTest.MyPerspective2"/>
</extension>
<extension
@@ -543,6 +551,12 @@
class="org.eclipse.ui.tests.api.MockViewPart"
id="org.eclipse.ui.tests.api.MockViewPartVisibleByDefault">
</view>
+ <view
+ name="org.eclipse.ui.tests.internal.Bug540297WorkbenchPageFindViewTest.MyViewPart"
+ icon="icons/view.gif"
+ class="org.eclipse.ui.tests.internal.Bug540297WorkbenchPageFindViewTest$MyViewPart"
+ id="org.eclipse.ui.tests.internal.Bug540297WorkbenchPageFindViewTest.MyViewPart">
+ </view>
</extension>
<extension
point="org.eclipse.ui.editors">
