Bug 255138 - AX: Eliminate all ways AXCoreObject::elementRect could be called on an AXIsolatedObject
Summary: AX: Eliminate all ways AXCoreObject::elementRect could be called on an AXIsol...
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-04-06 21:26 PDT by Tyler Wilcock
Modified: 2023-04-08 11:10 PDT (History)
10 users (show)

See Also:


Attachments
Patch (11.63 KB, patch)
2023-04-06 21:31 PDT, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (11.63 KB, patch)
2023-04-07 09:25 PDT, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (11.80 KB, patch)
2023-04-07 21:25 PDT, Tyler Wilcock
no flags Details | Formatted Diff | Diff

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