Bug 229713 – Braille display is blank in contenteditable elements when the field is followed by another element.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Andres Gonzalez]

Braille display is blank in contenteditable elements when the field is followed by another element.

[Andres Gonzalez]

rdar://82095237

[Andres Gonzalez]

To reproduce:
1. Safari with VoiceOver and Braille output.
2. open a page containing the following HTML snippet:

<div id="textfield" contenteditable="true">this is a test.</div>
<button>Submit</button>

Focus the text field and observe Braille output.

[Andres Gonzalez]

Created attachment 436887 [details]
Patch

[Andres Gonzalez]

Created attachment 436944 [details]
Patch

[chris fleizach]

Comment on attachment 436944 [details]
Patch

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

> Source/WebCore/ChangeLog:15
> +        ranges in text fields to match the number of charaters in the line.

if we're changing behavior, then I think we need a new test that will capture that

[Andres Gonzalez]

Created attachment 437105 [details]
Patch

[Andres Gonzalez]

(In reply to chris fleizach from comment #5)
> Comment on attachment 436944 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=436944&action=review
> 
> > Source/WebCore/ChangeLog:15
> > +        ranges in text fields to match the number of charaters in the line.
> 
> if we're changing behavior, then I think we need a new test that will
> capture that

Added test cases for single and multiline TextArea and content editable, and single line input text field.

[chris fleizach]

Comment on attachment 437105 [details]
Patch

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

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:2468
> +    return it.text().length() == 1 && it.text()[0] == '\n';

is it worth caching it.text(). it must cost something to gather the text for a range

[Andres Gonzalez]

(In reply to chris fleizach from comment #8)
> Comment on attachment 437105 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=437105&action=review
> 
> > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:2468
> > +    return it.text().length() == 1 && it.text()[0] == '\n';
> 
> is it worth caching it.text(). it must cost something to gather the text for
> a range

It is an inline accessor to m_text:

    StringView text() const { ASSERT(!atEnd()); return m_text; }

[EWS]

Committed r281920 (241229@main): <https://commits.webkit.org/241229@main>

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

[Andres Gonzalez]

rdar://82095237

[Darin Adler]

Comment on attachment 437105 [details]
Patch

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

>>> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:2468
>>> +    return it.text().length() == 1 && it.text()[0] == '\n';
>> 
>> is it worth caching it.text(). it must cost something to gather the text for a range
> 
> It is an inline accessor to m_text:
> 
>     StringView text() const { ASSERT(!atEnd()); return m_text; }

Pushing in the opposite direction: I’m pretty sure you can just write:

    return it.text() == "\n";

And the code will be nearly as efficient; don’t have to write out the length/character check.