Skip to content

Ensure active part receives focus on perspective switch #2841

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

Closed
wants to merge 3 commits into from
Closed

Ensure active part receives focus on perspective switch #2841

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
edbc549
Extract part focus code for re-use
trancexpress Mar 17, 2025
380a716
Ensure active part receives focus on perspective switch
trancexpress Mar 17, 2025
c2bd1ee
Version bump(s) for 4.36 stream
eclipse-platform-bot Mar 18, 2025
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
2 changes: 1 addition & 1 deletion bundles/org.eclipse.e4.ui.workbench/META-INF/MANIFEST.MF
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-SymbolicName: org.eclipse.e4.ui.workbench;singleton:=true
Bundle-Version: 1.17.0.qualifier
Bundle-Version: 1.17.100.qualifier
Bundle-Name: %pluginName
Bundle-Vendor: %providerName
Bundle-Localization: plugin
Expand Down
55 changes: 44 additions & 11 deletions ...org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/PartServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,10 @@ private MPart toPart(MStackElement stackElement) {

private boolean constructed = false;

private boolean switchingPerspective = false;
Copy link
Member

@iloveeclipse iloveeclipse Mar 18, 2025

Choose a reason for hiding this comment

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

Please don't init to defaults, here and below, and to be consistent we can remove it from above too.


private boolean focusingActivePart = false;

@Inject
public PartServiceImpl(MApplication application, @Optional MWindow window) {
// no need to track changes:
Expand Down Expand Up @@ -604,6 +608,15 @@ public boolean isPartOrPlaceholderInPerspective(String elementId, MPerspective p

@Override
public void switchPerspective(MPerspective perspective) {
try {
switchingPerspective = true;
switchPerspectiveInternal(perspective);
} finally {
switchingPerspective = false;
}
}

private void switchPerspectiveInternal(MPerspective perspective) {
Assert.isNotNull(perspective);
MWindow window = getWindow();
if (window != null && isInContainer(window, perspective)) {
Expand Down Expand Up @@ -687,6 +700,11 @@ public void activate(MPart part, boolean requiresFocus) {
}

private void activate(MPart part, boolean requiresFocus, boolean activateBranch) {
if (focusingActivePart) {
Activator.trace(Policy.DEBUG_FOCUS_FLAG,
"Active part is being focused on perspective switch: " + activePart, null);//$NON-NLS-1$
return;
}
if (part == null) {
if (constructed && activePart != null) {
if (Policy.DEBUG_FOCUS) {
Expand Down Expand Up @@ -727,12 +745,23 @@ private void activate(MPart part, boolean requiresFocus, boolean activateBranch)
IEclipseContext windowContext = window.getContext();
// check if the active part has changed or if we are no longer the active window
if (windowContext.getParent().getActiveChild() == windowContext && part == activePart) {
// insert it in the beginning of the activation history, it may not have been inserted
// pending when this service was instantiated
partActivationHistory.prepend(part);
UIEvents.publishEvent(UIEvents.UILifeCycle.ACTIVATE, part);
if (Policy.DEBUG_FOCUS) {
Activator.trace(Policy.DEBUG_FOCUS_FLAG, "Trying to activate already active part: " + part, null);//$NON-NLS-1$
if (!focusingActivePart) {
Copy link
Contributor Author

@trancexpress trancexpress Mar 18, 2025

Choose a reason for hiding this comment

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

This is getting ugly... We can also take code from PartRenderingEngine.focusGui(MUIElement) to only try to force focus by calling AbstractPartRenderer.forceFocus(MUIElement)... if that works differently. But I'm not sure that works any better.

What about just leaving the double focusing in? Maybe it causes trouble in some client code which starts receiving 2 focus events instead of 0... With the flag for switching perspective protecting the focus call, I think that should be the only case of different behavior.

Copy link
Member

@iloveeclipse iloveeclipse Mar 18, 2025

Choose a reason for hiding this comment

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

Let say it this way: most of e4 is ugly, but what should we do? At least this works.

// insert it in the beginning of the activation history, it may not have been
// inserted
// pending when this service was instantiated
partActivationHistory.prepend(part);
if (switchingPerspective && requiresFocus) {
focusingActivePart = true;
try {
focusPart(part);
} finally {
focusingActivePart = false;
}
}
UIEvents.publishEvent(UIEvents.UILifeCycle.ACTIVATE, part);
if (Policy.DEBUG_FOCUS) {
Activator.trace(Policy.DEBUG_FOCUS_FLAG, "Trying to activate already active part: " + part, null);//$NON-NLS-1$
}
}
return;
}
Expand Down Expand Up @@ -767,11 +796,7 @@ private void activate(MPart part, boolean requiresFocus, boolean activateBranch)
partActivationHistory.activate(part, activateBranch);

if (requiresFocus) {
IEclipseContext context = part.getContext();
if (context != null) {
IPresentationEngine pe = context.get(IPresentationEngine.class);
pe.focusGui(part);
}
focusPart(part);
}

// store the activation time to sort the parts in MRU order
Expand Down Expand Up @@ -1513,4 +1538,12 @@ private MElementContainer<? extends MUIElement> getContainer() {
}
return outerContainer;
}

private void focusPart(MPart part) {
IEclipseContext context = part.getContext();
if (context != null) {
IPresentationEngine pe = context.get(IPresentationEngine.class);
pe.focusGui(part);
}
}
}
Loading