Bug 208074

Created attachment 391428 [details]
Patch

Created attachment 391462 [details]
Patch

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?

Created attachment 391492 [details]
Patch

(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 on attachment 391492 [details]
Patch

Clearing flags on attachment: 391492

Committed r257200: <https://trac.webkit.org/changeset/257200>

All reviewed patches have been landed.  Closing bug.

<rdar://problem/59710821>