diff --git a/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/ApplicationPartServiceImpl.java b/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/ApplicationPartServiceImpl.java index d7eaaa26603..5a532b98bc1 100644 --- a/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/ApplicationPartServiceImpl.java +++ b/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/ApplicationPartServiceImpl.java @@ -131,7 +131,10 @@ public Optional switchPerspective(String perspectiveId) { } private void switchPerspectiveInternal(MPerspective perspective) { - perspective.getParent().setSelectedElement(perspective); + // Only set the perspective as selected if it is still to be rendered + if (perspective.isToBeRendered()) { + perspective.getParent().setSelectedElement(perspective); + } UIEvents.publishEvent(UIEvents.UILifeCycle.PERSPECTIVE_SWITCHED, perspective); } diff --git a/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/CommandLineOptionModelProcessor.java b/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/CommandLineOptionModelProcessor.java index 93a5da83a3e..03583b28aa7 100644 --- a/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/CommandLineOptionModelProcessor.java +++ b/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/CommandLineOptionModelProcessor.java @@ -65,7 +65,10 @@ private void selectForcedPerspective() { for (MPerspective persp : perspStack.getChildren()) { if (persp.getElementId().equals(forcedPerspectiveId)) { - perspStack.setSelectedElement(persp); + // Only set as selected if it's to be rendered + if (persp.isToBeRendered()) { + perspStack.setSelectedElement(persp); + } return; } } diff --git a/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/ModelServiceImpl.java b/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/ModelServiceImpl.java index 5fb2f8e0017..21baeb83c4a 100644 --- a/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/ModelServiceImpl.java +++ b/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/ModelServiceImpl.java @@ -609,7 +609,10 @@ public void bringToTop(MUIElement element) { element.setToBeRendered(true); } - window.getParent().setSelectedElement(window); + // Only set the window as selected if it is still to be rendered (not being removed) + if (window.isToBeRendered()) { + window.getParent().setSelectedElement(window); + } } else { showElementInWindow(window, element); } @@ -647,7 +650,10 @@ private void showElementInWindow(MWindow window, MUIElement element) { @SuppressWarnings("unchecked") MElementContainer container = (MElementContainer) parent; - container.setSelectedElement(element); + // Only set as selected if the element is still to be rendered + if (element.isToBeRendered()) { + container.setSelectedElement(element); + } if (window != parent) { showElementInWindow(window, parent); } diff --git a/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/PartServiceImpl.java b/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/PartServiceImpl.java index c5d66843adb..9d0607fad15 100644 --- a/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/PartServiceImpl.java +++ b/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/PartServiceImpl.java @@ -616,7 +616,10 @@ public void switchPerspective(MPerspective perspective) { Assert.isNotNull(perspective); MWindow window = getWindow(); if (window != null && isInContainer(window, perspective)) { - perspective.getParent().setSelectedElement(perspective); + // Only set the perspective as selected if it is still to be rendered + if (perspective.isToBeRendered()) { + perspective.getParent().setSelectedElement(perspective); + } List newPerspectiveParts = modelService.findElements(perspective, null, MPart.class, null); // if possible, keep the same active part across perspective switches @@ -771,7 +774,10 @@ private void activate(MPart part, boolean requiresFocus, boolean activateBranch) recordStackActivation(part); delegateBringToTop(part); - window.getParent().setSelectedElement(window); + // Only set the window as selected if it is still to be rendered (not being removed) + if (window.isToBeRendered()) { + window.getParent().setSelectedElement(window); + } partActivationHistory.activate(part, activateBranch); @@ -1323,7 +1329,10 @@ private void createElement(MUIElement element) { parent = element.getParent(); if (parent != null && parent.getChildren().size() == 1) { // if we're the only child, set ourselves as the selected element - parent.setSelectedElement(element); + // only if we're still to be rendered + if (element.isToBeRendered()) { + parent.setSelectedElement(element); + } } } @@ -1390,7 +1399,7 @@ public void hidePart(MPart part, boolean force) { MUIElement candidate = partActivationHistory.getSiblingSelectionCandidate(part); candidate = candidate == null ? null : candidate.getCurSharedRef() == null ? candidate : candidate.getCurSharedRef(); - if (candidate != null && children.contains(candidate)) { + if (candidate != null && children.contains(candidate) && candidate.isToBeRendered()) { parent.setSelectedElement(candidate); } else { for (MUIElement child : children) { diff --git a/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/application/EPartServiceTest.java b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/application/EPartServiceTest.java index b443c1999a1..f401ad956c6 100644 --- a/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/application/EPartServiceTest.java +++ b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/application/EPartServiceTest.java @@ -10100,6 +10100,39 @@ public void partVisible(MPart part) { } + /** + * Test for Bug 3509: IllegalArgumentException when exiting e4-RCP application. + * This test verifies that activating a part when the window is being removed + * (toBeRendered=false) doesn't throw an IllegalArgumentException. + */ + @Test + public void testActivatePartDuringWindowCleanup() { + createApplication("partId"); + + MWindow window = application.getChildren().get(0); + getEngine().createGui(window); + + EPartService partService = window.getContext().get(EPartService.class); + MPart part = partService.findPart("partId"); + assertNotNull(part); + + // Activate the part first + partService.activate(part); + assertEquals(part, partService.getActivePart()); + + // Simulate window cleanup by setting toBeRendered to false + window.setToBeRendered(false); + + // Now activate the part again - this should not throw an IllegalArgumentException + // even though the window is no longer toBeRendered + try { + partService.activate(part); + // If we reach here, the fix is working + } catch (IllegalArgumentException e) { + fail("Should not throw IllegalArgumentException when activating part during window cleanup: " + e.getMessage()); + } + } + static class ExceptionListener implements IPartListener { @Override