Skip to content
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.

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

Keyboard focus can activate top component #4603

Merged
merged 1 commit into from Oct 10, 2022
Merged

Keyboard focus can activate top component #4603

merged 1 commit into from Oct 10, 2022

Conversation

errael
Copy link
Contributor

@errael errael commented Sep 9, 2022

Fix for #4437. Discussion in that issue is useful in understanding this patch.

platform/core.windows is a mystery to me; this patch is almost identical to getting a mouse click, except it is triggered by keyboard focus change to handle other situations where the MainWindow is activated. There may be better solutions, but I believe this is safe.

After getting a focus event, the activate code is only invoked when it is strictly needed. This is an optimization, invoking the activation code multiple times shouldn't matter; it would be like clicking on an already active window.

@jtulach, any idea for a good candidate to review this? This area was written by Peter Zavadsky.

@eirikbakke
Copy link
Contributor

For others passing through, here is @neilcsmith-net's reproducibility comment from the other thread:

Try detaching a source editor tab into a separate window and Alt-TABing between. The cursor moves from one editor to the other, but the window system active tab doesn't change - undo/redo is wrong, navigator is wrong, and wrong tab is coloured as focused. Typing goes in the right place, though.

I can confirm this on my own Windows 11 machine; this is obviously incorrect behavior. If the patch fixes this problem, then great; thank you!

Looking at the patch, it does carefully apply the modified behavior only in some specific cases, which should make it lower-risk. Looks good to me, but maybe good to have one more reviewer look at it.

The next time I make a private NetBeans build I'll include this patch and test it...

@errael
Copy link
Contributor Author

errael commented Sep 16, 2022

The next time I make a private NetBeans build I'll include this patch and test it...

That would be great, I moved to linux last year, not tested on windows.

BTW, note that mouse click processing happens before keyboard focus change processing so this doesn't kick in when focus change due to mouse click.

@eirikbakke
Copy link
Contributor

I just tested this in a custom NetBeans build; the fix seems to work! Tested for correct active tab indication and Undo/Redo effect on Windows 11 with focus changed with Alt+Tab, Ctrl+Tab, clicking in Windows taskbar, clicking on tab within NetBeans, and clicking on window title bar.

I'll now leave this patch in my working IDE for a while to see that there are no problems.

Thanks for this!

@mbien mbien added the UI User Interface label Sep 29, 2022
Copy link
Contributor

@eirikbakke eirikbakke left a comment

Choose a reason for hiding this comment

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

I've been running my IDE with this patch active for the past few days with no problems, and it fixes the problem. I also reviewed the code previously.

I think this PR should be safe to merge.

@lkishalmi lkishalmi added this to the NB16 milestone Oct 10, 2022
@geertjanw geertjanw merged commit 2c27f5a into apache:master Oct 10, 2022
@errael errael deleted the KeyboardFocusCanActivateTopComponent branch October 10, 2022 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI User Interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants