Bug 261838 - AX: AccessibilityObject::listMarkerTextForNodeAndPosition forces resolution of Position to VisiblePosition even when it's not used
Summary: AX: AccessibilityObject::listMarkerTextForNodeAndPosition forces resolution o...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tyler Wilcock
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-09-20 13:38 PDT by Tyler Wilcock
Modified: 2023-09-21 00:34 PDT (History)
10 users (show)

See Also:

Attachments
Patch (3.87 KB, patch)
2023-09-20 14:06 PDT, Tyler Wilcock 		no flags Details | Formatted Diff | Diff
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 Tyler Wilcock 2023-09-20 13:38:12 PDT 
It can be expensive to turn a Position into a VisiblePosition. We should only do it when necessary.
Comment 1 Radar WebKit Bug Importer 2023-09-20 13:38:32 PDT 
<rdar://problem/115801743>
Comment 2 Tyler Wilcock 2023-09-20 14:06:41 PDT 
Created attachment 467794 [details]
Patch
Comment 3 chris fleizach 2023-09-20 14:36:54 PDT 
Comment on attachment 467794 [details]
Patch

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

> Source/WebCore/accessibility/AccessibilityObject.cpp:1704
> +    return listItem ? listMarkerText(*listItem, WTFMove(startPosition)) : StringView();

does this work

return listItem ? listMarkerText(*listItem, WTFMove(startPosition)) : { };
Comment 4 Tyler Wilcock 2023-09-20 14:47:19 PDT 
(In reply to chris fleizach from comment #3)
> Comment on attachment 467794 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=467794&action=review
> 
> > Source/WebCore/accessibility/AccessibilityObject.cpp:1704
> > +    return listItem ? listMarkerText(*listItem, WTFMove(startPosition)) : StringView();
> 
> does this work
> 
> return listItem ? listMarkerText(*listItem, WTFMove(startPosition)) : { };
It doesn't compile unfortunately. C++ doesn't allow you to have the curly-brace initializer on the right side of a ternary.

/Users/twilco/projects/web/OpenSource/Source/WebCore/accessibility/AccessibilityObject.cpp:1710:71: error: initializer list cannot be used on the right hand side of operator ':'
    return listItem ? listMarkerText(*listItem, startVisiblePosition) : { };
Comment 5 chris fleizach 2023-09-20 14:58:04 PDT 
(In reply to Tyler Wilcock from comment #4)
> (In reply to chris fleizach from comment #3)
> > Comment on attachment 467794 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=467794&action=review
> > 
> > > Source/WebCore/accessibility/AccessibilityObject.cpp:1704
> > > +    return listItem ? listMarkerText(*listItem, WTFMove(startPosition)) : StringView();
> > 
> > does this work
> > 
> > return listItem ? listMarkerText(*listItem, WTFMove(startPosition)) : { };
> It doesn't compile unfortunately. C++ doesn't allow you to have the
> curly-brace initializer on the right side of a ternary.
> 
> /Users/twilco/projects/web/OpenSource/Source/WebCore/accessibility/
> AccessibilityObject.cpp:1710:71: error: initializer list cannot be used on
> the right hand side of operator ':'
>     return listItem ? listMarkerText(*listItem, startVisiblePosition) : { };

sad face emoji
Comment 6 EWS 2023-09-20 22:01:19 PDT 
Found 1 new test failure: imported/w3c/web-platform-tests/css/css-transitions/changing-while-transition-001.html
Comment 7 EWS 2023-09-21 00:34:07 PDT 
Committed 268239@main (42da7d77805d): <https://commits.webkit.org/268239@main>

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