AX: iOS: scrollable elements do not allow 3-finger swipe (with VoiceOver)

[Michael Johnston]

Elements which have overflow: scroll; + -webkit-overflow-scrolling: touch; do not work with the 3-finger swipe gesture when VoiceOver is enabled. This worked in iOS 7.

Expected result: VoiceOver should announce "Page N of M" where N is the current page and M is the total number of pages determined by scroll height
Actual result: VoiceOver announces "Page 1 of 1" regardless of the scroll height

[Radar WebKit Bug Importer]

<rdar://problem/19918933>

[chris fleizach]

Created attachment 257143 [details]
patch

[chris fleizach]

Created attachment 257185 [details]
patch

[chris fleizach]

Created attachment 257204 [details]
patch

[Mario Sanchez Prada]

Comment on attachment 257204 [details]
patch

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

Overall looks good to me (and I like the idea of moving that into AccessibilityObject, too).

Setting r+ with just a few small comments below...

> Source/WebCore/ChangeLog:15
> +        (WebCore::AccessibilityObject::scrollToGlobalPoint):

It does not look like you modified this

> Source/WebCore/ChangeLog:21
> +        (WebCore::AccessibilityObject::lastKnownIsIgnoredValue):

Same for this one (not modified)

> Source/WebCore/ChangeLog:24
> +        (WebCore::AccessibilityObject::getScrollableAreaIfScrollable):
> +        (WebCore::AccessibilityObject::scrollTo):

Ditto for these two.

> Source/WebCore/accessibility/AccessibilityObject.cpp:2538
> +    for (; scrollers.second && !(scrollers.first = scrollers.second->getScrollableAreaIfScrollable()); scrollers.second = scrollers.second->parentObject()) { }

I find this line a bit hard to read. What about this:

// Search up the parent chain until we find the first one that's scrollable.
scrollers.first = nullptr;
scrollers.second = parentObject();
while (!scrollers.first && scrollers.second) {
  scrollers.first = scrollers.second->getScrollableAreaIfScrollable());
  scrollers.second = scrollers.second->parentObject());
}

Or even:

// Search up the parent chain until we find the first one that's scrollable.
scrollers.first = nullptr;
for (scrollers.second = parentObject(); !scrollers.first && scrollers.second; scrollers.second = scrollers.second->parentObject())
  scrollers.first = scrollers.second->getScrollableAreaIfScrollable());

> Source/WebCore/accessibility/AccessibilityObject.cpp:2565
> +    std::pair<ScrollableArea*, AccessibilityObject*> scrollers;
> +    scrollAreaAndAncestor(scrollers);
> +    if (!scrollers.first)

It looks like you could put these three lines in a separate function returning a ScrollableArea*, and use it from this and the previous two functions (but not from scrollByPage, as you also need the .second element there)

> Tools/DumpRenderTree/mac/AccessibilityNotificationHandler.mm:183
> -        JSObjectCallAsFunction([mainFrame globalContext], m_notificationFunctionCallback, 0, 2, arguments, 0);
> +        JSObjectCallAsFunction([mainFrame globalContext], m_notificationFunctionCallback, 0, 3, arguments, 0);

Nice catch

[chris fleizach]

http://trac.webkit.org/changeset/187371