Bug 255802 - AX: Retire accessibility PlainTextRange and use CharacterRange instead.
Summary: AX: Retire accessibility PlainTextRange and use CharacterRange instead.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Andres Gonzalez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-04-21 12:29 PDT by Andres Gonzalez
Modified: 2023-05-23 19:55 PDT (History)
13 users (show)

See Also:

Attachments
Patch (52.20 KB, patch)
2023-04-21 15:14 PDT, Andres Gonzalez 		no flags Details | Formatted Diff | Diff
Patch (52.20 KB, patch)
2023-04-24 09:24 PDT, Andres Gonzalez 		no flags Details | Formatted Diff | Diff
Patch (51.83 KB, patch)
2023-04-25 08:23 PDT, Andres Gonzalez 		no flags Details | Formatted Diff | Diff
Patch (52.83 KB, patch)
2023-04-25 10:35 PDT, Andres Gonzalez 		no flags Details | Formatted Diff | Diff
Patch (52.83 KB, patch)
2023-04-26 13:39 PDT, Andres Gonzalez 		no flags Details | Formatted Diff | Diff
Patch (52.77 KB, patch)
2023-05-04 07:22 PDT, Andres Gonzalez 		no flags Details | Formatted Diff | Diff
Patch (51.76 KB, patch)
2023-05-23 11:59 PDT, Andres Gonzalez 		no flags Details | Formatted Diff | Diff
Show Obsolete (6) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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].