From fd1c745dd969cb4147c744cc0bbc16b0c5bdf26e Mon Sep 17 00:00:00 2001 From: msramek Date: Tue, 8 Mar 2016 02:33:42 -0800 Subject: [PATCH] Improve the accessibility of ClearBrowsingDataPreferences This CL fixes accessibility issues of preferences in ClearBrowsingDataPreferences. 1. Checkboxes were losing focus after (un)checking. 2. The time period dropdown value was being re-announced whenever screen was scrolled away and back to it. 3. Checkboxes did not announce when the summary (the result of BrowsingDataCounter) was updated. 1. and 2. are caused by the View of the Preference being recreated. We now cache it in both ClearBrowsingDataCheckBoxPreference and SpinnerPreference. Note that this was also the cause of crbug.com/588687. In addition, in SpinnerPreference, the problem was also caused by adapter being constantly resupplied, even though it hasn't changed. We now only call setAdapter() if a different adapter has actually been passed to SpinnerPreference in the meantime. This is done for generality; currently, we only supply adapter to SpinnerPreference once. Problem 3. was solved by explicitly calling announceForAccessibility(result). Here, we want to provide the user with direct feedback - when a checkbox is checked, a subsequent counter result should be announced. However, when the dialog is just being initialized, or the time period changed, BrowsingDataCounters related to all checkboxes are reset, which would cause unnecessary noise. Therefore, we only announce the result if the recalculation was caused by changing the checkbox state. BUG=590755,588687 Review URL: https://codereview.chromium.org/1748403006 Cr-Commit-Position: refs/heads/master@{#379802} --- .../ClearBrowsingDataCheckBoxPreference.java | 15 ++++++--- .../preferences/SpinnerPreference.java | 19 ++++++++--- .../privacy/ClearBrowsingDataPreferences.java | 32 +++++++++++++++---- 3 files changed, 51 insertions(+), 15 deletions(-) diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/ClearBrowsingDataCheckBoxPreference.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/ClearBrowsingDataCheckBoxPreference.java index 05072296f0540..a7d360c9b2421 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/ClearBrowsingDataCheckBoxPreference.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/ClearBrowsingDataCheckBoxPreference.java @@ -17,6 +17,7 @@ * A preference representing one browsing data type in ClearBrowsingDataPreferences. */ public class ClearBrowsingDataCheckBoxPreference extends ChromeBaseCheckBoxPreference { + private LinearLayout mView; /** * Constructor for inflating from XML. @@ -27,18 +28,20 @@ public ClearBrowsingDataCheckBoxPreference(Context context, AttributeSet attrs) @Override public View onCreateView(ViewGroup parent) { - LinearLayout view = (LinearLayout) super.onCreateView(parent); + if (mView != null) return mView; + + mView = (LinearLayout) super.onCreateView(parent); // Checkboxes in the Clear Browsing Data dialog will show and hide the results of // BrowsingDataCounter. It is important that they will not change height when doing so. // We will therefore set a fixed height. int height = getContext().getResources().getDimensionPixelSize( R.dimen.clear_browsing_data_checkbox_height); - view.setMinimumHeight(height); + mView.setMinimumHeight(height); // The title and summary are enclosed in a common RelativeLayout. We must remove // its vertical padding for it to be correctly vertically centered in the fixed-height view. - View textLayout = (View) view.findViewById(android.R.id.title).getParent(); + View textLayout = (View) mView.findViewById(android.R.id.title).getParent(); ApiCompatibilityUtils.setPaddingRelative( textLayout, ApiCompatibilityUtils.getPaddingStart(textLayout), @@ -46,6 +49,10 @@ public View onCreateView(ViewGroup parent) { ApiCompatibilityUtils.getPaddingEnd(textLayout), 0); - return view; + return mView; + } + + public void announceForAccessibility(CharSequence announcement) { + if (mView != null) mView.announceForAccessibility(announcement); } } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/SpinnerPreference.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/SpinnerPreference.java index ff8b6e09fa491..1ad2a640fd975 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/SpinnerPreference.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/SpinnerPreference.java @@ -23,6 +23,7 @@ public class SpinnerPreference extends Preference { private Spinner mSpinner; private ArrayAdapter mAdapter; private int mSelectedIndex; + private View mView; /** * Constructor for inflating from XML. @@ -55,10 +56,11 @@ public Object getSelectedOption() { @Override public View onCreateView(ViewGroup parent) { - View view = super.onCreateView(parent); + if (mView != null) return mView; - ((TextView) view.findViewById(R.id.title)).setText(getTitle()); - mSpinner = (Spinner) view.findViewById(R.id.spinner); + mView = super.onCreateView(parent); + ((TextView) mView.findViewById(R.id.title)).setText(getTitle()); + mSpinner = (Spinner) mView.findViewById(R.id.spinner); mSpinner.setOnItemSelectedListener(new AdapterView.OnItemSelectedListener() { @Override public void onItemSelected( @@ -76,12 +78,19 @@ public void onNothingSelected(AdapterView parent) { } }); - return view; + return mView; } @Override protected void onBindView(View view) { - mSpinner.setAdapter(mAdapter); + super.onBindView(view); + + // Screen readers notice the setAdapter() call and announce it. We do not want the spinner + // to be announced every time the view is bound (e.g. when the user scrolls away from it + // and then back). Therefore, only update the adapter if it has actually changed. + if (mSpinner.getAdapter() != mAdapter) { + mSpinner.setAdapter(mAdapter); + } mSpinner.setSelection(mSelectedIndex); } } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java index 54e1914072ed7..ee0a7103b59cf 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferences.java @@ -7,7 +7,6 @@ import android.app.Activity; import android.app.ProgressDialog; import android.os.Bundle; -import android.preference.CheckBoxPreference; import android.preference.Preference; import android.preference.PreferenceFragment; import android.widget.ListView; @@ -16,6 +15,7 @@ import org.chromium.chrome.R; import org.chromium.chrome.browser.BrowsingDataType; import org.chromium.chrome.browser.preferences.ButtonPreference; +import org.chromium.chrome.browser.preferences.ClearBrowsingDataCheckBoxPreference; import org.chromium.chrome.browser.preferences.PrefServiceBridge; import org.chromium.chrome.browser.preferences.SpinnerPreference; import org.chromium.chrome.browser.preferences.privacy.BrowsingDataCounterBridge.BrowsingDataCounterCallback; @@ -39,19 +39,20 @@ private static class Item implements BrowsingDataCounterCallback, Preference.OnPreferenceClickListener { private final ClearBrowsingDataPreferences mParent; private final DialogOption mOption; - private final CheckBoxPreference mCheckbox; + private final ClearBrowsingDataCheckBoxPreference mCheckbox; private BrowsingDataCounterBridge mCounter; + private boolean mShouldAnnounceCounterResult; public Item(ClearBrowsingDataPreferences parent, DialogOption option, - CheckBoxPreference checkbox, + ClearBrowsingDataCheckBoxPreference checkbox, boolean selected, boolean enabled) { super(); mParent = parent; mOption = option; - mCounter = new BrowsingDataCounterBridge(this, mOption.getDataType()); mCheckbox = checkbox; + mCounter = new BrowsingDataCounterBridge(this, mOption.getDataType()); mCheckbox.setOnPreferenceClickListener(this); mCheckbox.setEnabled(enabled); @@ -76,6 +77,7 @@ public boolean onPreferenceClick(Preference preference) { assert mCheckbox == preference; mParent.updateButtonState(); + mShouldAnnounceCounterResult = true; PrefServiceBridge.getInstance().setBrowsingDataDeletionPreference( mOption.getDataType(), mCheckbox.isChecked()); return true; @@ -83,7 +85,19 @@ public boolean onPreferenceClick(Preference preference) { @Override public void onCounterFinished(String result) { - if (mCheckbox != null) mCheckbox.setSummaryOn(result); + mCheckbox.setSummaryOn(result); + if (mShouldAnnounceCounterResult) { + mCheckbox.announceForAccessibility(result); + } + } + + /** + * Sets whether the BrowsingDataCounter result should be announced. This is when the counter + * recalculation was caused by a checkbox state change (as opposed to fragment + * initialization or time period change). + */ + public void setShouldAnnounceCounterResult(boolean value) { + mShouldAnnounceCounterResult = value; } } @@ -272,6 +286,12 @@ public boolean onPreferenceClick(Preference preference) { @Override public boolean onPreferenceChange(Preference preference, Object value) { if (preference.getKey().equals(PREF_TIME_RANGE)) { + // Inform the items that a recalculation is going to happen as a result of the time + // period change. + for (Item item : mItems) { + item.setShouldAnnounceCounterResult(false); + } + PrefServiceBridge.getInstance().setBrowsingDataDeletionTimePeriod( ((TimePeriodSpinnerOption) value).getTimePeriod()); return true; @@ -310,7 +330,7 @@ public void onCreate(Bundle savedInstanceState) { mItems[i] = new Item( this, options[i], - (CheckBoxPreference) findPreference(options[i].getPreferenceKey()), + (ClearBrowsingDataCheckBoxPreference) findPreference(options[i].getPreferenceKey()), isOptionSelectedByDefault(options[i]), enabled); }