Fixes IgnorePointer and AbsorbPointer to only block user interactions… #120619

chunhtai · 2023-02-13T20:01:04Z

… in a11y

Fixes #59402
Fixes #37162

migration guide: flutter/website#8312

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

chunhtai · 2023-02-13T20:03:18Z

packages/flutter/lib/src/semantics/semantics.dart

+  /// Setting this true prevents user from activating pointer related
+  /// [SemanticsAction]s, such as [SemanticsAction.tap] or
+  /// [SemanticsAction.longPress].
+  bool get isUserInteractionsBlocked => _isUserInteractionsBlocked;


was curious if there is a better name

Maybe: blocksUserActions or blocksSemanticsActions (in general, maybe we should stick with the "actions" terminology throughout this PR to make it clear that this relates to SemanticsActions)?

I agree to change this to "Actions". but I may keep it past tense since it is already a done deal in SemanticsNode level. Present tense kinda implies it will do something that it hasn't done.

chunhtai · 2023-02-13T20:06:10Z

packages/flutter/lib/src/widgets/basic.dart

  const IgnorePointer({
    super.key,
    this.ignoring = true,
-    this.ignoringSemantics,
+    this.ignoringSemantics = false,


I think the ignoringSemantics was a workaround to provide people a way to get the semanitcs back if ignoring = true, but I am not sure.

If so, should we deprecate this and just ask developer to use ExcludeSemantics?

goderbauer · 2023-02-13T23:46:33Z

Looks like this is failing a lot of checks.

chunhtai · 2023-02-14T22:55:25Z

Sorry for the test failure. I fixed the test, this is ready for review

goderbauer

Thanks for looking into this long-standing issue!

packages/flutter/lib/src/semantics/semantics.dart

packages/flutter/test/widgets/basic_test.dart

goderbauer · 2023-02-15T01:03:59Z

packages/flutter/test/widgets/basic_test.dart

+    testWidgets('ignores user interactions', (WidgetTester tester) async {
+      final UniqueKey key = UniqueKey();
+      await tester.pumpWidget(
+          MaterialApp(


packages/flutter/test/widgets/absorb_pointer_test.dart

goderbauer · 2023-02-15T01:36:14Z

packages/flutter/lib/src/semantics/semantics.dart

+    if (child.isBlockingUserInteractions) {
+      child._actions.forEach((SemanticsAction key, SemanticsActionHandler value) {
+        if (_kSystemInteractions & key.index > 0) {
+          _actions[key] = value;
+        }
+      });
+    } else {
+      _actions.addAll(child._actions);
+    }


for my own understanding: could we skip this filtering? Just adjusting the _actionsAsBits should be enough to determine whether an action is supported or not, right?

I added this filtering because I was trying to support use case where merging child that has same action but blocked. I guess that is no longer the case.

Even without that requirement, why do we want to add a action to the map that will be blocked anyway?

It's fine to do that. I was just thinking if we don't have to do it what's the point of implementing all this extra logic?

packages/flutter/lib/src/semantics/semantics.dart

packages/flutter/lib/src/rendering/object.dart

goderbauer · 2023-02-15T17:43:36Z

packages/flutter/lib/src/semantics/semantics.dart

+  /// Setting this true prevents user from activating pointer related
+  /// [SemanticsAction]s, such as [SemanticsAction.tap] or
+  /// [SemanticsAction.longPress].
+  bool get isUserInteractionsBlocked => _isUserInteractionsBlocked;


Maybe: blocksUserActions or blocksSemanticsActions (in general, maybe we should stick with the "actions" terminology throughout this PR to make it clear that this relates to SemanticsActions)?

goderbauer · 2023-02-15T17:45:28Z

packages/flutter/lib/src/semantics/semantics.dart

+  /// Setting this true prevents user from activating pointer related
+  /// [SemanticsAction]s, such as [SemanticsAction.tap] or
+  /// [SemanticsAction.longPress].
+  bool isBlockingUserInteractions = false;


Random thought: Is there ever a use case for only blocking certain actions? I wonder if this should just take a list of blocked actions (that we internally translate to a bitMask right away) and provide a const value containing all block-able actions that the use cases in this PR would use. Would be a little more flexible, but not sure how useful that is...

I haven't found use cases other than blocking pointer related action. We can add a new api to block a list instead of just hardcoded actions in the future if we have more use cases, and then we can reimplment this boolean to use that API under the hood.

I was suspecting this may be an one-off case, that is why I go with hardcoded actions.

packages/flutter/lib/src/rendering/object.dart

goderbauer · 2023-02-25T00:35:34Z

packages/flutter/lib/src/widgets/basic.dart

@@ -7030,12 +7090,29 @@ class Semantics extends SingleChildRenderObjectWidget {
  /// an [ExcludeSemantics] widget and then another [Semantics] widget.
  final bool excludeSemantics;

+  /// Whether to block user interactions for the rendering subtree.
+  ///
+  /// Setting this to true will prevent user from interacting with this


user -> users

goderbauer · 2023-02-25T00:36:16Z

packages/flutter/lib/src/widgets/basic.dart

+  /// rendering object and its subtree through pointer-related
+  /// [SemanticsAction]s in assistive technologies.
+  ///
+  /// This [SemanticsNode] created from this rendering object is still focusable


This [SemanticsNode] -> The [SemanticsNode]

goderbauer · 2023-02-25T00:36:33Z

packages/flutter/lib/src/widgets/basic.dart

+  /// rendering object and its subtree through pointer-related
+  /// [SemanticsAction]s in assistive technologies.
+  ///
+  /// This [SemanticsNode] created from this rendering object is still focusable


Also, this doc is on a widget, not rendering object.

Also below, where rendering object is mentioned again.

goderbauer · 2023-02-25T00:37:47Z

packages/flutter/lib/src/widgets/basic.dart

+  /// are blocked. [SemanticsAction]s in the parent semantics node originated
+  /// from other sources are still allowed.


what does "originated from other sources" mean?

goderbauer · 2023-02-25T00:43:10Z

packages/flutter/lib/src/widgets/basic.dart

+  /// by assistive technologies. Only pointer-related [SemanticsAction]s, such as
+  /// [SemanticsAction.tap] or its friends, are blocked.
+  ///
+  /// If this rendering object is merged into a parent semantics node, only the


I am actually not 100% clear on how merging works here. If I have a parent SemanticsConfig with a tap handler and there are two child semantics configs: One of them has a longpress handler and the other one sets blockUserInteractions. These will all get implicitly merged into a single node, right? What actions are now allowed on that node?

Also, if I force merge (using MergeSemantics) a SemanticsNode with blockUserInteractions set into another node with an action handler, will the resulting node have its action blocked or not?

If I have a parent SemanticsConfig with a tap handler and there are two child semantics configs: One of them has a longpress handler and the other one sets blockUserInteractions. These will all get implicitly merged into a single node, right? What actions are now allowed on that node?

Only the action in blockUserInteractions child will be block and drop from the merging. So the onTap on parent and the onLongPress on the child will still be merged into the resulting node.

if I force merge (using MergeSemantics) a SemanticsNode with blockUserInteractions set into another node with an action handler, will the resulting node have its action blocked or not?

The resulting node will not have its actions blocked, the actions in the node with blockUserInteractions will be dropped.

Okay, both behave the same here, that's good. Thanks for the clarification!

(I can't remember, do you have tests in place for both of these scenarios to make sure that this doesn't break?)

yes, the tests in flutter/packages/flutter/test/widgets/semantics_test.dart

goderbauer · 2023-03-02T17:32:58Z

Looks like some tests are still failing?

chunhtai · 2023-03-02T17:43:56Z

ah that is the semantics handle dispose issue, will fix

goderbauer

I believe the only thing left to figure out on this PR are the custom actions.

goderbauer · 2023-03-02T17:34:36Z

packages/flutter/test/semantics/semantics_test.dart

+            '   STALE\n'
+            '   owner: null\n'
+            '   Rect.fromLTRB(60.0, 20.0, 80.0, 50.0)\n'
+            '   actions: didGainAccessibilityFocus, longPress🚫️, scrollUp🚫️,\n'
+            '     showOnScreen🚫️\n'


unindent these lines to match line 648

goderbauer · 2023-03-02T17:39:46Z

packages/flutter/lib/src/widgets/basic.dart

@@ -6706,6 +6735,22 @@ class IgnorePointer extends SingleChildRenderObjectWidget {
 /// ** See code in examples/api/lib/widgets/basic/absorb_pointer.0.dart **
 /// {@end-tool}
 ///
+/// ## Semantics
+/// Using this widget may also affect how semantics subtree underneath this


Suggested change

/// Using this widget may also affect how semantics subtree underneath this

/// Using this widget may also affect how the semantics subtree underneath this

goderbauer · 2023-03-02T17:39:58Z

packages/flutter/lib/src/widgets/basic.dart

@@ -6706,6 +6735,22 @@ class IgnorePointer extends SingleChildRenderObjectWidget {
 /// ** See code in examples/api/lib/widgets/basic/absorb_pointer.0.dart **
 /// {@end-tool}
 ///
+/// ## Semantics


nit: blank line after this heading.

goderbauer · 2023-03-02T17:41:03Z

packages/flutter/lib/src/widgets/basic.dart

+  /// rendering object and its subtree through pointer-related
+  /// [SemanticsAction]s in assistive technologies.
+  ///
+  /// This [SemanticsNode] created from this widget is still focusable


Suggested change

/// This [SemanticsNode] created from this widget is still focusable

/// The [SemanticsNode] created from this widget is still focusable

goderbauer · 2023-03-02T17:41:44Z

packages/flutter/lib/src/widgets/basic.dart

+  /// ```
+  ///
+  /// The result semantics node will still have `_myTap`, but the `_myLongPress`
+  /// will be blocked.


Thanks for adding the example!

goderbauer · 2023-03-02T17:44:37Z

packages/flutter/lib/src/semantics/semantics.dart

+  /// [SemanticsAction.tap] or its friends, are blocked.
+  ///
+  /// If this rendering object is merged into a parent semantics node, only the
+  /// [SemanticsAction]s from this rendering object and the rendering objects in


goderbauer · 2023-03-02T17:45:11Z

packages/flutter/lib/src/semantics/semantics.dart

@@ -1800,6 +1804,22 @@ class SemanticsNode extends AbstractNode with DiagnosticableTreeMixin {
    _markDirty();
  }

+  /// Whether this user can interact with this node in assistive technologies.


Suggested change

/// Whether this user can interact with this node in assistive technologies.

/// Whether the user can interact with this node via assistive technologies.

goderbauer · 2023-03-02T17:45:36Z

packages/flutter/lib/src/semantics/semantics.dart

@@ -1800,6 +1804,22 @@ class SemanticsNode extends AbstractNode with DiagnosticableTreeMixin {
    _markDirty();
  }

+  /// Whether this user can interact with this node in assistive technologies.
+  ///
+  /// This node can still be receive accessibility focus even if this is true.


Suggested change

/// This node can still be receive accessibility focus even if this is true.

/// This node can still receive accessibility focus even if this is true.

goderbauer · 2023-03-02T17:45:52Z

packages/flutter/lib/src/semantics/semantics.dart

+  /// Whether this user can interact with this node in assistive technologies.
+  ///
+  /// This node can still be receive accessibility focus even if this is true.
+  /// Setting this true prevents user from activating pointer related


Suggested change

/// Setting this true prevents user from activating pointer related

/// Setting this true prevents the user from activating pointer related

goderbauer · 2023-03-02T17:48:16Z

packages/flutter/lib/src/semantics/semantics.dart

@@ -68,6 +68,11 @@ typedef SemanticsUpdateCallback = void Function(ui.SemanticsUpdate update);
 /// value.
 typedef ChildSemanticsConfigurationsDelegate = ChildSemanticsConfigurationsResult Function(List<SemanticsConfiguration>);

+final int _kSystemInteractions = SemanticsAction.didGainAccessibilityFocus.index
+  | SemanticsAction.didLoseAccessibilityFocus.index
+  | SemanticsAction.customAction.index;


This is the only remaining open question on the PR. For me, if I have an interface where I can delete a ListItem by swiping right/left on it and I expose this action as a custom action for a11y users, I would expect it gets blocked when I use an IgnorePointer. Everything else would be surprising. I can't think of a use case where I would still want custom actions to be allowed. Do you have one? If so, we may need to make this configurable if it can go both ways.

chunhtai · 2023-03-14T21:58:58Z

some more updates, still working on resolving internal test failures

chunhtai · 2023-03-14T23:34:53Z

some of the test failures are due to flutter/engine#40146, waiting for it to merge

… in a11y

auto-submit · 2023-04-06T20:38:08Z

auto label is removed for flutter/flutter, pr: 120619, due to - The status or check suite Mac framework_tests_libraries has failed. Please fix the issues identified (or deflake) before re-applying this label.

…teractions… (flutter/flutter#120619)

flutter#120619) Fixes IgnorePointer and AbsorbPointer to only block user interactionsâ�¦

Hixie · 2023-07-25T06:28:36Z

I just ran into this deprecation in the documentation for SliverIgnorePointer, and it was quite confusing, because there's no context as to why I would have wanted to use this feature, why it's deprecated, or what it means to "Create a custom sliver ignore pointer widget instead" or why that would be better than having this feature. Since three quarters of the widget's documentation appears to be about this feature, it feels like the feature was quite important, and so its removal is surprising to the reader.

The migration guide doesn't really illuminate the reader, either. It doesn't provide any background regarding what the feature was, why it was normally used, or why you might no longer need it, other than "this workaround is no longer needed". However, if the workaround is no longer needed, one would assume the migration is just to remove the use of the property, whereas the migration guide says "consider using ExcludeSemantics instead" and "Consider creating your own custom widget", but there's no explanation as to why one would do either of these things.

It would be great if we could provide significantly more context in the dartdocs and the migration guide for this removal.

cc @chunhtai @goderbauer @sfshaza2

chunhtai · 2023-07-25T16:17:23Z

Hi @Hixie

there's no context as to why I would have wanted to use this feature

I can't really think of a use case for ignoringSemantics, either for it to be true or false, if the ignorePointer/AbsorbPointer and its sliver counter parts handle of the semantics correctly.

To me, the ignoringSemantics was here to work around that the ignorepointer couldn't handle the semantics before.

Therefore, I didn't add too much context on why you would want to use this property but merely provide migration guide if developers did use it.

This migration guide https://docs.flutter.dev/release/breaking-changes/ignoringsemantics-migration provides only the code samples for IgnorePointer. I can add more samples for AbsorbPointer, SliverIgnorePointer. Will they be enough to address your concerns?

Hixie · 2023-07-25T17:29:13Z

Well presumably we added this feature for a reason. :-)
That's what I would try to include in the explanations.

goderbauer · 2023-07-25T17:31:21Z

I think we had that "feature" because we didn't know how to do the right thing, which is only selectively blocking semantics interactions and not all semantics. There was no real use case for it, it was just a stop-gap solution.

Hixie · 2023-07-25T17:35:43Z

Stop-gap solution for what problem? That's what we should describe. That problem. And the better solution. :-)

chunhtai · 2023-07-25T19:03:06Z

yes I can definitely explain this in the doc. I will create a pr.

migration guide update flutter/website#9124 fixes the concerns in #120619

chunhtai commented Feb 13, 2023

View reviewed changes

chunhtai requested review from goderbauer and Hixie February 13, 2023 20:06

goderbauer reviewed Feb 15, 2023

View reviewed changes

chunhtai force-pushed the issues/59402 branch 3 times, most recently from 691fd61 to 57978c2 Compare February 23, 2023 20:29

chunhtai requested a review from goderbauer February 23, 2023 22:06

chunhtai force-pushed the issues/59402 branch from 57978c2 to 8a3ac83 Compare February 23, 2023 22:12

goderbauer reviewed Feb 25, 2023

View reviewed changes

packages/flutter/lib/src/rendering/object.dart Outdated Show resolved Hide resolved

goderbauer reviewed Feb 25, 2023

View reviewed changes

packages/flutter/lib/src/rendering/object.dart Outdated Show resolved Hide resolved

goderbauer reviewed Feb 25, 2023

View reviewed changes

chunhtai force-pushed the issues/59402 branch from f7ea841 to 2b354df Compare February 27, 2023 18:00

chunhtai requested a review from goderbauer February 27, 2023 18:02

chunhtai mentioned this pull request Feb 27, 2023

Adds ignoringSemantics migration guide flutter/website#8312

Merged

3 tasks

flutter-dashboard bot added the f: cupertino flutter/packages/flutter/cupertino repository label Feb 27, 2023

chunhtai force-pushed the issues/59402 branch from f761832 to ee0b94e Compare March 1, 2023 17:48

goderbauer reviewed Mar 2, 2023

View reviewed changes

chunhtai force-pushed the issues/59402 branch from ee0b94e to bc68659 Compare March 2, 2023 18:07

chunhtai requested a review from goderbauer March 2, 2023 22:49

chunhtai force-pushed the issues/59402 branch from 2407107 to f5a933a Compare March 13, 2023 16:59

chunhtai force-pushed the issues/59402 branch from aa8a1a4 to 9f64187 Compare March 14, 2023 23:41

chunhtai force-pushed the issues/59402 branch 2 times, most recently from 1347bf8 to 3f1028a Compare March 28, 2023 17:45

chunhtai added 3 commits April 6, 2023 12:26

Fixes IgnorePointer and AbsorbPointer to only block user interactions…

e08e349

… in a11y

Fix logic

6210886

fix test

3e645c2

chunhtai force-pushed the issues/59402 branch from eec01b6 to 3e645c2 Compare April 6, 2023 19:27

chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 6, 2023

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Apr 6, 2023

chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 7, 2023

auto-submit bot merged commit 9884b5f into flutter:master Apr 7, 2023
72 checks passed

HansMuller mentioned this pull request Apr 7, 2023

Flutter Framework Team: Notable Commits #121415

Open

engine-flutter-autoroll mentioned this pull request Apr 8, 2023

Roll Flutter from aa6a0a9dea63 to 3ea2d72446c6 (18 revisions) flutter/packages#3666

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 8, 2023

9884b5f55 Fixes IgnorePointer and AbsorbPointer to only block user in…

e4cbb25

…teractions… (flutter/flutter#120619)

exaby73 pushed a commit to NevercodeHQ/flutter that referenced this pull request Apr 17, 2023

Fixes IgnorePointer and AbsorbPointer to only block user interactions… (

2aaf88c

flutter#120619) Fixes IgnorePointer and AbsorbPointer to only block user interactionsâ�¦

chunhtai mentioned this pull request Jul 25, 2023

Adds more documentations around ignoreSemantics deprecations. #131287

Merged

8 tasks

auto-submit bot pushed a commit that referenced this pull request Aug 7, 2023

Adds more documentations around ignoreSemantics deprecations. (#131287)

b01cb30

migration guide update flutter/website#9124 fixes the concerns in #120619

		/// are blocked. [SemanticsAction]s in the parent semantics node originated
		/// from other sources are still allowed.

	/// Using this widget may also affect how semantics subtree underneath this
	/// Using this widget may also affect how the semantics subtree underneath this

	/// This [SemanticsNode] created from this widget is still focusable
	/// The [SemanticsNode] created from this widget is still focusable

	/// Whether this user can interact with this node in assistive technologies.
	/// Whether the user can interact with this node via assistive technologies.

	/// This node can still be receive accessibility focus even if this is true.
	/// This node can still receive accessibility focus even if this is true.

	/// Setting this true prevents user from activating pointer related
	/// Setting this true prevents the user from activating pointer related

Fixes IgnorePointer and AbsorbPointer to only block user interactions… #120619

Fixes IgnorePointer and AbsorbPointer to only block user interactions… #120619

Conversation

chunhtai commented Feb 13, 2023 • edited

Pre-launch Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goderbauer commented Feb 13, 2023

chunhtai commented Feb 14, 2023

goderbauer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chunhtai Feb 27, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chunhtai Feb 27, 2023 • edited

Choose a reason for hiding this comment

goderbauer commented Mar 2, 2023

chunhtai commented Mar 2, 2023

goderbauer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chunhtai commented Mar 14, 2023

chunhtai commented Mar 14, 2023

auto-submit bot commented Apr 6, 2023

Hixie commented Jul 25, 2023

chunhtai commented Jul 25, 2023

Hixie commented Jul 25, 2023

goderbauer commented Jul 25, 2023

Hixie commented Jul 25, 2023

chunhtai commented Jul 25, 2023

chunhtai commented Feb 13, 2023 •

edited

chunhtai Feb 27, 2023 •

edited

chunhtai Feb 27, 2023 •

edited