Bug 208434 - Fix for LayoutTests/accessibility/mac/search-text/search-text.html in IsolatedTree mode.
Summary: Fix for LayoutTests/accessibility/mac/search-text/search-text.html in Isolate...
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: 2020-03-01 16:42 PST by Andres Gonzalez
Modified: 2020-03-03 11:33 PST (History)
11 users (show)

See Also:


Attachments
Patch (32.15 KB, patch)
2020-03-01 17:02 PST, Andres Gonzalez
no flags Details | Formatted Diff | Diff
Patch (32.53 KB, patch)
2020-03-02 05:32 PST, Andres Gonzalez
no flags Details | Formatted Diff | Diff
Patch (34.55 KB, patch)
2020-03-02 13:30 PST, 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 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