diff --git a/lib/web_ui/lib/src/engine/semantics/semantics.dart b/lib/web_ui/lib/src/engine/semantics/semantics.dart index 6eca6cd3432d8d5a440d6c67d336b716e20c1395..032a88f9972ae64c55d0dd77d37129f4be7791fb 100644 --- a/lib/web_ui/lib/src/engine/semantics/semantics.dart +++ b/lib/web_ui/lib/src/engine/semantics/semantics.dart @@ -1480,7 +1480,18 @@ class EngineSemanticsOwner { /// Updates the semantics tree from data in the [uiUpdate]. void updateSemantics(ui.SemanticsUpdate uiUpdate) { if (!_semanticsEnabled) { - return; + if (ui.debugEmulateFlutterTesterEnvironment) { + // Running Flutter widget tests in a fake environment. Don't enable + // engine semantics. Test semantics trees violate invariants in ways + // production implementation isn't built to handle. For example, tests + // routinely reset semantics node IDs, which is messing up the update + // process. + return; + } else { + // Running a real app. Auto-enable engine semantics. + semanticsHelper.dispose(); // placeholder no longer needed + semanticsEnabled = true; + } } final SemanticsUpdate update = uiUpdate as SemanticsUpdate; diff --git a/lib/web_ui/lib/src/engine/semantics/semantics_helper.dart b/lib/web_ui/lib/src/engine/semantics/semantics_helper.dart index 4908f7c189355336c1fc982e0e0f15b4187fa6f6..737e50847dbc52ae8423d6b28b91e7984e68123f 100644 --- a/lib/web_ui/lib/src/engine/semantics/semantics_helper.dart +++ b/lib/web_ui/lib/src/engine/semantics/semantics_helper.dart @@ -51,6 +51,15 @@ class SemanticsHelper { html.Element prepareAccessibilityPlaceholder() { return _semanticsEnabler.prepareAccessibilityPlaceholder(); } + + /// Stops waiting for the user to enable semantics and removes the + /// placeholder. + /// + /// This is used when semantics is enabled programmatically and therefore the + /// placehodler is no longer needed. + void dispose() { + _semanticsEnabler.dispose(); + } } @visibleForTesting @@ -92,44 +101,34 @@ abstract class SemanticsEnabler { /// /// If not they are sent to framework as normal events. bool get isWaitingToEnableSemantics; + + /// Stops waiting for the user to enable semantics and removes the placeholder. + void dispose(); } +/// The desktop semantics enabler uses a simpler strategy compared to mobile. +/// +/// A placeholder element is created completely outside the view and is not +/// reachable via touch or mouse. Assistive technology can still find it either +/// using keyboard shortcuts or via next/previous touch gesture (for touch +/// screens). This simplification removes the need for pointer event +/// disambiguation or timers. The placeholder simply waits for a click event +/// and enables semantics. @visibleForTesting class DesktopSemanticsEnabler extends SemanticsEnabler { - /// We do not immediately enable semantics when the user requests it, but - /// instead wait for a short period of time before doing it. This is because - /// the request comes as an event targeted on the [_semanticsPlaceholder]. - /// This event, depending on the browser, comes as a burst of events. - /// For example, Safari on MacOS sends "pointerup", "pointerdown". So during a - /// short time period we consume all events and prevent forwarding to the - /// framework. Otherwise, the events will be interpreted twice, once as a - /// request to activate semantics, and a second time by Flutter's gesture - /// recognizers. - @visibleForTesting - Timer? semanticsActivationTimer; - /// A temporary placeholder used to capture a request to activate semantics. html.Element? _semanticsPlaceholder; - /// The number of events we processed that could potentially activate - /// semantics. - int semanticsActivationAttempts = 0; - - /// Instructs [_tryEnableSemantics] to remove [_semanticsPlaceholder]. - /// - /// The placeholder is removed upon any next event. - bool _schedulePlaceholderRemoval = false; - /// Whether we are waiting for the user to enable semantics. @override bool get isWaitingToEnableSemantics => _semanticsPlaceholder != null; @override bool tryEnableSemantics(html.Event event) { - if (_schedulePlaceholderRemoval) { - _semanticsPlaceholder!.remove(); - _semanticsPlaceholder = null; - semanticsActivationTimer = null; + // Semantics may be enabled programmatically. If there's a race between that + // and the DOM event, we may end up here while there's no longer a placeholder + // to work with. + if (!isWaitingToEnableSemantics) { return true; } @@ -154,37 +153,17 @@ class DesktopSemanticsEnabler extends SemanticsEnabler { return true; } - semanticsActivationAttempts += 1; - if (semanticsActivationAttempts >= kMaxSemanticsActivationAttempts) { - // We have received multiple user events, none of which resulted in - // semantics activation. This is a signal that the user is not interested - // in semantics, and so we will stop waiting for it. - _schedulePlaceholderRemoval = true; - return true; - } - - if (semanticsActivationTimer != null) { - // We are in a waiting period to activate a timer. While the timer is - // active we should consume events pertaining to semantics activation. - // Otherwise the event will also be interpreted by the framework and - // potentially result in activating a gesture in the app. - return false; - } - // Check for the event target. final bool enableConditionPassed = (event.target == _semanticsPlaceholder); - if (enableConditionPassed) { - assert(semanticsActivationTimer == null); - semanticsActivationTimer = Timer(_periodToConsumeEvents, () { - EngineSemanticsOwner.instance.semanticsEnabled = true; - _schedulePlaceholderRemoval = true; - }); - return false; + if (!enableConditionPassed) { + // This was not a semantics activating event; forward as normal. + return true; } - // This was not a semantics activating event; forward as normal. - return true; + EngineSemanticsOwner.instance.semanticsEnabled = true; + dispose(); + return false; } @override @@ -199,7 +178,7 @@ class DesktopSemanticsEnabler extends SemanticsEnabler { // Adding roles to semantics placeholder. 'aria-live' will make sure that // the content is announced to the assistive technology user as soon as the - // page receives focus. 'tab-index' makes sure the button is the first + // page receives focus. 'tabindex' makes sure the button is the first // target of tab. 'aria-label' is used to define the placeholder message // to the assistive technology user. placeholder @@ -207,6 +186,8 @@ class DesktopSemanticsEnabler extends SemanticsEnabler { ..setAttribute('aria-live', 'true') ..setAttribute('tabindex', '0') ..setAttribute('aria-label', placeholderMessage); + + // The placeholder sits just outside the window so only AT can reach it. placeholder.style ..position = 'absolute' ..left = '-1px' @@ -215,6 +196,12 @@ class DesktopSemanticsEnabler extends SemanticsEnabler { ..height = '1px'; return placeholder; } + + @override + void dispose() { + _semanticsPlaceholder?.remove(); + _semanticsPlaceholder = null; + } } @visibleForTesting @@ -254,6 +241,13 @@ class MobileSemanticsEnabler extends SemanticsEnabler { @override bool tryEnableSemantics(html.Event event) { + // Semantics may be enabled programmatically. If there's a race between that + // and the DOM event, we may end up here while there's no longer a placeholder + // to work with. + if (!isWaitingToEnableSemantics) { + return true; + } + if (_schedulePlaceholderRemoval) { // The event type can also be click for VoiceOver. final bool removeNow = (browserEngine != BrowserEngine.webkit || @@ -261,9 +255,7 @@ class MobileSemanticsEnabler extends SemanticsEnabler { event.type == 'pointerup' || event.type == 'click'); if (removeNow) { - _semanticsPlaceholder!.remove(); - _semanticsPlaceholder = null; - semanticsActivationTimer = null; + dispose(); } return true; } @@ -403,4 +395,11 @@ class MobileSemanticsEnabler extends SemanticsEnabler { return placeholder; } + + @override + void dispose() { + _semanticsPlaceholder?.remove(); + _semanticsPlaceholder = null; + semanticsActivationTimer = null; + } } diff --git a/lib/web_ui/test/engine/semantics/semantics_helper_test.dart b/lib/web_ui/test/engine/semantics/semantics_helper_test.dart index 0df7f7449da3ef51464e78a7a442fb53224ba2d4..3833f80a4ee83cb0eabd125494bbba74ea8110be 100644 --- a/lib/web_ui/test/engine/semantics/semantics_helper_test.dart +++ b/lib/web_ui/test/engine/semantics/semantics_helper_test.dart @@ -28,10 +28,6 @@ void testMain() { if (_placeholder != null) { _placeholder.remove(); } - if (desktopSemanticsEnabler?.semanticsActivationTimer != null) { - desktopSemanticsEnabler.semanticsActivationTimer.cancel(); - desktopSemanticsEnabler.semanticsActivationTimer = null; - } }); test('prepare accesibility placeholder', () async { @@ -73,9 +69,7 @@ void testMain() { expect(shouldForwardToFramework, true); } - }, - // TODO(nurhan): https://github.com/flutter/flutter/issues/50754 - skip: browserEngine == BrowserEngine.edge); + }); test( 'Relevants events targeting placeholder should not be forwarded to the framework', @@ -93,31 +87,13 @@ void testMain() { expect(shouldForwardToFramework, false); }); - test( - 'After max number of relevant events, events should be forwarded to the framework', - () async { - // Prework. Attach the placeholder to dom. + test('disposes of the placeholder', () { _placeholder = desktopSemanticsEnabler.prepareAccessibilityPlaceholder(); html.document.body.append(_placeholder); - html.Event event = html.MouseEvent('mousedown'); - _placeholder.dispatchEvent(event); - - bool shouldForwardToFramework = - desktopSemanticsEnabler.tryEnableSemantics(event); - - expect(shouldForwardToFramework, false); - - // Send max number of events; - for (int i = 1; i <= kMaxSemanticsActivationAttempts; i++) { - event = html.MouseEvent('mousedown'); - _placeholder.dispatchEvent(event); - - shouldForwardToFramework = - desktopSemanticsEnabler.tryEnableSemantics(event); - } - - expect(shouldForwardToFramework, true); + expect(_placeholder.isConnected, isTrue); + desktopSemanticsEnabler.dispose(); + expect(_placeholder.isConnected, isFalse); }); }); @@ -168,7 +144,5 @@ void testMain() { expect(shouldForwardToFramework, true); }); }, // Run the `MobileSemanticsEnabler` only on mobile browsers. - skip: operatingSystem == OperatingSystem.linux || - operatingSystem == OperatingSystem.macOs || - operatingSystem == OperatingSystem.windows); + skip: isDesktop); } diff --git a/lib/web_ui/test/engine/semantics/semantics_test.dart b/lib/web_ui/test/engine/semantics/semantics_test.dart index 4b2aee5bd9bcefd2727a06fe0387dad5fdc94b6a..6fa97764f62838ad40c7c9e36511df7622d145e3 100644 --- a/lib/web_ui/test/engine/semantics/semantics_test.dart +++ b/lib/web_ui/test/engine/semantics/semantics_test.dart @@ -83,23 +83,60 @@ void _testEngineSemanticsOwner() { expect(semantics().mode, AccessibilityMode.unknown); }); - test('auto-enables semantics', () async { + test('placeholder enables semantics', () async { domRenderer.reset(); // triggers `autoEnableOnTap` to be called expect(semantics().semanticsEnabled, false); // Synthesize a click on the placeholder. final html.Element placeholder = html.document.querySelectorAll('flt-semantics-placeholder').single; + expect(placeholder.isConnected, true); final html.Rectangle rect = placeholder.getBoundingClientRect(); placeholder.dispatchEvent(html.MouseEvent( 'click', clientX: (rect.left + (rect.right - rect.left) / 2).floor(), clientY: (rect.top + (rect.bottom - rect.top) / 2).floor(), )); - while (!semantics().semanticsEnabled) { - await Future.delayed(const Duration(milliseconds: 50)); + + // On mobile semantics is not enabled synchronously. This is because the + // placeholder receives pointer events in non-accessibility mode too, and + // therefore we wait to see if any subsequent pointer events are issued + // indicating that this is not a request to enable accessibility. + if (isMobile) { + while (!semantics().semanticsEnabled) { + await Future.delayed(const Duration(milliseconds: 50)); + } + } + expect(semantics().semanticsEnabled, true); + + // The placeholder should be removed + if (isMobile) { + // On mobile the placeholder is not removed synchronously. Instead it is + // removed upon the next DOM event. Otherwise Safari swallows pointerup. + expect(placeholder.isConnected, true); + placeholder.click(); + await Future.delayed(Duration.zero); } + expect(placeholder.isConnected, false); + }); + + test('auto-enables semantics', () async { + domRenderer.reset(); // triggers `autoEnableOnTap` to be called + expect(semantics().semanticsEnabled, false); + + final html.Element placeholder = + html.document.querySelectorAll('flt-semantics-placeholder').single; + expect(placeholder.isConnected, true); + + // Sending a semantics update should auto-enable engine semantics. + final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); + updateNode(builder, id: 0); + semantics().updateSemantics(builder.build()); + expect(semantics().semanticsEnabled, true); + + // The placeholder should be removed + expect(placeholder.isConnected, false); }); void renderLabel(String label) {