Ensure active part receives focus on perspective switch #2841
+45
−12
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
I'm not sure if the problem is seen only on Linux... I don't have a Windows machine to test on. The change here might not be needed on Windows... Or Mac. |
Contributor
Author
Ideally the correct part receives focus... I.e. whatever was focused in perspective X will get focused again when switching to X... @iloveeclipse are we looking into that? The initial bug report complained about losing focus in Eclipse; I'm not sure what the user is expecting of the focus when switching back and forth between perspectives. No idea how complicated keeping the correct active part for each perspective will be. Right now |
Contributor
|
This pull request changes some projects for the first time in this development cycle. An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch. Git patchFurther information are available in Common Build Issues - Missing version increments. |
Contributor
Member
|
Interesting one. Seem to be simple :-) What I see is that in the cases where switch perspective happens we have now fixed the problem. However, the new code
I'm not sure regarding the second point if that is good or bad. I also didn't checked what would happen in multi-window Eclipse case. You can see what is being focused by switching |
Contributor
Author
I don't see any weird behavior when using multiple windows. Any specific scenario you want tested?
Odd, the new code leads to recursive activation: I'll look into this. |
When switching a perspective, if a view exists in both the old and the new perspective, it doesn't gain focus after the perspective switch. This is contrary to the case in which a view is open only in the new perspective. Since editors are shared between perspectives, the same bug always occurs for editors - the editor will lose focus on a perspective switch. This change adds part focusing code on the code branch, on which the active part already exists in the new perspective. This ensures the active part gains focus. Note that this change doesn't preserve focus of the active part in each perspective. I.e. if View A is active in perspective X and View B was active in perspective Y, upon switching to perspective Y, View A will become active if it exists. The change only ensures that focus is not lost altogether. Fixes: eclipse-platform#2839 Signed-off-by: Simeon Andreev <[email protected]>
trancexpress
commented
Mar 18, 2025
| 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) { |
Contributor
Author
There was a problem hiding this comment.
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.
Member
There was a problem hiding this comment.
Let say it this way: most of e4 is ugly, but what should we do? At least this works.
Member
Let assume debugging case, starting debugger from java perspective, have a breakpoint hit and see what happens if debug perspective should be activated. Which window / view will be activated, does it have focus, etc. Play around that. |
Contributor
Author
In the double-focus case (I assume if we merge anything, it will be similar to that), there is no difference in behavior. If I trigger debugging (with a breakpoint) from window 1 (in Java perspective), window 2 (in debug perspective) is activated on the breakpoint hit and the Debug view has the focus. |
Contributor
|
Of course views must handle the case to get focus more than once... but this usually means the view has lost focus before. As we don't know what custom views do on focus (e.g. the might itself send out an event, refresh some state, whatever) it seems wrong to trigger that event unnecessarily (e.g. if the view already has the focus). |
Contributor
Author
I guess we can also debug why SWT/GTK+ is losing the focus in the first place, during the perspective switch. The platform UI code doesn't seem to expect this... |
Member
|
I've played now with the patch and found out to my surprise that if one opens a second window, the original code without patch works as expected and focus is being set on the part after switching to the different perspective. At same time same code doesn't work for the first window ?!? WHAT? |
Contributor
Author
The view with the missing focus must be open in both the old and new perspectives. See the reproduction steps on #2839. Have you ensured that? When I do that in the 2nd window, I still see the bug. |
Member
Yes, that was it, I had to open second perspective in the second view and the selected views were different because of that. |
|
|
||
| private boolean constructed = false; | ||
|
|
||
| private boolean switchingPerspective = false; |
Member
There was a problem hiding this comment.
Please don't init to defaults, here and below, and to be consistent we can remove it from above too.
iloveeclipse
approved these changes
Mar 18, 2025
Member
iloveeclipse
left a comment
iloveeclipse
left a comment
There was a problem hiding this comment.
LGTM.
I've checked on Windows, the patch seem to be not needed here (there are other oddities with the not proper active tab highlighting I don't see on Linux), but it seem not to harm either. I guess we are walking on a thin ice of CTabFolder & Co that are rendered / behave slightly different on different platforms.
| 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) { |
Member
There was a problem hiding this comment.
Let say it this way: most of e4 is ugly, but what should we do? At least this works.
Contributor
Author
Upon switching the perspective the first time, the focus is lost with this stack trace: After that its not regained - this bug. If I then manually focus the Console view and switch back to the original perspective (the 2nd perspective switch), the focus is again lost, but with this stack trace: I'm not sure how we can avoid this... So far I've not been able to restore the lost focus in the SWT code. |
Contributor
Author
|
@iloveeclipse what about this change in SWT? eclipse-platform/eclipse.platform.swt#1916 Since its in SWT, maybe its a good idea to merge it early in the 4.36 cycle... to hopefully find any problems it might cause. |
Contributor
Author
|
Fixed with eclipse-platform/eclipse.platform.swt#1915 instead. |
Contributor
|
@trancexpress good job in identify and fix the root cause here! This will benefit other use-cases as well! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When switching a perspective, if a view exists in both the old and the
new perspective, it doesn't gain focus after the perspective switch.
This is contrary to the case in which a view is open only in the new
perspective. Since editors are shared between perspectives, the same bug
always occurs for editors - the editor will lose focus on a perspective
switch.
This change adds part focusing code on the code branch, on which the
active part already exists in the new perspective. This ensures the
active part gains focus.
Note that this change doesn't preserve focus of the active part in each
perspective. I.e. if View A is active in perspective X and View B was
active in perspective Y, upon switching to perspective Y, View A will
become active if it exists. The change only ensures that focus is not
lost altogether.
Fixes: #2839