[6.x] Fix Link fieldtype preventing Cmd+S keyboard shortcut in Entry mode

[lazerg]

PageTree binds mod+s globally in created() and never destroys the binding. Mousetrap keeps one callback per combo, so a second bind removes the first.

For structured collections, the Link fieldtype's Entry selector renders PageTree with editable: false, and save() returns immediately in that state. Opening the selector replaces the entry publish form's save shortcut with a no-op, and nothing restores it. Cmd+S stays dead for the rest of the page.

This binds the shortcut only when the tree is editable, and destroys the binding on unmount.

Fixes #14598

[lazerg]

Hey @jasonvarga, please check this when you have time.

[duncanmcclean]

Sorry, we can't provide an ETA for reviewing/merging this pull request. 😞

It's already on our to-do list and we'll review it when we can. If you need to use this fix in the meantime, consider applying this PR as a composer patch.

[jasonvarga]

🤖 This review was generated via an AI review.

Requires changes

• Critical: The new forwardKeyboardShortcuts handler redispatches Cmd+S/Cmd+Enter to document, but the original event already bubbles there naturally (reka-ui's FocusScope doesn't stop propagation for non-Tab keys). This double-fires the global save handler — verified empirically — causing a duplicate save request whenever Cmd+S is pressed with any Stack open, not just Link fields.
• Warning: The mod+return branch checks event.key === 'return'/'Return', but the real value is 'Enter', so that branch is dead code.
• Note: The new tests only check that the internal dispatchEvent spy fired, so they wouldn't catch either issue above.

Would want FocusScope confirmed as the actual blocker (in a real browser) before this workaround goes in, and stopPropagation() on the original event if forwarding stays.

[lazerg]

You were right that FocusScope isn't the blocker. I dropped the Stack workaround and pushed the actual fix.

Propagation. In reka-ui 2.9.5, FocusScope.handleKeyDown only calls preventDefault(), and only inside the isTabKey branch. It never calls stopPropagation. It is a bubble-phase listener, so it could not keep the event away from document even if it tried. I checked in a browser as well. With the selector stack open, a document-level keydown listener sees Cmd+S exactly once, and Mousetrap's handler runs. The forwarding was redundant.

Double fire. Half right. It did double up plain document.addEventListener('keydown') listeners. I counted 2 against a baseline of 1. It did not cause a duplicate save. The re-dispatched KeyboardEvent carries keyCode/which of 0, and Mousetrap matches on which rather than key, so the synthetic event was invisible to every bindGlobal handler. Either way the code is gone now.

'return' vs 'Enter'. Correct. KeyboardEvent.key is 'Enter', so that branch never ran. Also gone.

Tests. Correct. A spy on dispatchEvent proved nothing about whether a save happened. The new test counts how many times the save handlers actually fire, and it fails against current 6.x.

What actually breaks it. PageTree binds mod+s globally in created() and never destroys it. Mousetrap keeps one callback per combo, so a second bind removes the first. For structured collections the Link fieldtype's Entry selector renders PageTree with editable: false, and save() returns straight away in that state. Opening the selector therefore replaces the entry publish form's save shortcut with a no-op, and nothing puts it back. That is why Cmd+S stays dead after you pick an entry.

The fix is small: bind the shortcut only when the tree is editable, and destroy the binding on unmount.

I verified it in a browser against the steps in #14598. Before, Cmd+S after choosing an entry sent no request at all. After, it sends one PATCH and no duplicates. The full JS suite passes.

[jasonvarga]

Thanks!