Developers should be discouraged from having unlabeled leaf a11y nodes

[dnfield]

For example, the following should trigger a test error:

await tester.pumpWidget(
  MaterialApp(
    home: Scaffold(
      body: Column(
        mainAxisAlignment: MainAxisAlignment.center,
        children: [
          const Text(
            'You have pushed the button this many times:',
          ),
          Text(
            '5',
            style: Theme.of(context).textTheme.headline4,
          ),
          Image.network('https://via.placeholder.com/150'),
        ],
       ),
    ),
  ),
);

The error should tell the developer that they should either provide a semantic label for the image, or exclude it from semantics. Otherwise screen readers may focus on the image and not have an appropriate thing to read out, which is frustrating for a11y users.

I think we should do this in testWidgets, with a walk of the a11y tree that checks for unlabeled/hinted/valued leaf nodes. It could be done as an a11y matcher, but ideally this is something tests would get opted into to catch errors that developers tend to miss in this regard.

/cc @goderbauer @chunhtai

[chunhtai]

Maybe we should print out warning instead? otherwise, it will be a massive breaking change.

We have different guideline like textContrastGuideline, labeledTapTargetGuideline... maybe we should create one for the labeled leaf node.

The leaf node might not always be focusable and we don't really know which node is focusable on the framework side, it is up to the platform to decide which node is focusable. The current implementation on iOS and Android are the same, but I am not sure other platforms such as fuchsia and Linux

- (BOOL)isAccessibilityElement {
  if (![self isAccessibilityBridgeAlive]) {
    return false;
  }

  // Note: hit detection will only apply to elements that report
  // -isAccessibilityElement of YES. The framework will continue scanning the
  // entire element tree looking for such a hit.

  //  We enforce in the framework that no other useful semantics are merged with these nodes.
  if ([self node].HasFlag(flutter::SemanticsFlags::kScopesRoute)) {
    return false;
  }

  // If the node is scrollable AND hidden OR
  // The node has a label, value, or hint OR
  // The node has non-scrolling related actions.
  //
  // The kIsHidden flag set with the scrollable flag means this node is now
  // hidden but still is a valid target for a11y focus in the tree, e.g. a list
  // item that is currently off screen but the a11y navigation needs to know
  // about.
  return (([self node].flags & flutter::kScrollableSemanticsFlags) != 0 &&
          ([self node].flags & static_cast<int32_t>(flutter::SemanticsFlags::kIsHidden)) != 0) ||
         ![self node].label.empty() || ![self node].value.empty() || ![self node].hint.empty() ||
         ([self node].actions & ~flutter::kScrollableSemanticsActions) != 0;
}

[dnfield]

It always creates extra work though, right? I think starting with a matcher is fine but I'm not sure who will use it. Having a flag to opt in to might be good.

It would definitely be breaking for customers. I don't think developers typically think about this.

[goderbauer]

Maybe we should print out warning instead?

Warnings get spammy pretty quickly and then just ignored if there are too many. To avoid the breaking change, this could be opt-in (or opt-out).

[chunhtai]

Sounds good, but then the problem still remain. We don't know which node will be focusable on the framework side, and not every leaf node is focusable. We can duplicate the logic from the android/ios accessibility bridge, but it may be hard to maintain

[dnfield]

Why do we need to know whether it's focusable?

If it is focusable it needs a label for sure. If it's not it just shouldn't be there at all.

We already have a guideline matcher for labeling tap targets.

[chunhtai]

We have a lot of intermediate nodes in the tree, some of them are used for dpi scaling or rect transform, some of them are there for route transition. labeling tap targets is more straightforward. if something is tappable, it must be focusable thus needs a label.

BTW, the Image.network('https://via.placeholder.com/150') should not be focusable in ios or android if it does not have a label. Are you seeing this behavior in the app?

[dnfield]

No, this came up because of an internal test that was using semantics debugger