Bug 255802

Summary: AX: Retire accessibility PlainTextRange and use CharacterRange instead.
Product: WebKit Reporter: Andres Gonzalez <andresg_22>
Component: AccessibilityAssignee: Andres Gonzalez <andresg_22>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, andresg_22, apinheiro, cfleizach, cgarcia, darin, dmazzoni, ews-watchlist, jcraig, jdiggs, samuel_white, tyler_w, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Andres Gonzalez 2023-04-21 12:29:50 PDT 
PlainTextRange duplicates CharacterRange functionality and it is missing some like operator NSRange.
Comment 1 Radar WebKit Bug Importer 2023-04-21 12:30:05 PDT 
<rdar://problem/108380311>
Comment 2 Andres Gonzalez 2023-04-21 15:14:18 PDT 
Created attachment 466034 [details]
Patch
Comment 3 Tyler Wilcock 2023-04-21 15:42:07 PDT 
Comment on attachment 466034 [details]
Patch

r+ after EWS passes
Comment 4 Andres Gonzalez 2023-04-24 09:24:54 PDT 
Created attachment 466064 [details]
Patch
Comment 5 Andres Gonzalez 2023-04-25 08:23:16 PDT 
Created attachment 466074 [details]
Patch
Comment 6 Andres Gonzalez 2023-04-25 10:35:18 PDT 
Created attachment 466077 [details]
Patch
Comment 7 Andres Gonzalez 2023-04-26 13:39:33 PDT 
Created attachment 466110 [details]
Patch
Comment 8 Andres Gonzalez 2023-05-04 07:22:37 PDT 
Created attachment 466206 [details]
Patch
Comment 9 EWS 2023-05-04 09:11:21 PDT 
commit-queue failed to commit attachment 466206 [details] to WebKit repository. To retry, please set cq+ flag again.
Comment 10 Carlos Garcia Campos 2023-05-08 01:54:16 PDT 
Comment on attachment 466206 [details]
Patch

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

> Source/WebCore/accessibility/AccessibilityObject.cpp:1522
> -    unsigned textLength = getLengthForTextRange();
> -    if (range.start + range.length > textLength)
> +    if (range.location + range.length > text().length())

There's a change in behavior here. ATSPI implements getLengthForTextRange() in a different way, I don't know why, but it's required here.

> Source/WebCore/accessibility/AccessibilityObject.cpp:1533
> -    unsigned textLength = getLengthForTextRange();
> -    if (range.start + range.length > textLength)
> +    unsigned textLength = text().length();
> +    if (range.location + range.length > textLength)

And here.
Comment 11 Andres Gonzalez 2023-05-23 11:59:45 PDT 
Created attachment 466465 [details]
Patch
Comment 12 Darin Adler 2023-05-23 15:24:48 PDT 
Generally it should be OK to use CharacterRange as a parameter rather than const CharacterRange& and should be more efficient rather than less efficient. Worth considering when touching all these call sites.
Comment 13 EWS 2023-05-23 19:55:29 PDT 
Committed 264452@main (13f559711a81): <https://commits.webkit.org/264452@main>

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