[Webkit-unassigned] [Bug 206093] AX: Unable to use AccessibilityObject::replaceTextInRange to insert text at first time when the text fields are empty

[bugzilla-daemon at webkit.org]

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

--- Comment #5 from Canhai Chen <canhai_chen at apple.com> ---
Comment on attachment 387722
  --> https://bugs.webkit.org/attachment.cgi?id=387722
Patch

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

>> Source/WebCore/accessibility/AccessibilityObject.cpp:1192
>> +    if (!range.start && !range.length && !textLength)
> 
> I think I agree that this is an invalid range, but should be also be checking for an invalid range passed into replaceTextInRange?
> 
> also is the client that's calling into here doing any value checking before they send the range in?

Yes, I think another possible fix here is to check the range first in `accessibilityReplaceRange:withText:`, if the range is not (0, 0), we continue to call `AccessibilityObject::replaceTextInRange`, otherwise we call `AccessibilityObject::insertText`. But I think `AccessibilityObject::replaceTextInRange` itself should be able to handle (0, 0) case.

The client is calling here with `accessibilityReplaceRange:withText:` interface, so it does not have knowledge about whether the element is from WebKit or AppKit (although we have ways to try to determine it). And in AppKit, the `accessibilityReplaceRange:withText:` works fine even when the range is (0, 0).

I think the range (0, 0) is "valid" for `accessibilityReplaceRange:withText:` but "invalid" for a `VisibleSelection`. The reason it caused the text replacement failed is because the `WebCore::Range` it creates here will make the `setSelectedRange` create a new `VisibleSelection` whose container node will be used to determine whether the element is editable or not. Most of the cases in text replacement, the container node is an editable `WebCore::Text` element. But the new `VisibleSelection` it creates for (0, 0) here will return the parent node as the container node (could be a `HTMLDivElement` or `HTMLBodyElement`), which makes it uneditable.

Return nullptr if the range is (0, 0) and the text length is 0 here can avoid this, so that when the frame selection is trying to `setSelectedRange` before replacing text, instead of creating an uneditable `VisibleSelection`, it will just return and later in `Editor::replaceSelectionWithText`, it will use the default VisibleSelection with range (0, 0), of which the container node is an editable `TextControlInnerTextElement`.

One workaround for the client right now is to send a "invalid" range like (X, 0) when the range is (0, 0), so that it will fall into the invalid case here and use the correct `VisibleSelection` with `TextControlInnerTextElement` to do the text replacement.

>> Source/WebCore/accessibility/AccessibilityObject.cpp:1193
>> +        return nullptr;
> 
> I think this can be written as if (range.isNull() && !textLength)

Thanks for the note, will update in a new patch!

-- 
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/20200116/ae8f798a/attachment.htm>