Why the total length? do you know why this is called?

The behavior is indifferent whether you return (0, 0) or (0, [self accessibilityNumberOfCharacters]). The goal of this PR is to throw out BASE_UNREACHABLE() so macos apps don't crash.

according to the doc, it should return the range of glyph
https://developer.apple.com/documentation/appkit/nsaccessibility/1531615-accessibilityrangeforposition
Let's at least try to do the right thing even if it is indifferent

Oh interesting; if that's the case then let's see if we can hit-test this for the character in question.

@cbracken is hit-testing a specific character possible? The AXTree does not have access to the RenderObjectTree; hence, we cannot translate the NSPoint(x, y) to any particular position in Text/Strings rendered by flutter. If that's true, then we will be unable to implement this function in any meaningful way. For now, taking out BASE_UNREACHABLE will be the appropriate fix.

A nice improvement to this test would be to actually verify that a proper range is returned.

On the node above you could add:

std::string value = "Hello";
root.AddStringAttribute(ax::mojom::StringAttribute::kValue, value);

Then down here you could do:

NSRange range = [native_root accessibilityRangeForPosition:(NSPoint)point];
EXPECT_EQ(range.location, 0);
EXPECT_EQ(range.length, value.length());

This is a good idea. Although, the test fails with

std::string value = "ä";

I believe this is because ä is one glyph that is composed of two UTF-16 code units.

It can be written as a single codepoint or as a combining character sequence.

[NSString rangeOfComposedCharacterSequence] can get you the range for a character but we'd still need a way to get at the index corresponding to a glyph.

aside: to be totally unambiguous/clear to the reader, you could write this using a unicode escape: "\u00e4" for the single codepoint version or "\u0061\u0308" for the decomposed version. In that case, probably worth adding a comment // ä for reference. That also reduces the chance that someone with a weird character encoding set in their editor ever breaks this test.

@cbracken Following up on this comment we might want to leave the test alone, such that it only tests the crash. Right now, it might not be possible for accessibilityRangeForPosition to return anything meaningful.

It's not from within this part of the code.

@chunhtai if we delegated this to a method in FlutterPlatformNodeDelegate do you know if there's a reasonable way at getting at glyph-level text metrics from the semantics tree? Off the top of my head I'm not aware of anything.

@a-wallen I can't remember, what happens if we return NSMakeRange(0, 0) or NSMakeRange(NSNotFound, 0)?

Not that I am aware. If we have to do this, framework will have to send text metrics to the AXTree. To do that, Skia SKParagraph needs to have additional API to expose these metrics so that framework can get it. I don't think this is worth it unless this method is used in a meaningful way by the macos

We do need an implementation since macOS calls the current one and it crashes, but @a-wallen did some poking around and returning (0,0) (similar to the failure case of the existing methods) triggers a fallback where macOS calls accessibilityRangeForLine:, which then returns the full string, which fixes the app crash and gives us the correct behaviour. That seems like the best approach (with a comment on the 0,0 return explaining the difficulty in looking up text metrics).

Looking at the similar code for locating a text range from a point for iOS, it looks like it's still the // TODO(cbracken): implement I wrote 5 years ago, almost certainly because of the difficulty in hit-testing with text metrics.

@cbracken and @chunhtai, I'd like to merge this PR with 0aa2f5f. Editing accessibilityRangeForPosition to return NSMakeRange(0,0) causes a fallback on accessibilityRangeForLine produces the appropriate behavior for Flutter for the time being.

This sounds good to me for now. BTW, @cbracken If iOS needs it, we may consider implement it for real.