Bug 208434

Summary: Fix for LayoutTests/accessibility/mac/search-text/search-text.html in IsolatedTree mode.
Product: WebKit Reporter: Andres Gonzalez <andresg_22>
Component: New BugsAssignee: Andres Gonzalez <andresg_22>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, aboxhall, apinheiro, cfleizach, commit-queue, dmazzoni, ews-watchlist, jcraig, jdiggs, samuel_white, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=208488
https://bugs.webkit.org/show_bug.cgi?id=208520
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Andres Gonzalez 2020-03-01 16:42:55 PST 
Fix for LayoutTests/accessibility/mac/search-text/search-text.html in IsolatedTree mode.
Comment 1 Andres Gonzalez 2020-03-01 17:02:36 PST 
Created attachment 392111 [details]
Patch
Comment 2 chris fleizach 2020-03-01 22:43:27 PST 
Comment on attachment 392111 [details]
Patch

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

> Source/WebCore/accessibility/AccessibilityObject.h:759
> +    AccessibilityObject* webAreaObject() const override { return nullptr; }

the type here is AccessibilityObject, but in the interface it's AXCoreObject

> Source/WebCore/accessibility/AccessibilityScrollView.h:44
> +    AccessibilityObject* webAreaObject() const override;

interface class defines this as AXCoreObject

> Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:229
> +            && item.m_wrapper)

this line can go on the last one. it looks like it's short enough
Comment 3 Andres Gonzalez 2020-03-02 05:32:02 PST 
Created attachment 392134 [details]
Patch
Comment 4 Andres Gonzalez 2020-03-02 05:43:09 PST 
(In reply to chris fleizach from comment #2)
> Comment on attachment 392111 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=392111&action=review
> 
> > Source/WebCore/accessibility/AccessibilityObject.h:759
> > +    AccessibilityObject* webAreaObject() const override { return nullptr; }
> 
> the type here is AccessibilityObject, but in the interface it's AXCoreObject

It is a long chain to change this since callers are assigning the return value to AccessibilityObject. It works because the override is returning a subclass. There are other methods whose overrides also return a subclass. I propose we leave it like this for now, and make a complete pass through to clean them up all at once in a separate patch.
> 
> > Source/WebCore/accessibility/AccessibilityScrollView.h:44
> > +    AccessibilityObject* webAreaObject() const override;
> 
> interface class defines this as AXCoreObject

Same comment as above.
> 
> > Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:229
> > +            && item.m_wrapper)
> 
> this line can go on the last one. it looks like it's short enough

Done.
Comment 5 Andres Gonzalez 2020-03-02 13:30:02 PST 
Created attachment 392180 [details]
Patch
Comment 6 Andres Gonzalez 2020-03-02 13:31:54 PST 
(In reply to Andres Gonzalez from comment #5)
> Created attachment 392180 [details]
> Patch

Fix for GTK build.
Comment 7 WebKit Commit Bot 2020-03-02 14:50:37 PST 
Comment on attachment 392180 [details]
Patch

Clearing flags on attachment: 392180

Committed r257739: <https://trac.webkit.org/changeset/257739>
Comment 8 WebKit Commit Bot 2020-03-02 14:50:38 PST 
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2020-03-02 14:51:16 PST 
<rdar://problem/59969174>
Comment 10 Ryan Haddad 2020-03-02 17:16:16 PST 
This change appears to have introduced layout test crashes, as indicated by EWS: https://ews-build.webkit.org/#/builders/31/builds/4027

See: https://bugs.webkit.org/show_bug.cgi?id=208488