Bug 255138

Summary:

AX: Eliminate all ways AXCoreObject::elementRect could be called on an AXIsolatedObject

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
Patch	none
Patch	none

Description Tyler Wilcock 2023-04-06 21:26:52 PDT

In a future patch, we will be caching the AXCoreObject::relativeFrame of each isolated object. The relative frame is the position of an object relative to the current viewport (i.e. relative frame is affected by things like scrolling). This is different than `AXCoreObject::elementRect`, which is the "absolute" rect (relative to the root document origin, not effected by scrolling).

To avoid the need to cache both `elementRect` and `relativeFrame`, we should eliminate all ways `elementRect` could be called on an `AXIsolatedObject` in the wrapper and elsewhere.

Comment 1 Radar WebKit Bug Importer 2023-04-06 21:27:02 PDT

<rdar://problem/107741185>

Comment 2 Tyler Wilcock 2023-04-06 21:31:24 PDT

Created attachment 465807 [details]
Patch

Comment 3 chris fleizach 2023-04-07 07:42:18 PDT

Can you fix spacing after : here

1768        auto size = backingObject->size();
 1769        return [NSValue valueWithSize: NSMakeSize(size.width(),

Comment 4 Tyler Wilcock 2023-04-07 09:25:42 PDT

Created attachment 465810 [details]
Patch

Comment 5 chris fleizach 2023-04-07 12:11:16 PDT

Comment on attachment 465810 [details]
Patch

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

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1392
> +    return @(cgPoint);

do we need two lines of code here can can we do in one

also does @() turn this into a NSValue? I had always hoped it would, but don't know if I tested it

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1769
> +        return [NSValue valueWithSize:NSMakeSize(size.width(), size.height())];

is there no overload for

(NSValue *)backingObject->size()

Comment 6 Tyler Wilcock 2023-04-07 21:25:47 PDT

Created attachment 465815 [details]
Patch

Comment 7 Tyler Wilcock 2023-04-07 21:26:59 PDT

(In reply to chris fleizach from comment #5)
> Comment on attachment 465810 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=465810&action=review
> 
> > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1392
> > +    return @(cgPoint);
> 
> do we need two lines of code here can can we do in one
> 
> also does @() turn this into a NSValue? I had always hoped it would, but
> don't know if I tested it
Fixed this. And yes, @() on "some" (quoting https://clang.llvm.org/docs/ObjectiveCLiterals.html) structs will result in an NSValue wrapper. The basic CG structs are included in this "some" (I just confirmed it). 

> > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1769
> > +        return [NSValue valueWithSize:NSMakeSize(size.width(), size.height())];
> 
> is there no overload for
> 
> (NSValue *)backingObject->size()
Checked and unfortunately there's no overload to automatically turn an IntSize into an NSValue.

Comment 8 EWS 2023-04-08 11:10:54 PDT

Committed 262752@main (2d9b495ba023): <https://commits.webkit.org/262752@main>

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