Commit 6ad725ce authored by Dominic Mazzoni's avatar Dominic Mazzoni Committed by Commit Bot

Android accessibility: improve handling of same-page links

This is a new attempt at fixing bug 657157, previous attempts were
reverted due to regressions.

When setting accessibility focus to a node, auto-focus it if it's a link.
This is needed for some sites that have skip links that are only visible
when focused.

In addition, when following a same-page link, move accessibility focus
to the new target location on the page only after a short delay. This
avoids a race condition due to focus also changing at the same time.

Tested manually on three pages mentioned in bug 657157:
1. getbootstrap.com, which has a same-page link that's only visible
   when focused
2. http://jsfiddle.net/mev0c4dt/show/ - a test page for a regression
   that happened with a previous attempt to fix this bug
3. https://codepen.io/artesea/pen/jVdLXP - a test page for a regression
   that happened with a previous attempt to fix this bug

Bug: 657157
Change-Id: I791544af3074f5aac5116c6695332d8178da21d6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1873964Reviewed-by: default avatarAkihiro Ota <akihiroota@chromium.org>
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710150}
parent 155e5253
......@@ -1099,6 +1099,11 @@ void WebContentsAccessibilityAndroid::MoveAccessibilityFocus(
// as that will result in loading inline text boxes for the whole tree.
if (node != node->manager()->GetRoot())
node->manager()->LoadInlineTextBoxes(*node);
// Auto-focus links, because some websites have skip links that are
// only visible when focused. See http://crbug.com/657157
if (node->IsLink())
node->manager()->SetFocus(*node);
}
bool WebContentsAccessibilityAndroid::IsSlider(JNIEnv* env,
......
......@@ -66,6 +66,7 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
private static final String ACTION_ARGUMENT_SET_TEXT_CHARSEQUENCE =
"ACTION_ARGUMENT_SET_TEXT_CHARSEQUENCE";
private static final int WINDOW_CONTENT_CHANGED_DELAY_MS = 500;
private static final int SCROLLED_TO_ANCHOR_DELAY_MS = 500;
// Constants from AccessibilityNodeInfo defined in the M SDK.
// Source: https://developer.android.com/reference/android/R.id.html
......@@ -100,6 +101,7 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
protected int mAccessibilityFocusId;
protected int mSelectionNodeId;
private Runnable mSendWindowContentChangedRunnable;
private Runnable mScrolledToAnchorRunnable;
private View mAutofillPopupView;
private CaptioningController mCaptioningController;
......@@ -1048,8 +1050,27 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
}
@CalledByNative
private void handleScrolledToAnchor(int id) {
private void handleScrolledToAnchor(final int id) {
// "Scrolled to anchor" means that the user followed a same-page link;
// the id here is of the element that was scrolled into view, and that
// should be where accessibility focus lands.
//
// However, in practice there's a race condition because following a
// same-page link often results in a focus change from the same-page link
// that was focused previously.
//
// As a result, it works better to move accessibility focus to the new
// location after a short delay, after the focus change.
if (mScrolledToAnchorRunnable != null) return;
mScrolledToAnchorRunnable = new Runnable() {
@Override
public void run() {
moveAccessibilityFocusToId(id);
mScrolledToAnchorRunnable = null;
}
};
mView.postDelayed(mScrolledToAnchorRunnable, SCROLLED_TO_ANCHOR_DELAY_MS);
}
@CalledByNative
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment