Skip to content

[GTK4] Fix keyboard navigation in menus - #3603

Merged
akurtakov merged 1 commit into
eclipse-platform:masterfrom
akurtakov:gtk4-fix-menu-keyboard-navigation
Sep 19, 2026
Merged

akurtakov merged 1 commit into
eclipse-platform:masterfrom
akurtakov:gtk4-fix-menu-keyboard-navigation

Conversation

@akurtakov

@akurtakov akurtakov commented Sep 18, 2026

Copy link
Copy Markdown
Member

Arrow keys did nothing in an open popup or menu bar drop-down:
Shell.gtk_move_focus redirected the traversal to the focus control,
which pulled the focus out of the menu. Move the focus within the
menu's popover instead. Also keep the workbench from restoring the
saved focus while a menu holds it, and restore it once the menu closes.

Left in a submenu closed the whole menu, a menu opened from the
keyboard showed no selection although GTK had focused its first row,
a submenu whose rows Eclipse replaces on SWT.Show lost its focus, and
moving from a native row onto an image row left both rows highlighted.
Hide the submenu before GTK handles Left, select the focused first row
of a menu opened from the keyboard, refocus the first row after
SWT.Show, and clear the other rows when an image row takes the focus.
A tooltip appearing elsewhere cleared the keyboard selection: the
pointer-driven selection sync now leaves it alone while no row is
under the pointer. In a menu with a CHECK item that has an image the
focus got stuck on the first image row: the check indicator claimed
the traversal step without taking the focus, so it is now excluded
from traversal. A submenu with only disabled items trapped the
navigation: GTK put the focus on its scrolled window and kept sending
every key there. Keep the focus on the cascade row instead, let Up and
Down close such a submenu and move on, and let Right leave a menu bar
drop-down from it as from a leaf row.

Assisted-by: Anthropic Claude Code (claude-fable-5-1)

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Menu-bar switching can lose focus, skip no disabled entries, and mishandle right-to-left navigation.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes GTK4 keyboard focus and selection behavior for popup menus, menu-bar drop-downs, and submenus.

Changes:

  • Routes keyboard traversal through active menu popovers.
  • Preserves and restores focus when menus open and close.
  • Synchronizes row highlighting and prevents check indicators from capturing traversal.
File summaries
File Description
Shell.java Routes focus traversal into GTK4 menus.
MenuItem.java Corrects custom-row focus and highlighting.
Menu.java Implements navigation, selection, and focus restoration.
Display.java Tracks temporarily untargetable menu rows.
GTK.java Adds GTK direction constants.
Review details

Suppressed comments (2)

bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Menu.java:1441

  • The deferred restore also runs when this drop-down was hidden because keyboard navigation activated a neighboring menu-bar item. At that point the new menu's focused row is still a descendant of the same parentMenu.handle, so this condition restores the saved control and pulls focus straight back out of the newly opened menu. Exclude focus that is currently inside any menu popover before restoring.
			if (focus == 0 || !GTK.gtk_widget_get_mapped(focus) || parentMenu != null && isDescendant(focus, parentMenu.handle)) shell.restoreFocus();

bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Menu.java:1349

  • Only the immediately adjacent menu-bar child is tried. If that item is disabled or hidden, gtk_widget_activate does nothing but moveFocus still consumes the arrow key, so keyboard navigation gets stuck instead of advancing to the next usable menu. Walk siblings (with wraparound) until a visible, sensitive item is found, or return without activation after reaching the current item.
	long next = right ? GTK4.gtk_widget_get_next_sibling(item) : GTK4.gtk_widget_get_prev_sibling(item);
	if (next == 0) next = right ? GTK4.gtk_widget_get_first_child(bar) : GTK4.gtk_widget_get_last_child(bar);
	GTK.gtk_widget_activate(next);
  • Files reviewed: 5/5 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Menu.java Outdated
@github-actions

github-actions Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Test Results (linux)

  109 files  ±0    109 suites  ±0   14m 37s ⏱️ -22s
4 632 tests ±0  4 398 ✅ ±0  234 💤 ±0  0 ❌ ±0 
3 470 runs  ±0  3 381 ✅ ±0   89 💤 ±0  0 ❌ ±0 

Results for commit f97a512. ± Comparison against base commit 2d549b1.

♻️ This comment has been updated with latest results.

@akurtakov
akurtakov force-pushed the gtk4-fix-menu-keyboard-navigation branch from b955760 to f5fd76b Compare September 18, 2026 18:53
@akurtakov
akurtakov requested a balanced review from Copilot September 18, 2026 18:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Closing one menu-bar drop-down can asynchronously pull focus out of a newly opened neighboring menu.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Menu.java Outdated
@akurtakov
akurtakov force-pushed the gtk4-fix-menu-keyboard-navigation branch from f5fd76b to c0ac938 Compare September 18, 2026 19:35
@akurtakov
akurtakov requested a balanced review from Copilot September 18, 2026 19:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

RTL traversal, popup focus containment, and callback-driven native-handle invalidation remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Menu.java:1292

  • This only recognizes the LTR “back to parent” key. Menu supports SWT.RIGHT_TO_LEFT and applies an RTL GTK direction, where Right closes a submenu and Left opens it; as written, Right still follows GTK's broken close path and can collapse the whole RTL menu. Choose the closing direction from the menu orientation.
	boolean closing = direction == GTK.GTK_DIR_LEFT && popover != root;
  • Files reviewed: 5/5 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Menu.java Outdated
Comment thread bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Menu.java Outdated
@akurtakov
akurtakov force-pushed the gtk4-fix-menu-keyboard-navigation branch from c0ac938 to 9934bc9 Compare September 18, 2026 19:50
Arrow keys did nothing in an open popup or menu bar drop-down:
Shell.gtk_move_focus redirected the traversal to the focus control,
which pulled the focus out of the menu. Move the focus within the
menu's popover instead. Also keep the workbench from restoring the
saved focus while a menu holds it, and restore it once the menu closes.

Left in a submenu closed the whole menu, a menu opened from the
keyboard showed no selection although GTK had focused its first row,
a submenu whose rows Eclipse replaces on SWT.Show lost its focus, and
moving from a native row onto an image row left both rows highlighted.
Hide the submenu before GTK handles Left, select the focused first row
of a menu opened from the keyboard, refocus the first row after
SWT.Show, and clear the other rows when an image row takes the focus.
A tooltip appearing elsewhere cleared the keyboard selection: the
pointer-driven selection sync now leaves it alone while no row is
under the pointer. In a menu with a CHECK item that has an image the
focus got stuck on the first image row: the check indicator claimed
the traversal step without taking the focus, so it is now excluded
from traversal. A submenu with only disabled items trapped the
navigation: GTK put the focus on its scrolled window and kept sending
every key there. Keep the focus on the cascade row instead, let Up and
Down close such a submenu and move on, and let Right leave a menu bar
drop-down from it as from a leaf row.

Assisted-by: Anthropic Claude Code (claude-fable-5-1)
@akurtakov
akurtakov force-pushed the gtk4-fix-menu-keyboard-navigation branch from 9934bc9 to f97a512 Compare September 19, 2026 07:16
@akurtakov
akurtakov requested a balanced review from Copilot September 19, 2026 07:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Horizontal submenu navigation does not account for right-to-left menu orientation.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (2)

@akurtakov

Copy link
Copy Markdown
Member Author

Merge to get things to usable state so details can be worked on .

@akurtakov akurtakov self-assigned this Sep 19, 2026
@akurtakov
akurtakov merged commit 969c513 into eclipse-platform:master Sep 19, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants