[Webkit-unassigned] [Bug 261056] AX: Expose accessibility attributes for inline text predictions

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=261056

--- Comment #12 from Joshua Hoffman <jhoffman23 at apple.com> ---
(In reply to Andres Gonzalez from comment #11)
> (In reply to Joshua Hoffman from comment #10)
> > Created attachment 467570 [details]
> > Patch
> 
> --- a/Source/WebCore/accessibility/ios/AccessibilityObjectIOS.mm
> +++ b/Source/WebCore/accessibility/ios/AccessibilityObjectIOS.mm
> 
> +static void
> attributedStringSetCompositionAttributes(NSMutableAttributedString
> *attributedString, Node& node)
> 
> Does this need to be different from the same function in
> AccessibilityObjectMac.mm? Or can we have a common COCOA function?
We need to have these stay separate functions, since we expose different attributes for each platform. 

> +    if (&node != editor.compositionNode())
> +        return; 
> 
> Seems like this check should be earlier in the function, same as in the MAC
> version.
When accessing completed words, the previous composition will have ended, so the node we are looking at will not be equal to the compositionNode of the editor. We need them to be equal only when parsing presented (and incomplete) completions.

> --- a/Source/WebCore/editing/Editor.cpp
> +++ b/Source/WebCore/editing/Editor.cpp
> 
>              cache->onTextCompositionChange(*previousCompositionNode, state,
> true);
> +
> +            // WebKit needs to store inline-predicted words to expose their
> position and text values to assistive technologies.
> +            if (state == AXObjectCache::CompositionState::Ended &&
> !m_lastPresentedTextPrediction.text.isEmpty()) {
> +                String previousCompositionNodeText =
> previousCompositionNode->wholeText();
> +                size_t wordStart = 0;
> +                if (previousCompositionNodeText.length()) {
> +                    for (size_t position =
> previousCompositionNodeText.length() - 1; position > 0; position--) {
> +                        if
> (isASCIIWhitespace(previousCompositionNodeText[position])) {
> +                            wordStart = position + 1;
> +                            break;
> +                        }
> +                    }
> +                }
> +                previousCompositionNodeText =
> previousCompositionNodeText.substring(wordStart);
> +
> +                String completePredictedWord =
> makeString(previousCompositionNodeText, m_lastPresentedTextPrediction.text);
> +                m_lastPresentedTextPredictionComplete = {
> completePredictedWord, wordStart };
> +
> +                // Reset last presented prediction since a candidate was
> accepted.
> +                m_lastPresentedTextPrediction.reset();
> +            }
>          }
>          if (m_compositionNode) {
>              auto state = previousCompositionNode ?
> AXObjectCache::CompositionState::InProgress :
> AXObjectCache::CompositionState::Started;
>              cache->onTextCompositionChange(*m_compositionNode, state, true);
> +            m_lastPresentedTextPredictionComplete.reset();
> 
> Instead of keeping these variables in the Editor, I wondering if it would
> make more sense to pass them as context to
> AXObjectCache::onTextCompositionChange.

I have debated this as well. Ultimately, I believe maintaining this state here will make more sense so that the state remains up-to-date, and we don't have to worry about updating the cached properties. Happy to discuss further, however.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20230906/7686b89f/attachment.htm>