Bug 174856 – AX: Incorrect range from index and length in contenteditable with <p> tags

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

[Nan Wang]

For something like this:
<div contenteditable="true">
<p>ab</p>
<p>cd</p>
</div>

On Mac when we get AXValueAttribute value it would be:
"ab\n\ncd"

So getting the string from range {2, 4} should be:
"\n\ncd"

But instead we get:
"\ncd"

<rdar://problem/30187854>

[Nan Wang]

Created attachment 316437 [details]
patch

[chris fleizach]

Comment on attachment 316437 [details]
patch

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

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3956
> +            return m_object->doAXStringForRange(plainTextRange);

is this not possible to fix in the characterOffsetForIndex? do you know the reason?

[Nan Wang]

Comment on attachment 316437 [details]
patch

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

>> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3956
>> +            return m_object->doAXStringForRange(plainTextRange);
> 
> is this not possible to fix in the characterOffsetForIndex? do you know the reason?

I think the problem here is that for text controls, the AXValue comes from the AccessibilityNodeObject::text() (we call downcast<HTMLTextFormControlElement>(*node).value() or downcast<Element>(node)->innerText()).
This value can be different from what textIterator gives us. In this case it will contain two newline characters instead of one visually.
So when we try to get the range of text we should match that with the entire element value.

[chris fleizach]

Comment on attachment 316437 [details]
patch

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

>>> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3956
>>> +            return m_object->doAXStringForRange(plainTextRange);
>> 
>> is this not possible to fix in the characterOffsetForIndex? do you know the reason?
> 
> I think the problem here is that for text controls, the AXValue comes from the AccessibilityNodeObject::text() (we call downcast<HTMLTextFormControlElement>(*node).value() or downcast<Element>(node)->innerText()).
> This value can be different from what textIterator gives us. In this case it will contain two newline characters instead of one visually.
> So when we try to get the range of text we should match that with the entire element value.

does this need to be nativeTextControl() then? I think textControl also pulls in role=textbox

[Nan Wang]

Comment on attachment 316437 [details]
patch

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

>>>> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3956
>>>> +            return m_object->doAXStringForRange(plainTextRange);
>>> 
>>> is this not possible to fix in the characterOffsetForIndex? do you know the reason?
>> 
>> I think the problem here is that for text controls, the AXValue comes from the AccessibilityNodeObject::text() (we call downcast<HTMLTextFormControlElement>(*node).value() or downcast<Element>(node)->innerText()).
>> This value can be different from what textIterator gives us. In this case it will contain two newline characters instead of one visually.
>> So when we try to get the range of text we should match that with the entire element value.
> 
> does this need to be nativeTextControl() then? I think textControl also pulls in role=textbox

I think isTextControl() is what we want, otherwise the problematic contenteditable wouldn't be covered. nativeTextControl() only returns true for form controls and <textarea>

[chris fleizach]

Comment on attachment 316437 [details]
patch

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

>>>>> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3956
>>>>> +            return m_object->doAXStringForRange(plainTextRange);
>>>> 
>>>> is this not possible to fix in the characterOffsetForIndex? do you know the reason?
>>> 
>>> I think the problem here is that for text controls, the AXValue comes from the AccessibilityNodeObject::text() (we call downcast<HTMLTextFormControlElement>(*node).value() or downcast<Element>(node)->innerText()).
>>> This value can be different from what textIterator gives us. In this case it will contain two newline characters instead of one visually.
>>> So when we try to get the range of text we should match that with the entire element value.
>> 
>> does this need to be nativeTextControl() then? I think textControl also pulls in role=textbox
> 
> I think isTextControl() is what we want, otherwise the problematic contenteditable wouldn't be covered. nativeTextControl() only returns true for form controls and <textarea>

ok so contenteitable also ends up using innerText() instead of a text iterator?

[Nan Wang]

Comment on attachment 316437 [details]
patch

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

>>>>>> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3956
>>>>>> +            return m_object->doAXStringForRange(plainTextRange);
>>>>> 
>>>>> is this not possible to fix in the characterOffsetForIndex? do you know the reason?
>>>> 
>>>> I think the problem here is that for text controls, the AXValue comes from the AccessibilityNodeObject::text() (we call downcast<HTMLTextFormControlElement>(*node).value() or downcast<Element>(node)->innerText()).
>>>> This value can be different from what textIterator gives us. In this case it will contain two newline characters instead of one visually.
>>>> So when we try to get the range of text we should match that with the entire element value.
>>> 
>>> does this need to be nativeTextControl() then? I think textControl also pulls in role=textbox
>> 
>> I think isTextControl() is what we want, otherwise the problematic contenteditable wouldn't be covered. nativeTextControl() only returns true for form controls and <textarea>
> 
> ok so contenteitable also ends up using innerText() instead of a text iterator?

Yes, for NSRange calls. The text marker stuff will still be using textIterator.

[WebKit Commit Bot]

Comment on attachment 316437 [details]
patch

Clearing flags on attachment: 316437

Committed r219949: <http://trac.webkit.org/changeset/219949>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[James Craig]

*** Bug 168957 has been marked as a duplicate of this bug. ***