fix: skip the shortcut owner token when the owner is the UI (#25631) (CP: 25.3) by vaadin-bot · Pull Request #25643 · vaadin/flow

vaadin-bot · 2026-09-10T13:39:24Z

This PR cherry-picks changes from the original PR #25631 to branch 25.3.

Original PR description

Summary

A shortcut's owner-scope guard puts a unique id into the keydown filter text, and that text is kept for the whole life of the UI by the client expression cache and both constant pools. A shortcut owned by the UI does not need that id, because the UI element is <body>, which can never be inside an open popover or modal. Such shortcuts now share one filter expression instead of creating a new one per registration.

Fixes #25624

What changed

ShortcutRegistration no longer generates the owner token eagerly. getOwnerToken() is now the single place that decides whether a token is needed and creates it, and it returns null when none is needed: the guard is off (allowEventsFromNestedModals) or the lifecycle owner is the UI. Callers just use what they get.

For a UI-owned shortcut the filter is now window.Vaadin.Flow.shortcut.eventInTopLevelScope(event), the same text for every registration.

FlowShortcut.js gains eventInTopLevelScope(event), which only checks that the keydown did not originate inside a popover or modal. The shared "which scope did the event come from" logic moved into a small _originScope helper used by both guards.

Side fix: with the guard off, updateOwnerMarker used to ask for a token before checking the flag, so a registration that never uses one still created it.

Test summary

# Status What the test verifies Why it matters

1 ✅ A UI-owned shortcut leaves the UI element unmarked and its filter is eventInTopLevelScope(event), with no owner attribute or token in the text This is the fix; a token here is what leaked per registration

2 ✅ Removing and re-registering the same UI shortcut produces exactly the same set of keydown expressions A filter text that differs per registration grows the client expression cache and both constant pools forever

3 ✅ A component-owned shortcut still marks its owner and its filter locates that owner by the marker token The normal, non-UI path must keep working unchanged

4 ✅ In a real browser, a UI-owned Alt+S stays silent for a keydown inside an open popover and fires once for a keydown on the top layer The new token-free client helper must guard exactly like the token-based one

5 ✅ A shortcut owned by a component inside a popover still fires for keydowns from that popover (#24974) Guards against re-breaking the earlier fix while the test view was reworked

ShortcutRegistrationTest.ownerScopeGuard_uiLifecycleOwner_filterHasNoTokenAndUiIsNotMarked → 1

ShortcutRegistrationTest.ownerScopeGuard_uiLifecycleOwner_repeatedRegistrationsShareFilter → 2

ShortcutRegistrationTest.ownerScopeGuard_componentLifecycleOwner_filterLocatesOwnerByToken → 3

PopoverOwnerShortcutIT.uiOwnedShortcut_firesOnlyOutsideThePopover → 4 (uses the new UI-owned shortcut, second input and sync button added to PopoverOwnerShortcutView)

PopoverOwnerShortcutIT.ownerInsidePopoverFocused_shortcutFires → 5 (pre-existing, now shares the view with the new case)

Deliberately untested: skipping the token when allowEventsFromNestedModals is on has no observable effect (no filter is generated in that case at all), and the existing ownerScopeGuard_lifecycleOwnerNotMarkedWhenAllowed already pins that the owner ends up unmarked. The _originScope extraction in FlowShortcut.js is a pure move, still covered by the existing delegate tests.

## Summary A shortcut's owner-scope guard puts a unique id into the keydown filter text, and that text is kept for the whole life of the UI by the client expression cache and both constant pools. A shortcut owned by the `UI` does not need that id, because the UI element is `<body>`, which can never be inside an open popover or modal. Such shortcuts now share one filter expression instead of creating a new one per registration. Fixes #25624 ## What changed - `ShortcutRegistration` no longer generates the owner token eagerly. `getOwnerToken()` is now the single place that decides whether a token is needed and creates it, and it returns `null` when none is needed: the guard is off (`allowEventsFromNestedModals`) or the lifecycle owner is the `UI`. Callers just use what they get. - For a UI-owned shortcut the filter is now `window.Vaadin.Flow.shortcut.eventInTopLevelScope(event)`, the same text for every registration. - `FlowShortcut.js` gains `eventInTopLevelScope(event)`, which only checks that the keydown did not originate inside a popover or modal. The shared "which scope did the event come from" logic moved into a small `_originScope` helper used by both guards. - Side fix: with the guard off, `updateOwnerMarker` used to ask for a token before checking the flag, so a registration that never uses one still created it. ## Test summary | # | Status | What the test verifies | Why it matters | |---|--------|------------------------|----------------| | 1 | ✅ | A UI-owned shortcut leaves the UI element unmarked and its filter is `eventInTopLevelScope(event)`, with no owner attribute or token in the text | This is the fix; a token here is what leaked per registration | | 2 | ✅ | Removing and re-registering the same UI shortcut produces exactly the same set of keydown expressions | A filter text that differs per registration grows the client expression cache and both constant pools forever | | 3 | ✅ | A component-owned shortcut still marks its owner and its filter locates that owner by the marker token | The normal, non-UI path must keep working unchanged | | 4 | ✅ | In a real browser, a UI-owned Alt+S stays silent for a keydown inside an open popover and fires once for a keydown on the top layer | The new token-free client helper must guard exactly like the token-based one | | 5 | ✅ | A shortcut owned by a component inside a popover still fires for keydowns from that popover (#24974) | Guards against re-breaking the earlier fix while the test view was reworked | - `ShortcutRegistrationTest.ownerScopeGuard_uiLifecycleOwner_filterHasNoTokenAndUiIsNotMarked` → 1 - `ShortcutRegistrationTest.ownerScopeGuard_uiLifecycleOwner_repeatedRegistrationsShareFilter` → 2 - `ShortcutRegistrationTest.ownerScopeGuard_componentLifecycleOwner_filterLocatesOwnerByToken` → 3 - `PopoverOwnerShortcutIT.uiOwnedShortcut_firesOnlyOutsideThePopover` → 4 (uses the new UI-owned shortcut, second input and sync button added to `PopoverOwnerShortcutView`) - `PopoverOwnerShortcutIT.ownerInsidePopoverFocused_shortcutFires` → 5 (pre-existing, now shares the view with the new case) Deliberately untested: skipping the token when `allowEventsFromNestedModals` is on has no observable effect (no filter is generated in that case at all), and the existing `ownerScopeGuard_lifecycleOwnerNotMarkedWhenAllowed` already pins that the owner ends up unmarked. The `_originScope` extraction in `FlowShortcut.js` is a pure move, still covered by the existing delegate tests. --------- Co-authored-by: totally-not-ai[bot] <290682512+totally-not-ai[bot]@users.noreply.github.com>

vaadin-bot · 2026-09-10T13:52:43Z

This PR is eligible for auto-merging policy, so it has been approved automatically. If there are pending conditions, auto merge (with 'squash' method) has been enabled for this PR [Message is sent from bot]

github-actions · 2026-09-10T13:53:58Z

A UI-owned shortcut now emits one shared keydown filter instead of a unique one per registration

flowchart LR
    subgraph Before
        direction TB
        B1["ShortcutRegistration, UI owner"] -->|"ownerToken = sc-uuid, always"| B2["generateOwnerScopeFilter()"]
        B2 -->|"emits, new text per registration"| B3["eventInOwnerScope(event, '[...sc-uuid]')"]
    end
    subgraph After
        direction TB
        A1["ShortcutRegistration, UI owner"] -->|"getOwnerToken() returns null (new)"| A2["generateOwnerScopeFilter()"]
        A2 -->|"emits, same text every registration"| A3["eventInTopLevelScope(event) (new)"]:::changed
    end
    Before ~~~ After
    classDef changed stroke:#c9a227,stroke-width:3px

The figure shows what generateOwnerScopeFilter() produces for a shortcut whose lifecycleOwner is the UI. Before, ShortcutRegistration always held an ownerToken (sc-<uuid>) and baked it into eventInOwnerScope(event, '[...]'), so each registration crossed to the client as distinct filter text. After, getOwnerToken() returns null for a UI owner and the filter becomes the constant eventInTopLevelScope(event) (new helper in FlowShortcut.js) — identical for every registration, which per the PR description and #25624 is what stops the per-registration growth of the client expression cache and constant pools.

Diagram Bot draws the mechanism this pull request touches; it does not review the change. Verify it against the diff.

Generated by Diagram Bot for issue #25643 · ◷

sonarqubecloud · 2026-09-10T13:56:28Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

github-actions · 2026-09-10T14:03:43Z

Test Results

1 439 files + 3 1 523 suites +3 1h 38m 22s ⏱️ - 1m 28s
12 028 tests +39 11 960 ✅ +40 68 💤 ±0 0 ❌ ±0
12 346 runs +41 12 278 ✅ +42 68 💤 ±0 0 ❌ ±0

Results for commit 3d7d01a. ± Comparison against base commit 3c890d9.

github-actions Bot added the +0.0.1 label Sep 10, 2026

vaadin-review-bot approved these changes Sep 10, 2026

View reviewed changes

vaadin-bot enabled auto-merge (squash) September 10, 2026 13:52

vaadin-bot merged commit cee0b87 into 25.3 Sep 10, 2026
42 checks passed

vaadin-bot deleted the cherry-pick-25631-to-25.3-1789047546915 branch September 10, 2026 14:04

Navigation Menu

Uh oh!

fix: skip the shortcut owner token when the owner is the UI (#25631) (CP: 25.3) - #25643

fix: skip the shortcut owner token when the owner is the UI (#25631) (CP: 25.3)#25643
vaadin-bot merged 1 commit into
25.3from
cherry-pick-25631-to-25.3-1789047546915

vaadin-bot commented Sep 10, 2026

Uh oh!

vaadin-bot commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

sonarqubecloud Bot commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

#	Status	What the test verifies	Why it matters
1	✅	A UI-owned shortcut leaves the UI element unmarked and its filter is `eventInTopLevelScope(event)`, with no owner attribute or token in the text	This is the fix; a token here is what leaked per registration
2	✅	Removing and re-registering the same UI shortcut produces exactly the same set of keydown expressions	A filter text that differs per registration grows the client expression cache and both constant pools forever
3	✅	A component-owned shortcut still marks its owner and its filter locates that owner by the marker token	The normal, non-UI path must keep working unchanged
4	✅	In a real browser, a UI-owned Alt+S stays silent for a keydown inside an open popover and fires once for a keydown on the top layer	The new token-free client helper must guard exactly like the token-based one
5	✅	A shortcut owned by a component inside a popover still fires for keydowns from that popover (#24974)	Guards against re-breaking the earlier fix while the test view was reworked

Uh oh!

Conversation

vaadin-bot commented Sep 10, 2026

This PR cherry-picks changes from the original PR #25631 to branch 25.3.

Original PR description

Summary

What changed

Test summary

Uh oh!

vaadin-bot commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 10, 2026

A UI-owned shortcut now emits one shared keydown filter instead of a unique one per registration

Uh oh!

sonarqubecloud Bot commented Sep 10, 2026

Quality Gate passed

Uh oh!

github-actions Bot commented Sep 10, 2026

Test Results

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants