Skip to content

Commit

Permalink
Improve the accessibility of ClearBrowsingDataPreferences
Browse files Browse the repository at this point in the history
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}
  • Loading branch information
msramek authored and Commit bot committed Mar 8, 2016
1 parent 3c7b134 commit fd1c745
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -27,25 +28,31 @@ 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),
0,
ApiCompatibilityUtils.getPaddingEnd(textLayout),
0);

return view;
return mView;
}

public void announceForAccessibility(CharSequence announcement) {
if (mView != null) mView.announceForAccessibility(announcement);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public class SpinnerPreference extends Preference {
private Spinner mSpinner;
private ArrayAdapter<Object> mAdapter;
private int mSelectedIndex;
private View mView;

/**
* Constructor for inflating from XML.
Expand Down Expand Up @@ -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(
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -76,14 +77,27 @@ public boolean onPreferenceClick(Preference preference) {
assert mCheckbox == preference;

mParent.updateButtonState();
mShouldAnnounceCounterResult = true;
PrefServiceBridge.getInstance().setBrowsingDataDeletionPreference(
mOption.getDataType(), mCheckbox.isChecked());
return true;
}

@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;
}
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down

0 comments on commit fd1c745

Please sign in to comment.