Bug 229045

Summary: Expose [WebAccessibilityObjectWrapper lineRectsAndText] on MacOS.
Product: WebKit Reporter: Andres Gonzalez <andresg_22>
Component: New BugsAssignee: Andres Gonzalez <andresg_22>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, cfleizach, dmazzoni, ews-watchlist, jcraig, jdiggs, samuel_white
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=229261
Attachments:
Description Flags
Patch
none
Patch none

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