Skip to content

Commit d27125f

Browse files
authored
[web] Send the correct view ID with semantics actions (flutter/engine#56595)
Currently, all semantics actions that we send to the framework contain `viewId: 0`. This leads to issues such as flutter#158530 because the framework routes the pointer event to the wrong view. Let's send the correct view ID with semantics actions. Fixes flutter#158530
1 parent 453afd6 commit d27125f

11 files changed

+47
-23
lines changed

engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher.dart

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1298,14 +1298,18 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher {
12981298
/// Engine code should use this method instead of the callback directly.
12991299
/// Otherwise zones won't work properly.
13001300
void invokeOnSemanticsAction(
1301-
int nodeId, ui.SemanticsAction action, ByteData? args) {
1301+
int viewId,
1302+
int nodeId,
1303+
ui.SemanticsAction action,
1304+
ByteData? args,
1305+
) {
13021306
invoke1<ui.SemanticsActionEvent>(
13031307
_onSemanticsActionEvent,
13041308
_onSemanticsActionEventZone,
13051309
ui.SemanticsActionEvent(
13061310
type: action,
13071311
nodeId: nodeId,
1308-
viewId: 0, // TODO(goderbauer): Wire up the real view ID.
1312+
viewId: viewId,
13091313
arguments: args,
13101314
),
13111315
);

engine/src/flutter/lib/web_ui/lib/src/engine/pointer_binding.dart

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ class ClickDebouncer {
271271
/// Forwards the event to the framework, unless it is deduplicated because
272272
/// the corresponding pointer down/up events were recently flushed to the
273273
/// framework already.
274-
void onClick(DomEvent click, int semanticsNodeId, bool isListening) {
274+
void onClick(DomEvent click, int viewId, int semanticsNodeId, bool isListening) {
275275
assert(click.type == 'click');
276276

277277
if (!isDebouncing) {
@@ -280,7 +280,7 @@ class ClickDebouncer {
280280
// recently and if the node is currently listening to event, forward to
281281
// the framework.
282282
if (isListening && _shouldSendClickEventToFramework(click)) {
283-
_sendSemanticsTapToFramework(click, semanticsNodeId);
283+
_sendSemanticsTapToFramework(click, viewId, semanticsNodeId);
284284
}
285285
return;
286286
}
@@ -292,7 +292,7 @@ class ClickDebouncer {
292292
final DebounceState state = _state!;
293293
_state = null;
294294
state.timer.cancel();
295-
_sendSemanticsTapToFramework(click, semanticsNodeId);
295+
_sendSemanticsTapToFramework(click, viewId, semanticsNodeId);
296296
} else {
297297
// The semantic node is not listening to taps. Flush the pointer events
298298
// for the framework to figure out what to do with them. It's possible
@@ -301,7 +301,11 @@ class ClickDebouncer {
301301
}
302302
}
303303

304-
void _sendSemanticsTapToFramework(DomEvent click, int semanticsNodeId) {
304+
void _sendSemanticsTapToFramework(
305+
DomEvent click,
306+
int viewId,
307+
int semanticsNodeId,
308+
) {
305309
// Tappable nodes can be nested inside other tappable nodes. If a click
306310
// lands on an inner element and is allowed to propagate, it will also
307311
// land on the ancestor tappable, leading to both the descendant and the
@@ -312,7 +316,11 @@ class ClickDebouncer {
312316
click.stopPropagation();
313317

314318
EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
315-
semanticsNodeId, ui.SemanticsAction.tap, null);
319+
viewId,
320+
semanticsNodeId,
321+
ui.SemanticsAction.tap,
322+
null,
323+
);
316324
reset();
317325
}
318326

engine/src/flutter/lib/web_ui/lib/src/engine/semantics/focusable.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ class AccessibilityFocusManager {
206206
// shifting focus.
207207
if (_lastEvent != AccessibilityFocusManagerEvent.requestedFocus) {
208208
EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
209+
_owner.viewId,
209210
target.semanticsNodeId,
210211
ui.SemanticsAction.focus,
211212
null,

engine/src/flutter/lib/web_ui/lib/src/engine/semantics/incrementable.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,11 @@ class SemanticIncrementable extends SemanticRole {
4343
if (newInputValue > _currentSurrogateValue) {
4444
_currentSurrogateValue += 1;
4545
EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
46-
semanticsObject.id, ui.SemanticsAction.increase, null);
46+
viewId, semanticsObject.id, ui.SemanticsAction.increase, null);
4747
} else if (newInputValue < _currentSurrogateValue) {
4848
_currentSurrogateValue -= 1;
4949
EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
50-
semanticsObject.id, ui.SemanticsAction.decrease, null);
50+
viewId, semanticsObject.id, ui.SemanticsAction.decrease, null);
5151
}
5252
}));
5353

engine/src/flutter/lib/web_ui/lib/src/engine/semantics/scrollable.dart

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,20 +76,20 @@ class SemanticScrollable extends SemanticRole {
7676
if (doScrollForward) {
7777
if (semanticsObject.isVerticalScrollContainer) {
7878
EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
79-
semanticsId, ui.SemanticsAction.scrollUp, null);
79+
viewId, semanticsId, ui.SemanticsAction.scrollUp, null);
8080
} else {
8181
assert(semanticsObject.isHorizontalScrollContainer);
8282
EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
83-
semanticsId, ui.SemanticsAction.scrollLeft, null);
83+
viewId, semanticsId, ui.SemanticsAction.scrollLeft, null);
8484
}
8585
} else {
8686
if (semanticsObject.isVerticalScrollContainer) {
8787
EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
88-
semanticsId, ui.SemanticsAction.scrollDown, null);
88+
viewId, semanticsId, ui.SemanticsAction.scrollDown, null);
8989
} else {
9090
assert(semanticsObject.isHorizontalScrollContainer);
9191
EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
92-
semanticsId, ui.SemanticsAction.scrollRight, null);
92+
viewId, semanticsId, ui.SemanticsAction.scrollRight, null);
9393
}
9494
}
9595
}

engine/src/flutter/lib/web_ui/lib/src/engine/semantics/semantics.dart

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,9 @@ abstract class SemanticRole {
444444
/// The semantics object managed by this role.
445445
final SemanticsObject semanticsObject;
446446

447+
/// The ID of the Flutter View that this [SemanticRole] belongs to.
448+
int get viewId => semanticsObject.owner.viewId;
449+
447450
/// Whether this role accepts pointer events.
448451
///
449452
/// This boolean decides whether to set the `pointer-events` CSS property to
@@ -764,6 +767,9 @@ abstract class SemanticBehavior {
764767

765768
final SemanticRole owner;
766769

770+
/// The ID of the Flutter View that this [SemanticBehavior] belongs to.
771+
int get viewId => semanticsObject.owner.viewId;
772+
767773
/// Whether this role accepts pointer events.
768774
///
769775
/// This boolean decides whether to set the `pointer-events` CSS property to
@@ -2381,12 +2387,15 @@ class EngineSemantics {
23812387

23822388
/// The top-level service that manages everything semantics-related.
23832389
class EngineSemanticsOwner {
2384-
EngineSemanticsOwner(this.semanticsHost) {
2390+
EngineSemanticsOwner(this.viewId, this.semanticsHost) {
23852391
registerHotRestartListener(() {
23862392
_rootSemanticsElement?.remove();
23872393
});
23882394
}
23892395

2396+
/// The ID of the Flutter View that this semantics owner belongs to.
2397+
final int viewId;
2398+
23902399
/// The permanent element in the view's DOM structure that hosts the semantics
23912400
/// tree.
23922401
///

engine/src/flutter/lib/web_ui/lib/src/engine/semantics/tappable.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ class Tappable extends SemanticBehavior {
4646
_clickListener = createDomEventListener((DomEvent click) {
4747
PointerBinding.clickDebouncer.onClick(
4848
click,
49+
viewId,
4950
semanticsObject.id,
5051
_isListening,
5152
);

engine/src/flutter/lib/web_ui/lib/src/engine/semantics/text_field.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ class SemanticTextField extends SemanticRole {
278278
// IMPORTANT: because this event listener can be triggered by either or
279279
// both a "focus" and a "click" DOM events, this code must be idempotent.
280280
EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
281-
semanticsObject.id, ui.SemanticsAction.focus, null);
281+
viewId, semanticsObject.id, ui.SemanticsAction.focus, null);
282282
}));
283283
editableElement.addEventListener('click', createDomEventListener((DomEvent event) {
284284
editableElement.focusWithoutScroll();

engine/src/flutter/lib/web_ui/lib/src/engine/window.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ class EngineFlutterView implements ui.FlutterView {
157157

158158
final JsViewConstraints? _jsViewConstraints;
159159

160-
late final EngineSemanticsOwner semantics = EngineSemanticsOwner(dom.semanticsHost);
160+
late final EngineSemanticsOwner semantics = EngineSemanticsOwner(viewId, dom.semanticsHost);
161161

162162
@override
163163
ui.Size get physicalSize {

engine/src/flutter/lib/web_ui/test/engine/pointer_binding_test.dart

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3021,7 +3021,7 @@ void _testClickDebouncer({required PointerBinding Function() getBinding}) {
30213021
}
30223022
);
30233023

3024-
PointerBinding.clickDebouncer.onClick(click, 42, true);
3024+
PointerBinding.clickDebouncer.onClick(click, view.viewId, 42, true);
30253025
expect(PointerBinding.clickDebouncer.isDebouncing, false);
30263026
expect(pointerPackets, isEmpty);
30273027
expect(semanticsActions, <CapturedSemanticsEvent>[
@@ -3046,7 +3046,7 @@ void _testClickDebouncer({required PointerBinding Function() getBinding}) {
30463046
}
30473047
);
30483048

3049-
PointerBinding.clickDebouncer.onClick(click, 42, true);
3049+
PointerBinding.clickDebouncer.onClick(click, view.viewId, 42, true);
30503050
expect(pointerPackets, isEmpty);
30513051
expect(semanticsActions, <CapturedSemanticsEvent>[
30523052
(type: ui.SemanticsAction.tap, nodeId: 42)
@@ -3070,7 +3070,7 @@ void _testClickDebouncer({required PointerBinding Function() getBinding}) {
30703070
}
30713071
);
30723072

3073-
PointerBinding.clickDebouncer.onClick(click, 42, false);
3073+
PointerBinding.clickDebouncer.onClick(click, view.viewId, 42, false);
30743074
expect(
30753075
reason: 'When tappable declares that it is not listening to click events '
30763076
'the debouncer flushes the pointer events to the framework and '
@@ -3129,7 +3129,7 @@ void _testClickDebouncer({required PointerBinding Function() getBinding}) {
31293129
'clientY': testElement.getBoundingClientRect().y,
31303130
}
31313131
);
3132-
PointerBinding.clickDebouncer.onClick(click, 42, true);
3132+
PointerBinding.clickDebouncer.onClick(click, view.viewId, 42, true);
31333133

31343134
expect(
31353135
reason: 'Because the DOM click event was deduped.',
@@ -3190,7 +3190,7 @@ void _testClickDebouncer({required PointerBinding Function() getBinding}) {
31903190
'clientY': testElement.getBoundingClientRect().y,
31913191
}
31923192
);
3193-
PointerBinding.clickDebouncer.onClick(click, 42, true);
3193+
PointerBinding.clickDebouncer.onClick(click, view.viewId, 42, true);
31943194

31953195
expect(
31963196
reason: 'Because the DOM click event was deduped.',
@@ -3245,7 +3245,7 @@ void _testClickDebouncer({required PointerBinding Function() getBinding}) {
32453245
'clientY': testElement.getBoundingClientRect().y,
32463246
}
32473247
);
3248-
PointerBinding.clickDebouncer.onClick(click, 42, true);
3248+
PointerBinding.clickDebouncer.onClick(click, view.viewId, 42, true);
32493249

32503250
expect(
32513251
reason: 'The DOM click should still be sent to the framework because it '

engine/src/flutter/lib/web_ui/test/engine/window_test.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,8 @@ Future<void> testMain() async {
239239
expect(ui.PlatformDispatcher.instance.onSemanticsActionEvent, same(callback));
240240
});
241241

242-
EnginePlatformDispatcher.instance.invokeOnSemanticsAction(0, ui.SemanticsAction.tap, null);
242+
EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
243+
myWindow.viewId, 0, ui.SemanticsAction.tap, null);
243244
});
244245

245246
test('onAccessibilityFeaturesChanged preserves the zone', () {

0 commit comments

Comments
 (0)