Skip to content

Commit

Permalink
Update autofill keyboard accessory a11y.
Browse files Browse the repository at this point in the history
The bug is about focus after autofill, but the keyboard accessory
has similar default Home focus in other places, this CL tries to
address the whole screen reader UX as these places are also related
in code.

Key points of changes:
1. Put a11y focus on the list of password options (first item)
   upon opening (was on the Home button)
1.1. Announcement of opened password options (Showing saved passwords
     and password options) removed, the description of the trigger
     button is changed to compensate it.
2. Put a11y focus on the active DOM element after closing Password
   options list and showing keyboard (was on the Home button)
3. Put a11y focus on the active DOM element after autofilling (was on
   the Home button)

Please find these changes in the same order performed in videos:

before: http://screencast/cast/NTcwNTQ2NzA1MDMyODA2NHw1MDcxYjVkYi04ZQ
after: http://screencast/cast/NDk3MDgxMzc2Njc2MjQ5Nnw1NGNjZjM4Zi04YQ

Bug: 1136267
Change-Id: Ibeb1df7bdbb623916797235c9786d63a6daf514f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4129649
Commit-Queue: Dmitry Vykochko <vykochko@google.com>
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1108251}
  • Loading branch information
DVykochko authored and Chromium LUCI CQ committed Feb 22, 2023
1 parent 46cebed commit 89a0507
Show file tree
Hide file tree
Showing 37 changed files with 323 additions and 171 deletions.
1 change: 1 addition & 0 deletions chrome/android/features/keyboard_accessory/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ robolectric_binary("keyboard_accessory_junit_tests") {
"//chrome/browser/password_manager/android:java",
"//chrome/browser/tab:java",
"//chrome/browser/tabmodel:java",
"//chrome/browser/util:java",
"//chrome/test/android:chrome_java_unit_test_support",
"//components/autofill/android:autofill_java",
"//components/browser_ui/bottomsheet/android:java",
Expand Down
4 changes: 3 additions & 1 deletion chrome/android/features/keyboard_accessory/internal/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,11 @@ android_library("internal_java") {
"java/src/org/chromium/chrome/browser/keyboard_accessory/sheet_component/AccessorySheetViewBinder.java",
"java/src/org/chromium/chrome/browser/keyboard_accessory/sheet_component/NoSwipeViewPager.java",
"java/src/org/chromium/chrome/browser/keyboard_accessory/sheet_tabs/AccessorySheetTabCoordinator.java",
"java/src/org/chromium/chrome/browser/keyboard_accessory/sheet_tabs/AccessorySheetTabItemsModel.java",
"java/src/org/chromium/chrome/browser/keyboard_accessory/sheet_tabs/AccessorySheetTabMediator.java",
"java/src/org/chromium/chrome/browser/keyboard_accessory/sheet_tabs/AccessorySheetTabMetricsRecorder.java",
"java/src/org/chromium/chrome/browser/keyboard_accessory/sheet_tabs/AccessorySheetTabModel.java",
"java/src/org/chromium/chrome/browser/keyboard_accessory/sheet_tabs/AccessorySheetTabProperties.java",
"java/src/org/chromium/chrome/browser/keyboard_accessory/sheet_tabs/AccessorySheetTabView.java",
"java/src/org/chromium/chrome/browser/keyboard_accessory/sheet_tabs/AccessorySheetTabViewBinder.java",
"java/src/org/chromium/chrome/browser/keyboard_accessory/sheet_tabs/AddressAccessoryInfoView.java",
"java/src/org/chromium/chrome/browser/keyboard_accessory/sheet_tabs/AddressAccessorySheetCoordinator.java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Use of this source code is governed by a BSD-style license that can be
found in the LICENSE file.
-->

<androidx.recyclerview.widget.RecyclerView
<org.chromium.chrome.browser.keyboard_accessory.sheet_tabs.AccessorySheetTabView
xmlns:android="http://schemas.android.com/apk/res/android"
android:id="@+id/addresses_sheet"
android:fillViewport="true"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Use of this source code is governed by a BSD-style license that can be
found in the LICENSE file.
-->

<androidx.recyclerview.widget.RecyclerView
<org.chromium.chrome.browser.keyboard_accessory.sheet_tabs.AccessorySheetTabView
xmlns:android="http://schemas.android.com/apk/res/android"
android:id="@+id/credit_card_sheet"
android:fillViewport="true"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Use of this source code is governed by a BSD-style license that can be
found in the LICENSE file.
-->

<androidx.recyclerview.widget.RecyclerView
<org.chromium.chrome.browser.keyboard_accessory.sheet_tabs.AccessorySheetTabView
xmlns:android="http://schemas.android.com/apk/res/android"
android:id="@+id/passwords_sheet"
android:fillViewport="true"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
import org.chromium.chrome.browser.tab.TabObserver;
import org.chromium.chrome.browser.tabmodel.TabModelObserver;
import org.chromium.chrome.browser.tabmodel.TabModelSelectorTabModelObserver;
import org.chromium.chrome.browser.util.ChromeAccessibilityUtil;
import org.chromium.components.autofill.AutofillDelegate;
import org.chromium.components.autofill.AutofillSuggestion;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
Expand All @@ -67,6 +68,7 @@
import org.chromium.components.browser_ui.widget.InsetObserverViewSupplier;
import org.chromium.components.browser_ui.widget.gesture.BackPressHandler;
import org.chromium.content_public.browser.WebContents;
import org.chromium.content_public.browser.WebContentsAccessibility;
import org.chromium.ui.DropdownPopupWindow;
import org.chromium.ui.base.ViewUtils;
import org.chromium.ui.base.WindowAndroid;
Expand Down Expand Up @@ -498,6 +500,7 @@ private void enforceStateProperties(@KeyboardExtensionState int extensionState)
ViewUtils.requestLayout(compositorViewHolderSupplier.get(),
"ManualFillingMediator.enforceStateProperties");
}
trySetA11yFocusOnWebContents();
}
TraceEvent.end("ManualFillingMediator#enforceStateProperties");
}
Expand Down Expand Up @@ -635,6 +638,21 @@ private void changeBottomControlSpaceForState(int extensionState) {
return mActivity.getActivityTabProvider().get();
}

/**
* @return {@link WebContentsAccessibility} instance of the active tab, if available.
*/
private @Nullable WebContentsAccessibility getActiveWebContentsAccessibility() {
if (!ChromeAccessibilityUtil.get().isAccessibilityEnabled()) return null;

Tab tab = getActiveBrowserTab();
if (tab == null) return null;

WebContents webContents = tab.getWebContents();
if (webContents == null) return null;

return WebContentsAccessibility.fromWebContents(webContents);
}

/**
* Registers a {@link TabObserver} to the given {@link Tab} if it hasn't been done yet.
* Using this function avoid deleting and readding the observer (each O(N)) since the tab does
Expand Down Expand Up @@ -807,6 +825,13 @@ private static String getNameForState(@KeyboardExtensionState int state) {
return null;
}

private void trySetA11yFocusOnWebContents() {
WebContentsAccessibility accessibility = getActiveWebContentsAccessibility();
if (accessibility != null) {
accessibility.restoreFocus();
}
}

@VisibleForTesting
void setInsetObserverViewSupplier(Supplier<InsetObserverView> insetObserverViewSupplier) {
mInsetObserverViewSupplier = insetObserverViewSupplier;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import android.view.View;
import android.view.ViewGroup;

import org.chromium.chrome.browser.keyboard_accessory.data.KeyboardAccessoryData;
import org.chromium.ui.modelutil.PropertyKey;
import org.chromium.ui.modelutil.PropertyModel;

Expand All @@ -32,9 +31,6 @@ static void bind(PropertyModel model, View sheetView, PropertyKey propertyKey) {
} else if (propertyKey == VISIBLE) {
view.bringToFront(); // Ensure toolbars and other containers are overlaid.
view.setVisibility(model.get(VISIBLE) ? View.VISIBLE : View.GONE);
if (model.get(VISIBLE) && model.get(ACTIVE_TAB_INDEX) != NO_ACTIVE_TAB) {
announceOpenedTab(view, model.get(TABS).get(model.get(ACTIVE_TAB_INDEX)));
}
} else if (propertyKey == HEIGHT) {
ViewGroup.LayoutParams p = view.getLayoutParams();
p.height = model.get(HEIGHT);
Expand All @@ -53,9 +49,4 @@ static void bind(PropertyModel model, View sheetView, PropertyKey propertyKey) {
assert false : "Every possible property update needs to be handled!";
}
}

static void announceOpenedTab(View announcer, KeyboardAccessoryData.Tab tab) {
if (tab.getOpeningAnnouncement() == null) return;
announcer.announceForAccessibility(tab.getOpeningAnnouncement());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@

package org.chromium.chrome.browser.keyboard_accessory.sheet_tabs;

import static org.chromium.chrome.browser.keyboard_accessory.sheet_tabs.AccessorySheetTabProperties.IS_DEFAULT_A11Y_FOCUS_REQUESTED;
import static org.chromium.chrome.browser.keyboard_accessory.sheet_tabs.AccessorySheetTabProperties.ITEMS;
import static org.chromium.chrome.browser.keyboard_accessory.sheet_tabs.AccessorySheetTabProperties.SCROLL_LISTENER;

import android.content.Context;
import android.graphics.drawable.Drawable;
import android.view.ViewGroup;
Expand All @@ -20,6 +24,8 @@
import org.chromium.chrome.browser.keyboard_accessory.data.KeyboardAccessoryData;
import org.chromium.chrome.browser.keyboard_accessory.data.KeyboardAccessoryData.AccessorySheetData;
import org.chromium.chrome.browser.keyboard_accessory.data.Provider;
import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.modelutil.PropertyModelChangeProcessor;

/**
* This coordinator aims to be the base class for sheets to be added to the ManualFillingCoordinator
Expand All @@ -29,6 +35,8 @@ public abstract class AccessorySheetTabCoordinator implements KeyboardAccessoryD
private final KeyboardAccessoryData.Tab mTab;
private final RecyclerView.OnScrollListener mScrollListener;

protected final PropertyModel mModel;

/**
* Provides the icon used for a sheet. Simplifies mocking in controller tests.
*/
Expand Down Expand Up @@ -58,23 +66,30 @@ public static void setIconForTesting(Drawable icon) {
* @param title A {@link String} permanently displayed in the bar above the keyboard.
* @param icon The icon that represents this sheet in the keyboard accessory tab switcher.
* @param contentDescription A description for this sheet used in the tab switcher.
* @param openingAnnouncement The announced string when opening this sheet.
* @param layout The layout containing all views that are used by this sheet.
* @param tabType The type of this tab as used in histograms.
* @param scrollListener An optional listener that will be bound to an inflated recycler view.
*/
AccessorySheetTabCoordinator(String title, Drawable icon, String contentDescription,
String openingAnnouncement, @LayoutRes int layout, @AccessoryTabType int tabType,
@LayoutRes int layout, @AccessoryTabType int tabType,
@Nullable RecyclerView.OnScrollListener scrollListener) {
mTab = new KeyboardAccessoryData.Tab(
title, icon, contentDescription, openingAnnouncement, layout, tabType, this);
title, icon, contentDescription, layout, tabType, this);
mScrollListener = scrollListener;
mModel = new PropertyModel.Builder(AccessorySheetTabProperties.ALL_KEYS)
.with(ITEMS, new AccessorySheetTabItemsModel())
.with(SCROLL_LISTENER, scrollListener)
.with(IS_DEFAULT_A11Y_FOCUS_REQUESTED, false)
.build();
}

@CallSuper
@Override
public void onTabCreated(ViewGroup view) {
AccessorySheetTabViewBinder.initializeView((RecyclerView) view, mScrollListener);

PropertyModelChangeProcessor.create(
mModel, (AccessorySheetTabView) view, AccessorySheetTabViewBinder::bind);
}

@CallSuper
Expand Down Expand Up @@ -107,4 +122,9 @@ public KeyboardAccessoryData.Tab getTab() {
public void registerDataProvider(Provider<AccessorySheetData> sheetDataProvider) {
sheetDataProvider.addObserver(getMediator());
}

@VisibleForTesting
AccessorySheetTabItemsModel getSheetDataPiecesForTesting() {
return mModel.get(ITEMS);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
* This class describes the {@link ListModel} used for keyboard accessory sheets like the
* {@link PasswordAccessorySheetCoordinator}.
*/
class AccessorySheetTabModel extends ListModel<AccessorySheetTabModel.AccessorySheetDataPiece> {
class AccessorySheetTabItemsModel
extends ListModel<AccessorySheetTabItemsModel.AccessorySheetDataPiece> {
/**
* The {@link AccessorySheetData} has to be mapped to single items in a {@link RecyclerView}.
* This class allows wrapping the {@link AccessorySheetData} into small chunks that are
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

package org.chromium.chrome.browser.keyboard_accessory.sheet_tabs;

import static org.chromium.chrome.browser.keyboard_accessory.sheet_tabs.AccessorySheetTabProperties.IS_DEFAULT_A11Y_FOCUS_REQUESTED;
import static org.chromium.chrome.browser.keyboard_accessory.sheet_tabs.AccessorySheetTabProperties.ITEMS;

import androidx.annotation.CallSuper;
import androidx.annotation.Nullable;

Expand All @@ -19,8 +22,9 @@
import org.chromium.chrome.browser.keyboard_accessory.data.KeyboardAccessoryData.PromoCodeInfo;
import org.chromium.chrome.browser.keyboard_accessory.data.KeyboardAccessoryData.UserInfo;
import org.chromium.chrome.browser.keyboard_accessory.data.Provider;
import org.chromium.chrome.browser.keyboard_accessory.sheet_tabs.AccessorySheetTabModel.AccessorySheetDataPiece;
import org.chromium.chrome.browser.keyboard_accessory.sheet_tabs.AccessorySheetTabModel.AccessorySheetDataPiece.Type;
import org.chromium.chrome.browser.keyboard_accessory.sheet_tabs.AccessorySheetTabItemsModel.AccessorySheetDataPiece;
import org.chromium.chrome.browser.keyboard_accessory.sheet_tabs.AccessorySheetTabItemsModel.AccessorySheetDataPiece.Type;
import org.chromium.chrome.browser.util.ChromeAccessibilityUtil;
import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.modelutil.PropertyModelChangeProcessor;

Expand All @@ -33,7 +37,7 @@
* accessory sheet tab view.
*/
class AccessorySheetTabMediator implements Provider.Observer<AccessorySheetData> {
private final AccessorySheetTabModel mModel;
private final PropertyModel mModel;
private final @AccessoryTabType int mTabType;
private final @Type int mUserInfoType;
private final @AccessoryAction int mManageActionToRecord;
Expand All @@ -58,11 +62,11 @@ public interface ToggleChangeDelegate {
@Override
public void onItemAvailable(int typeId, AccessorySheetData accessorySheetData) {
TraceEvent.begin("AccessorySheetTabMediator#onItemAvailable");
mModel.set(splitIntoDataPieces(accessorySheetData));
mModel.get(ITEMS).set(splitIntoDataPieces(accessorySheetData));
TraceEvent.end("AccessorySheetTabMediator#onItemAvailable");
}

AccessorySheetTabMediator(AccessorySheetTabModel model, @AccessoryTabType int tabType,
AccessorySheetTabMediator(PropertyModel model, @AccessoryTabType int tabType,
@Type int userInfoType, @AccessoryAction int manageActionToRecord,
@Nullable ToggleChangeDelegate toggleChangeDelegate) {
mModel = model;
Expand All @@ -74,14 +78,26 @@ public void onItemAvailable(int typeId, AccessorySheetData accessorySheetData) {

@CallSuper
void onTabShown() {
AccessorySheetTabMetricsRecorder.recordSheetSuggestions(mTabType, mModel);
AccessorySheetTabMetricsRecorder.recordSheetSuggestions(mTabType, mModel.get(ITEMS));

// This is a compromise: we log an impression, even if the user didn't scroll down far
// enough to see it. If we moved it into the view layer (i.e. when the actual button is
// created and shown), we could record multiple impressions of the user scrolls up and
// down repeatedly.
ManualFillingMetricsRecorder.recordActionImpression(mManageActionToRecord);
recordToggleImpression();

setDefaultA11yFocus();
}

void setDefaultA11yFocus() {
if (!ChromeAccessibilityUtil.get().isAccessibilityEnabled()) {
return;
}

// Reset the "requested" flag to make sure it takes effect in the binder.
mModel.set(IS_DEFAULT_A11Y_FOCUS_REQUESTED, false);
mModel.set(IS_DEFAULT_A11Y_FOCUS_REQUESTED, true);
}

private AccessorySheetDataPiece[] splitIntoDataPieces(AccessorySheetData accessorySheetData) {
Expand Down Expand Up @@ -128,21 +144,22 @@ private AccessorySheetDataPiece createDataPieceForToggle(OptionToggle toggle) {
}

private void updateOptionToggleEnabled() {
for (int i = 0; i < mModel.size(); ++i) {
AccessorySheetDataPiece data = mModel.get(i);
for (int i = 0; i < mModel.get(ITEMS).size(); ++i) {
AccessorySheetDataPiece data = mModel.get(ITEMS).get(i);
if (AccessorySheetDataPiece.getType(data) == Type.OPTION_TOGGLE) {
OptionToggle toggle = (OptionToggle) data.getDataPiece();
OptionToggle updatedToggle = new OptionToggle(toggle.getDisplayText(),
!toggle.isEnabled(), toggle.getActionType(), toggle.getCallback());
mModel.update(i, new AccessorySheetDataPiece(updatedToggle, Type.OPTION_TOGGLE));
mModel.get(ITEMS).update(
i, new AccessorySheetDataPiece(updatedToggle, Type.OPTION_TOGGLE));
break;
}
}
}

private void recordToggleImpression() {
for (int i = 0; i < mModel.size(); ++i) {
AccessorySheetDataPiece data = mModel.get(i);
for (int i = 0; i < mModel.get(ITEMS).size(); ++i) {
AccessorySheetDataPiece data = mModel.get(ITEMS).get(i);
if (AccessorySheetDataPiece.getType(data) == Type.OPTION_TOGGLE) {
OptionToggle toggle = (OptionToggle) data.getDataPiece();
ManualFillingMetricsRecorder.recordToggleImpression(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
package org.chromium.chrome.browser.keyboard_accessory.sheet_tabs;

import static org.chromium.chrome.browser.keyboard_accessory.ManualFillingMetricsRecorder.getHistogramForType;
import static org.chromium.chrome.browser.keyboard_accessory.sheet_tabs.AccessorySheetTabModel.AccessorySheetDataPiece.Type.ADDRESS_INFO;
import static org.chromium.chrome.browser.keyboard_accessory.sheet_tabs.AccessorySheetTabModel.AccessorySheetDataPiece.Type.CREDIT_CARD_INFO;
import static org.chromium.chrome.browser.keyboard_accessory.sheet_tabs.AccessorySheetTabModel.AccessorySheetDataPiece.Type.PASSWORD_INFO;
import static org.chromium.chrome.browser.keyboard_accessory.sheet_tabs.AccessorySheetTabModel.AccessorySheetDataPiece.Type.PROMO_CODE_INFO;
import static org.chromium.chrome.browser.keyboard_accessory.sheet_tabs.AccessorySheetTabModel.AccessorySheetDataPiece.getType;
import static org.chromium.chrome.browser.keyboard_accessory.sheet_tabs.AccessorySheetTabItemsModel.AccessorySheetDataPiece.Type.ADDRESS_INFO;
import static org.chromium.chrome.browser.keyboard_accessory.sheet_tabs.AccessorySheetTabItemsModel.AccessorySheetDataPiece.Type.CREDIT_CARD_INFO;
import static org.chromium.chrome.browser.keyboard_accessory.sheet_tabs.AccessorySheetTabItemsModel.AccessorySheetDataPiece.Type.PASSWORD_INFO;
import static org.chromium.chrome.browser.keyboard_accessory.sheet_tabs.AccessorySheetTabItemsModel.AccessorySheetDataPiece.Type.PROMO_CODE_INFO;
import static org.chromium.chrome.browser.keyboard_accessory.sheet_tabs.AccessorySheetTabItemsModel.AccessorySheetDataPiece.getType;

import org.chromium.base.metrics.RecordHistogram;
import org.chromium.chrome.browser.keyboard_accessory.AccessoryTabType;
Expand All @@ -35,7 +35,7 @@ private AccessorySheetTabMetricsRecorder() {}
* @param suggestionList The list containing all suggestions.
*/
static void recordSheetSuggestions(@AccessoryTabType int tabType,
ListModel<AccessorySheetTabModel.AccessorySheetDataPiece> suggestionList) {
ListModel<AccessorySheetTabItemsModel.AccessorySheetDataPiece> suggestionList) {
int interactiveSuggestions = 0;
for (int i = 0; i < suggestionList.size(); ++i) {
if (getType(suggestionList.get(i)) == PASSWORD_INFO
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package org.chromium.chrome.browser.keyboard_accessory.sheet_tabs;

import androidx.recyclerview.widget.RecyclerView;

import org.chromium.ui.modelutil.PropertyKey;
import org.chromium.ui.modelutil.PropertyModel.ReadableObjectPropertyKey;
import org.chromium.ui.modelutil.PropertyModel.WritableObjectPropertyKey;

/**
* These properties make up the model of the AccessorySheetTab component.
*/
class AccessorySheetTabProperties {
static final ReadableObjectPropertyKey<AccessorySheetTabItemsModel> ITEMS =
new ReadableObjectPropertyKey<>("items");
static final ReadableObjectPropertyKey<RecyclerView.OnScrollListener> SCROLL_LISTENER =
new ReadableObjectPropertyKey<>("scroll_listener");
static final WritableObjectPropertyKey<Boolean> IS_DEFAULT_A11Y_FOCUS_REQUESTED =
new WritableObjectPropertyKey<>("is_default_a11y_focus_requested");

static final PropertyKey[] ALL_KEYS = {ITEMS, SCROLL_LISTENER, IS_DEFAULT_A11Y_FOCUS_REQUESTED};

private AccessorySheetTabProperties() {}
}

0 comments on commit 89a0507

Please sign in to comment.