[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 https://github.com/flutter/flutter/issues/158530 because the framework routes the pointer event to the wrong view.

Let's send the correct view ID with semantics actions.

Fixes https://github.com/flutter/flutter/issues/158530
This commit is contained in:
Mouad Debbar 2024-11-15 15:28:59 -05:00 committed by GitHub
parent 453afd650f
commit d27125f326
11 changed files with 47 additions and 23 deletions

View File

@ -1298,14 +1298,18 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher {
/// Engine code should use this method instead of the callback directly. /// Engine code should use this method instead of the callback directly.
/// Otherwise zones won't work properly. /// Otherwise zones won't work properly.
void invokeOnSemanticsAction( void invokeOnSemanticsAction(
int nodeId, ui.SemanticsAction action, ByteData? args) { int viewId,
int nodeId,
ui.SemanticsAction action,
ByteData? args,
) {
invoke1<ui.SemanticsActionEvent>( invoke1<ui.SemanticsActionEvent>(
_onSemanticsActionEvent, _onSemanticsActionEvent,
_onSemanticsActionEventZone, _onSemanticsActionEventZone,
ui.SemanticsActionEvent( ui.SemanticsActionEvent(
type: action, type: action,
nodeId: nodeId, nodeId: nodeId,
viewId: 0, // TODO(goderbauer): Wire up the real view ID. viewId: viewId,
arguments: args, arguments: args,
), ),
); );

View File

@ -271,7 +271,7 @@ class ClickDebouncer {
/// Forwards the event to the framework, unless it is deduplicated because /// Forwards the event to the framework, unless it is deduplicated because
/// the corresponding pointer down/up events were recently flushed to the /// the corresponding pointer down/up events were recently flushed to the
/// framework already. /// framework already.
void onClick(DomEvent click, int semanticsNodeId, bool isListening) { void onClick(DomEvent click, int viewId, int semanticsNodeId, bool isListening) {
assert(click.type == 'click'); assert(click.type == 'click');
if (!isDebouncing) { if (!isDebouncing) {
@ -280,7 +280,7 @@ class ClickDebouncer {
// recently and if the node is currently listening to event, forward to // recently and if the node is currently listening to event, forward to
// the framework. // the framework.
if (isListening && _shouldSendClickEventToFramework(click)) { if (isListening && _shouldSendClickEventToFramework(click)) {
_sendSemanticsTapToFramework(click, semanticsNodeId); _sendSemanticsTapToFramework(click, viewId, semanticsNodeId);
} }
return; return;
} }
@ -292,7 +292,7 @@ class ClickDebouncer {
final DebounceState state = _state!; final DebounceState state = _state!;
_state = null; _state = null;
state.timer.cancel(); state.timer.cancel();
_sendSemanticsTapToFramework(click, semanticsNodeId); _sendSemanticsTapToFramework(click, viewId, semanticsNodeId);
} else { } else {
// The semantic node is not listening to taps. Flush the pointer events // The semantic node is not listening to taps. Flush the pointer events
// for the framework to figure out what to do with them. It's possible // for the framework to figure out what to do with them. It's possible
@ -301,7 +301,11 @@ class ClickDebouncer {
} }
} }
void _sendSemanticsTapToFramework(DomEvent click, int semanticsNodeId) { void _sendSemanticsTapToFramework(
DomEvent click,
int viewId,
int semanticsNodeId,
) {
// Tappable nodes can be nested inside other tappable nodes. If a click // Tappable nodes can be nested inside other tappable nodes. If a click
// lands on an inner element and is allowed to propagate, it will also // lands on an inner element and is allowed to propagate, it will also
// land on the ancestor tappable, leading to both the descendant and the // land on the ancestor tappable, leading to both the descendant and the
@ -312,7 +316,11 @@ class ClickDebouncer {
click.stopPropagation(); click.stopPropagation();
EnginePlatformDispatcher.instance.invokeOnSemanticsAction( EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
semanticsNodeId, ui.SemanticsAction.tap, null); viewId,
semanticsNodeId,
ui.SemanticsAction.tap,
null,
);
reset(); reset();
} }

View File

@ -206,6 +206,7 @@ class AccessibilityFocusManager {
// shifting focus. // shifting focus.
if (_lastEvent != AccessibilityFocusManagerEvent.requestedFocus) { if (_lastEvent != AccessibilityFocusManagerEvent.requestedFocus) {
EnginePlatformDispatcher.instance.invokeOnSemanticsAction( EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
_owner.viewId,
target.semanticsNodeId, target.semanticsNodeId,
ui.SemanticsAction.focus, ui.SemanticsAction.focus,
null, null,

View File

@ -43,11 +43,11 @@ class SemanticIncrementable extends SemanticRole {
if (newInputValue > _currentSurrogateValue) { if (newInputValue > _currentSurrogateValue) {
_currentSurrogateValue += 1; _currentSurrogateValue += 1;
EnginePlatformDispatcher.instance.invokeOnSemanticsAction( EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
semanticsObject.id, ui.SemanticsAction.increase, null); viewId, semanticsObject.id, ui.SemanticsAction.increase, null);
} else if (newInputValue < _currentSurrogateValue) { } else if (newInputValue < _currentSurrogateValue) {
_currentSurrogateValue -= 1; _currentSurrogateValue -= 1;
EnginePlatformDispatcher.instance.invokeOnSemanticsAction( EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
semanticsObject.id, ui.SemanticsAction.decrease, null); viewId, semanticsObject.id, ui.SemanticsAction.decrease, null);
} }
})); }));

View File

@ -76,20 +76,20 @@ class SemanticScrollable extends SemanticRole {
if (doScrollForward) { if (doScrollForward) {
if (semanticsObject.isVerticalScrollContainer) { if (semanticsObject.isVerticalScrollContainer) {
EnginePlatformDispatcher.instance.invokeOnSemanticsAction( EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
semanticsId, ui.SemanticsAction.scrollUp, null); viewId, semanticsId, ui.SemanticsAction.scrollUp, null);
} else { } else {
assert(semanticsObject.isHorizontalScrollContainer); assert(semanticsObject.isHorizontalScrollContainer);
EnginePlatformDispatcher.instance.invokeOnSemanticsAction( EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
semanticsId, ui.SemanticsAction.scrollLeft, null); viewId, semanticsId, ui.SemanticsAction.scrollLeft, null);
} }
} else { } else {
if (semanticsObject.isVerticalScrollContainer) { if (semanticsObject.isVerticalScrollContainer) {
EnginePlatformDispatcher.instance.invokeOnSemanticsAction( EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
semanticsId, ui.SemanticsAction.scrollDown, null); viewId, semanticsId, ui.SemanticsAction.scrollDown, null);
} else { } else {
assert(semanticsObject.isHorizontalScrollContainer); assert(semanticsObject.isHorizontalScrollContainer);
EnginePlatformDispatcher.instance.invokeOnSemanticsAction( EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
semanticsId, ui.SemanticsAction.scrollRight, null); viewId, semanticsId, ui.SemanticsAction.scrollRight, null);
} }
} }
} }

View File

@ -444,6 +444,9 @@ abstract class SemanticRole {
/// The semantics object managed by this role. /// The semantics object managed by this role.
final SemanticsObject semanticsObject; final SemanticsObject semanticsObject;
/// The ID of the Flutter View that this [SemanticRole] belongs to.
int get viewId => semanticsObject.owner.viewId;
/// Whether this role accepts pointer events. /// Whether this role accepts pointer events.
/// ///
/// This boolean decides whether to set the `pointer-events` CSS property to /// This boolean decides whether to set the `pointer-events` CSS property to
@ -764,6 +767,9 @@ abstract class SemanticBehavior {
final SemanticRole owner; final SemanticRole owner;
/// The ID of the Flutter View that this [SemanticBehavior] belongs to.
int get viewId => semanticsObject.owner.viewId;
/// Whether this role accepts pointer events. /// Whether this role accepts pointer events.
/// ///
/// This boolean decides whether to set the `pointer-events` CSS property to /// This boolean decides whether to set the `pointer-events` CSS property to
@ -2381,12 +2387,15 @@ class EngineSemantics {
/// The top-level service that manages everything semantics-related. /// The top-level service that manages everything semantics-related.
class EngineSemanticsOwner { class EngineSemanticsOwner {
EngineSemanticsOwner(this.semanticsHost) { EngineSemanticsOwner(this.viewId, this.semanticsHost) {
registerHotRestartListener(() { registerHotRestartListener(() {
_rootSemanticsElement?.remove(); _rootSemanticsElement?.remove();
}); });
} }
/// The ID of the Flutter View that this semantics owner belongs to.
final int viewId;
/// The permanent element in the view's DOM structure that hosts the semantics /// The permanent element in the view's DOM structure that hosts the semantics
/// tree. /// tree.
/// ///

View File

@ -46,6 +46,7 @@ class Tappable extends SemanticBehavior {
_clickListener = createDomEventListener((DomEvent click) { _clickListener = createDomEventListener((DomEvent click) {
PointerBinding.clickDebouncer.onClick( PointerBinding.clickDebouncer.onClick(
click, click,
viewId,
semanticsObject.id, semanticsObject.id,
_isListening, _isListening,
); );

View File

@ -278,7 +278,7 @@ class SemanticTextField extends SemanticRole {
// IMPORTANT: because this event listener can be triggered by either or // IMPORTANT: because this event listener can be triggered by either or
// both a "focus" and a "click" DOM events, this code must be idempotent. // both a "focus" and a "click" DOM events, this code must be idempotent.
EnginePlatformDispatcher.instance.invokeOnSemanticsAction( EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
semanticsObject.id, ui.SemanticsAction.focus, null); viewId, semanticsObject.id, ui.SemanticsAction.focus, null);
})); }));
editableElement.addEventListener('click', createDomEventListener((DomEvent event) { editableElement.addEventListener('click', createDomEventListener((DomEvent event) {
editableElement.focusWithoutScroll(); editableElement.focusWithoutScroll();

View File

@ -157,7 +157,7 @@ class EngineFlutterView implements ui.FlutterView {
final JsViewConstraints? _jsViewConstraints; final JsViewConstraints? _jsViewConstraints;
late final EngineSemanticsOwner semantics = EngineSemanticsOwner(dom.semanticsHost); late final EngineSemanticsOwner semantics = EngineSemanticsOwner(viewId, dom.semanticsHost);
@override @override
ui.Size get physicalSize { ui.Size get physicalSize {

View File

@ -3021,7 +3021,7 @@ void _testClickDebouncer({required PointerBinding Function() getBinding}) {
} }
); );
PointerBinding.clickDebouncer.onClick(click, 42, true); PointerBinding.clickDebouncer.onClick(click, view.viewId, 42, true);
expect(PointerBinding.clickDebouncer.isDebouncing, false); expect(PointerBinding.clickDebouncer.isDebouncing, false);
expect(pointerPackets, isEmpty); expect(pointerPackets, isEmpty);
expect(semanticsActions, <CapturedSemanticsEvent>[ expect(semanticsActions, <CapturedSemanticsEvent>[
@ -3046,7 +3046,7 @@ void _testClickDebouncer({required PointerBinding Function() getBinding}) {
} }
); );
PointerBinding.clickDebouncer.onClick(click, 42, true); PointerBinding.clickDebouncer.onClick(click, view.viewId, 42, true);
expect(pointerPackets, isEmpty); expect(pointerPackets, isEmpty);
expect(semanticsActions, <CapturedSemanticsEvent>[ expect(semanticsActions, <CapturedSemanticsEvent>[
(type: ui.SemanticsAction.tap, nodeId: 42) (type: ui.SemanticsAction.tap, nodeId: 42)
@ -3070,7 +3070,7 @@ void _testClickDebouncer({required PointerBinding Function() getBinding}) {
} }
); );
PointerBinding.clickDebouncer.onClick(click, 42, false); PointerBinding.clickDebouncer.onClick(click, view.viewId, 42, false);
expect( expect(
reason: 'When tappable declares that it is not listening to click events ' reason: 'When tappable declares that it is not listening to click events '
'the debouncer flushes the pointer events to the framework and ' 'the debouncer flushes the pointer events to the framework and '
@ -3129,7 +3129,7 @@ void _testClickDebouncer({required PointerBinding Function() getBinding}) {
'clientY': testElement.getBoundingClientRect().y, 'clientY': testElement.getBoundingClientRect().y,
} }
); );
PointerBinding.clickDebouncer.onClick(click, 42, true); PointerBinding.clickDebouncer.onClick(click, view.viewId, 42, true);
expect( expect(
reason: 'Because the DOM click event was deduped.', reason: 'Because the DOM click event was deduped.',
@ -3190,7 +3190,7 @@ void _testClickDebouncer({required PointerBinding Function() getBinding}) {
'clientY': testElement.getBoundingClientRect().y, 'clientY': testElement.getBoundingClientRect().y,
} }
); );
PointerBinding.clickDebouncer.onClick(click, 42, true); PointerBinding.clickDebouncer.onClick(click, view.viewId, 42, true);
expect( expect(
reason: 'Because the DOM click event was deduped.', reason: 'Because the DOM click event was deduped.',
@ -3245,7 +3245,7 @@ void _testClickDebouncer({required PointerBinding Function() getBinding}) {
'clientY': testElement.getBoundingClientRect().y, 'clientY': testElement.getBoundingClientRect().y,
} }
); );
PointerBinding.clickDebouncer.onClick(click, 42, true); PointerBinding.clickDebouncer.onClick(click, view.viewId, 42, true);
expect( expect(
reason: 'The DOM click should still be sent to the framework because it ' reason: 'The DOM click should still be sent to the framework because it '

View File

@ -239,7 +239,8 @@ Future<void> testMain() async {
expect(ui.PlatformDispatcher.instance.onSemanticsActionEvent, same(callback)); expect(ui.PlatformDispatcher.instance.onSemanticsActionEvent, same(callback));
}); });
EnginePlatformDispatcher.instance.invokeOnSemanticsAction(0, ui.SemanticsAction.tap, null); EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
myWindow.viewId, 0, ui.SemanticsAction.tap, null);
}); });
test('onAccessibilityFeaturesChanged preserves the zone', () { test('onAccessibilityFeaturesChanged preserves the zone', () {