Skip to content

Fix E4 model perspectives being converted to orphan local copies #3972

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
akurtakov merged 1 commit into from
May 7, 2026
Merged

Fix E4 model perspectives being converted to orphan local copies #3972

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/WorkbenchPage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2020 IBM Corporation and others.
* Copyright (c) 2000, 2026 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand Down Expand Up @@ -3908,8 +3908,19 @@ private Perspective getPerspective(MPerspective mperspective) {
PerspectiveDescriptor perspectiveDesc = (PerspectiveDescriptor) getPerspectiveDesc(
mperspective.getElementId());
if (perspectiveDesc == null) {
fixedPerspective = true;
perspectiveDesc = fixOrphanPerspective(mperspective);
if (mperspective.getContributorURI() != null) {
PerspectiveRegistry reg = (PerspectiveRegistry) PlatformUI.getWorkbench().getPerspectiveRegistry();
// Registers an E4 perspective that is contributed directly via the application
// model (e.g. via a model fragment), without renaming it or marking it as a
// local copy. Unlike PerspectiveRegistry#addPerspective(MPerspective),
// this method does not add the perspective to the application snippets.
reg.createDescriptor(mperspective);
perspectiveDesc = (PerspectiveDescriptor) getPerspectiveDesc(mperspective.getElementId());
sortedPerspectives.add(perspectiveDesc);
} else {
fixedPerspective = true;
perspectiveDesc = fixOrphanPerspective(mperspective);
}
}
Perspective p = new Perspective(perspectiveDesc, mperspective, this);
modelToPerspectiveMapping.put(mperspective, p);
Expand Down
4 changes: 2 additions & 2 deletions ....eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/registry/PerspectiveRegistry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public void addPerspective(MPerspective perspective) {
createDescriptor(perspective);
}

private void createDescriptor(MPerspective perspective) {
public void createDescriptor(MPerspective perspective) {
String label = perspective.getLocalizedLabel();
String originalId = getOriginalId(perspective);
PerspectiveDescriptor originalDescriptor = descriptors.get(originalId);
Expand Down Expand Up @@ -319,7 +319,7 @@ private String getOriginalId(MPerspective p) {
p.getContributorURI()));
}
int index = id.lastIndexOf('.');
// Custom perspectives store the user defined names in their labels
// Custom perspectives store the user defined names in their labels.
String trimE4 = label.trim();
String trimE3 = label.replace(' ', '_').trim();
if (id.endsWith(label)) {
Expand Down
44 changes: 43 additions & 1 deletion tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/api/IWorkbenchPageTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2019 IBM Corporation and others.
* Copyright (c) 2000, 2026 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand Down Expand Up @@ -48,7 +48,10 @@
import org.eclipse.core.runtime.ILogListener;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.Platform;
import org.eclipse.e4.ui.model.application.ui.advanced.MPerspective;
import org.eclipse.e4.ui.model.application.ui.advanced.MPerspectiveStack;
import org.eclipse.e4.ui.model.application.ui.basic.MPart;
import org.eclipse.e4.ui.workbench.modeling.EModelService;
import org.eclipse.jface.text.IDocument;
import org.eclipse.jface.util.IPropertyChangeListener;
import org.eclipse.swt.events.ShellEvent;
Expand Down Expand Up @@ -80,7 +83,9 @@
import org.eclipse.ui.ide.IDE;
import org.eclipse.ui.internal.WorkbenchPage;
import org.eclipse.ui.internal.WorkbenchPlugin;
import org.eclipse.ui.internal.WorkbenchWindow;
import org.eclipse.ui.internal.registry.IActionSetDescriptor;
import org.eclipse.ui.internal.registry.PerspectiveDescriptor;
import org.eclipse.ui.internal.util.Util;
import org.eclipse.ui.navigator.resources.ProjectExplorer;
import org.eclipse.ui.part.FileEditorInput;
Expand Down Expand Up @@ -3349,6 +3354,43 @@ public void testGetViewStackWithSecondaryId() throws PartInitException {
assertEquals(part, stack[0]);
}

@Test
public void testE4PerspectiveNotConvertedToLocalCopy() {
IWorkbenchWindow window = openTestWindow();
EModelService modelService = window.getService(EModelService.class);

WorkbenchWindow workbenchWindow = (WorkbenchWindow) window;
List<MPerspectiveStack> stacks = modelService.findElements(workbenchWindow.getModel(), null,
MPerspectiveStack.class);
assertFalse("Perspective stack should exist in the window model", stacks.isEmpty());
MPerspectiveStack perspectiveStack = stacks.get(0);

String e4Id = "org.eclipse.ui.tests.internal.e4.perspective.model";
String e4Label = "E4ModelPerspective";

assertNull("e4 perspective ID should not be in perspective registry",
PlatformUI.getWorkbench().getPerspectiveRegistry().findPerspectiveWithId(e4Id));

MPerspective e4Perspective = modelService.createModelElement(MPerspective.class);
e4Perspective.setElementId(e4Id);
e4Perspective.setLabel(e4Label);
e4Perspective.setContributorURI("platform:/plugin/org.eclipse.ui.tests");

perspectiveStack.getChildren().add(e4Perspective);
perspectiveStack.setSelectedElement(e4Perspective);
processEvents();

assertEquals("Element ID should not have changed", e4Id, e4Perspective.getElementId());

IPerspectiveDescriptor descriptor = PlatformUI.getWorkbench().getPerspectiveRegistry()
.findPerspectiveWithId(e4Id);
assertNotNull(descriptor);

assertFalse(((PerspectiveDescriptor) descriptor).hasCustomDefinition());

PlatformUI.getWorkbench().getPerspectiveRegistry().deletePerspective(descriptor);
Copy link
Copy Markdown
Contributor

@merks merks May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this line cleanup for after the test to avoid this leaking into other tests? If the test fails, might this leak anyway and then cause other tests to fail too?

Copy link
Copy Markdown
Member Author

@akurtakov akurtakov May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failures in other tests can occur only if the same ids are used or number of elements are checked inside the same suite. As every test suite creates new workbench it shouldn't be possible to leak into other test suites are these model changes should be gone on new startup as they are not backed by any permanent source (e.g. fragment.e4xmi). With all of the above, it is still nice to clean up when we can to prevent time lost in the future.

}

private static class ShellStateListener implements ShellListener {
private final AtomicBoolean shellIsActive;

Expand Down
1 change: 0 additions & 1 deletion ...rg.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/internal/InternalTestSuite.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import org.eclipse.ui.tests.markers.MarkerViewUtilTest;
import org.eclipse.ui.tests.markers.ResourceMappingMarkersTest;
import org.eclipse.ui.tests.markers.ScopeAreaTest;

import org.junit.platform.suite.api.SelectClasses;
import org.junit.platform.suite.api.Suite;

Expand Down
Loading