Bug 229045 - Expose [WebAccessibilityObjectWrapper lineRectsAndText] on MacOS.
Summary: Expose [WebAccessibilityObjectWrapper lineRectsAndText] on MacOS.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andres Gonzalez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-08-12 12:10 PDT by Andres Gonzalez
Modified: 2021-08-18 16:56 PDT (History)
8 users (show)

See Also:


Attachments
Patch (53.67 KB, patch)
2021-08-14 11:15 PDT, Andres Gonzalez
no flags Details | Formatted Diff | Diff
Patch (55.18 KB, patch)
2021-08-15 08:01 PDT, Andres Gonzalez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andres Gonzalez 2021-08-12 12:10:49 PDT
Expose [WebAccessibilityObjectWrapper lineRectsAndText] on MacOS.
Comment 1 Andres Gonzalez 2021-08-12 12:15:26 PDT
<rdar://81819301>
Comment 2 Andres Gonzalez 2021-08-14 11:15:00 PDT
Created attachment 435541 [details]
Patch
Comment 3 Andres Gonzalez 2021-08-15 08:01:29 PDT
Created attachment 435563 [details]
Patch

Skip test on mac-wk1 and win.
Comment 4 Andres Gonzalez 2021-08-15 08:07:20 PDT
The style error is that the second line in:

        return @{ @"rect": [NSValue valueWithRect:[self convertRectToSpace:FloatRect(line.first) space:AccessibilityConversionSpace::Screen]],
                  @"text": line.second.get() };

Has an odd number of indent spaces to align the two entries in the dictionary. I believe it makes it more readable this way, so left it as is.
Comment 5 Andres Gonzalez 2021-08-16 07:44:05 PDT
The win test failures are unrelated to this patch.
Comment 6 chris fleizach 2021-08-16 09:07:29 PDT
Comment on attachment 435563 [details]
Patch

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

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm:341
> +    // rdar://7980318 if we start a call, then block in WebThreadLock(), then we're dealloced on another, thread, we could

if we're going to move this method around I feel like maybe we should combine it together. it's about the same except the WebThreadLock part which could be conditionalized

We probably need to leave the _prepareAXCall method itself as a pass through until we update the bundle side
Comment 7 Andres Gonzalez 2021-08-16 10:04:11 PDT
(In reply to chris fleizach from comment #6)
> Comment on attachment 435563 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=435563&action=review
> 
> > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm:341
> > +    // rdar://7980318 if we start a call, then block in WebThreadLock(), then we're dealloced on another, thread, we could
> 
> if we're going to move this method around I feel like maybe we should
> combine it together. it's about the same except the WebThreadLock part which
> could be conditionalized
> 
> We probably need to leave the _prepareAXCall method itself as a pass through
> until we update the bundle side

Agree. I added this comment to the base header:

// FIXME: unified these two methods into one.
#if PLATFORM(MAC)
// Updates the underlying object and accessibility hierarchy , and returns the
// corresponding AXCoreObject.
- (WebCore::AXCoreObject*)updateObjectBackingStore;
#else
- (BOOL)_prepareAccessibilityCall;
#endif

I'll make that change separate. The request was not to make any cleanup/rework changes until branching to avoid possible merge/cherry picking conflicts, so I tried to keep this change to the minimum possible.
Comment 8 EWS 2021-08-16 14:04:26 PDT
Committed r281108 (240567@main): <https://commits.webkit.org/240567@main>

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