Implement NSAccessibilityScrollToVisibleAction on Mac

Implement NSAccessibilityScrollToVisibleAction on Mac

If you navigate to an element with VoiceOver, Chrome wasn't
scrolling it into view (unless it was focusable). To fix this
we mainly just needed to implement
NSAccessibilityScrollToVisibleAction.

To get this to work in PDF files too, we needed an extra fix
since the PDF node is outside of web content but can't scroll
itself, so we get the PDF node's coordinates and then tell the
web document to scroll to make that bounding box visible.

BUG=649861, 54724
Committed: https://crrev.com/7d7a94d999847d71bc9f0557fd4103efa5a8ed71
Cr-Commit-Position: refs/heads/master@{#420977}

[dmazzoni, 2016-09-26 16:20:54]

dmazzoni@chromium.org changed reviewers:
+ ellyjones@chromium.org

[dmazzoni, 2016-09-26 16:20:58]

The CQ bit was checked by dmazzoni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-26 16:21:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Elly Fong-Jones, 2016-09-26 16:25:39]

lgtm!

https://codereview.chromium.org/2365263003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/accessibility/AXObject.cpp (right):

https://codereview.chromium.org/2365263003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/accessibility/AXObject.cpp:1519: const
AXObject* scrollParent = this;
what's up with this?

[commit-bot: I haz the power, 2016-09-26 16:54:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-26 16:54:47]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dmazzoni, 2016-09-26 18:00:40]

https://codereview.chromium.org/2365263003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/accessibility/AXObject.cpp (right):

https://codereview.chromium.org/2365263003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/accessibility/AXObject.cpp:1519: const
AXObject* scrollParent = this;
On 2016/09/26 at 16:25:39, Elly Fong-Jones wrote:
> what's up with this?

It's part of the change.

Previously we didn't have a use case for calling
scrollToMakeVisibleWithSubFocus when the object itself
is the one that needs to scroll. Previously we'd call
it on an object like a heading or a button and it'd
search up for the nearest scrollable container.

In this case we want to call it on the document, which
is scrollable - but we want to tell it to scroll to a
certain bounding box rect (subfocus).

[Elly Fong-Jones, 2016-09-26 18:06:46]

On 2016/09/26 18:00:40, dmazzoni wrote:
>
https://codereview.chromium.org/2365263003/diff/1/third_party/WebKit/Source/m...
> File third_party/WebKit/Source/modules/accessibility/AXObject.cpp (right):
> 
>
https://codereview.chromium.org/2365263003/diff/1/third_party/WebKit/Source/m...
> third_party/WebKit/Source/modules/accessibility/AXObject.cpp:1519: const
> AXObject* scrollParent = this;
> On 2016/09/26 at 16:25:39, Elly Fong-Jones wrote:
> > what's up with this?
> 
> It's part of the change.
> 
> Previously we didn't have a use case for calling
> scrollToMakeVisibleWithSubFocus when the object itself
> is the one that needs to scroll. Previously we'd call
> it on an object like a heading or a button and it'd
> search up for the nearest scrollable container.
> 
> In this case we want to call it on the document, which
> is scrollable - but we want to tell it to scroll to a
> certain bounding box rect (subfocus).

Got it, thanks!

[dmazzoni, 2016-09-26 18:40:12]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2016-09-26 18:40:13]

The patchset sent to the CQ was uploaded after l-g-t-m from
ellyjones@chromium.org
Link to the patchset: https://codereview.chromium.org/2365263003/#ps20001
(title: "Fix endless loop in AXObject::scrollToMakeVisibleWithSubFocus")

[commit-bot: I haz the power, 2016-09-26 18:41:00]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-26 20:23:09]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-26 20:26:35]

Description was changed from

==========
Implement NSAccessibilityScrollToVisibleAction on Mac

If you navigate to an element with VoiceOver, Chrome wasn't
scrolling it into view (unless it was focusable). To fix this
we mainly just needed to implement
NSAccessibilityScrollToVisibleAction.

To get this to work in PDF files too, we needed an extra fix
since the PDF node is outside of web content but can't scroll
itself, so we get the PDF node's coordinates and then tell the
web document to scroll to make that bounding box visible.

BUG=649861,54724
==========

to

==========
Implement NSAccessibilityScrollToVisibleAction on Mac

If you navigate to an element with VoiceOver, Chrome wasn't
scrolling it into view (unless it was focusable). To fix this
we mainly just needed to implement
NSAccessibilityScrollToVisibleAction.

To get this to work in PDF files too, we needed an extra fix
since the PDF node is outside of web content but can't scroll
itself, so we get the PDF node's coordinates and then tell the
web document to scroll to make that bounding box visible.

BUG=649861,54724

Committed: https://crrev.com/7d7a94d999847d71bc9f0557fd4103efa5a8ed71
Cr-Commit-Position: refs/heads/master@{#420977}
==========

[commit-bot: I haz the power, 2016-09-26 20:26:36]

Patchset 2 (id:??) landed as
https://crrev.com/7d7a94d999847d71bc9f0557fd4103efa5a8ed71
Cr-Commit-Position: refs/heads/master@{#420977}