Bug 261838

Summary:

AX: AccessibilityObject::listMarkerTextForNodeAndPosition forces resolution of Position to VisiblePosition even when it's not used

Product:

WebKit

Reporter:

Tyler Wilcock <tyler_w>

Component:

Accessibility

Assignee:

Tyler Wilcock <tyler_w>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aboxhall, andresg_22, apinheiro, cfleizach, dmazzoni, ews-watchlist, jcraig, jdiggs, samuel_white, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

Other

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none

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].