Bug 257867 – Support custom annotations for Accessibility in composition contexts.

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[Jack Wiig]

This change broadens the information that we can parse from an attributed string in a composition context. Now, annotations (attributes that have no meaningful value, usually @YES or "1") can be conveyed. We use this in accessibility to convey when an inline prediction is happening, so assistive technologies know not to process that text as part of the user's typing.

[Radar WebKit Bug Importer]

<rdar://problem/110488738>

[Jack Wiig]

Created attachment 466642 [details]
Patch

[Jack Wiig]

Created attachment 466644 [details]
Patch

[Jack Wiig]

Created attachment 466652 [details]
Patch

[Jack Wiig]

Created attachment 466653 [details]
Patch

[Jack Wiig]

Created attachment 466654 [details]
Patch

[Tyler Wilcock]

Comment on attachment 466654 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=466654&action=review

> Source/WebKit/UIProcess/mac/WebViewImpl.mm:5111
> +            auto it = annotations.find(key);
> +            if (it == annotations.end())
> +                it = annotations.add(key, Vector<CharacterRange> { }).iterator;

I think HashMap::ensure does this (find a value, initializing it if necessary). Might express the intent a bit better (if it works here).

> Source/WebKit/UIProcess/mac/WebViewImpl.mm:5150
>              underlines = compositionUnderlines(string);
> +        annotations = compositionAnnotations(string);

The indentation is different between these two lines. Not sure which is right, though.

> Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:669
> +            auto it = annotations.find(name);
> +            if (it == annotations.end())
> +                it = annotations.add(name, Vector<WebCore::CharacterRange> { }).iterator;

Another place where HashMap::ensure might work?

[Jack Wiig]

I didn't know the function of HashMap::ensure, but now that I do, I'll switch to using that. Thanks!

As for the indentation difference, the first line is the single-line else clause if HAVE(REDESIGNED_TEXT_CURSOR) is true, otherwise, it's just part of the same scope as the annotations line I added. I think the thought was it's safer to leave it indented to convey that it's part of an else block, but that was not code I wrote.

[Tyler Wilcock]

> As for the indentation difference, the first line is the single-line else
> clause if HAVE(REDESIGNED_TEXT_CURSOR) is true, otherwise, it's just part of
> the same scope as the annotations line I added. I think the thought was it's
> safer to leave it indented to convey that it's part of an else block, but
> that was not code I wrote.
Got it, sounds good, thanks!

[EWS]

Committed 265055@main (ef93c1b412c0): <https://commits.webkit.org/265055@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 466654 [details].

[EWS]

Test gardening commit 279418@main (190ac0f18d5d): <https://commits.webkit.org/279418@main>

Reviewed commits have been landed. Closing PR #29202 and removing active labels.