Bug 208074 - AXIsolatedObject support for tables.
Summary: AXIsolatedObject support for tables.
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-02-21 13:51 PST by Andres Gonzalez
Modified: 2020-02-23 16:55 PST (History)
10 users (show)

See Also:


Attachments
Patch (152.92 KB, patch)
2020-02-21 14:41 PST, Andres Gonzalez
no flags Details | Formatted Diff | Diff
Patch (154.02 KB, patch)
2020-02-22 10:21 PST, Andres Gonzalez
no flags Details | Formatted Diff | Diff
Patch (154.03 KB, patch)
2020-02-23 10:33 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-02-21 13:51:50 PST
AXIsolatedObject support for tables.
Comment 1 Andres Gonzalez 2020-02-21 14:41:03 PST
Created attachment 391428 [details]
Patch
Comment 2 Andres Gonzalez 2020-02-22 10:21:23 PST
Created attachment 391462 [details]
Patch
Comment 3 chris fleizach 2020-02-22 12:05:44 PST
Comment on attachment 391462 [details]
Patch

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

> Source/WebCore/accessibility/AccessibilityTableRow.cpp:71
> +    return is<AccessibilityTable>(table)  && downcast<AccessibilityTable>(*table).isExposable();

extra space after

isAccessibilityTable>(table)

> Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:481
> +    auto cell = Accessibility::retrieveValueFromMainThread<AXCoreObject*>([&columnIndex, &rowIndex, this] () -> AXCoreObject* {

I feel like we should get the cellID on the main thread instead of returning the object off the main thread and then access ivars off the main thread

> Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h:398
> +    int axColumnCount() const override { return intAttributeValue(AXPropertyName::AXColumnCount); }

can these be unsigned or do they need to be int?
Comment 4 Andres Gonzalez 2020-02-23 10:33:56 PST
Created attachment 391492 [details]
Patch
Comment 5 Andres Gonzalez 2020-02-23 10:37:33 PST
(In reply to chris fleizach from comment #3)
> Comment on attachment 391462 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=391462&action=review
> 
> > Source/WebCore/accessibility/AccessibilityTableRow.cpp:71
> > +    return is<AccessibilityTable>(table)  && downcast<AccessibilityTable>(*table).isExposable();
> 
> extra space after
> 
> isAccessibilityTable>(table)

Fixed.
> 
> > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:481
> > +    auto cell = Accessibility::retrieveValueFromMainThread<AXCoreObject*>([&columnIndex, &rowIndex, this] () -> AXCoreObject* {
> 
> I feel like we should get the cellID on the main thread instead of returning
> the object off the main thread and then access ivars off the main thread

Good point, done.
> 
> > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h:398
> > +    int axColumnCount() const override { return intAttributeValue(AXPropertyName::AXColumnCount); }
> 
> can these be unsigned or do they need to be int?

They can be -1. In the implementation we have the following comment:

    // The ARIA spec states, "Authors must set the value of aria-rowcount to an integer equal to the
    // number of rows in the full table. If the total number of rows is unknown, authors must set
    // the value of aria-rowcount to -1 to indicate that the value should not be calculated by the
    // user agent." ...
Comment 6 WebKit Commit Bot 2020-02-23 16:54:47 PST
Comment on attachment 391492 [details]
Patch

Clearing flags on attachment: 391492

Committed r257200: <https://trac.webkit.org/changeset/257200>
Comment 7 WebKit Commit Bot 2020-02-23 16:54:49 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2020-02-23 16:55:15 PST
<rdar://problem/59710821>