diff options
author | Alan Viverette <alanv@google.com> | 2015-09-10 11:10:41 -0400 |
---|---|---|
committer | Alan Viverette <alanv@google.com> | 2015-09-10 15:17:11 +0000 |
commit | a84dda54e408ce23ae0a77e35bf78c22ddd81351 (patch) | |
tree | 74e7b0f0e0307f85989adbdb61ac6e02b689745a | |
parent | 0018323c07d119fde7260d66f60dd9987c7b74ea (diff) | |
download | frameworks_base-a84dda54e408ce23ae0a77e35bf78c22ddd81351.zip frameworks_base-a84dda54e408ce23ae0a77e35bf78c22ddd81351.tar.gz frameworks_base-a84dda54e408ce23ae0a77e35bf78c22ddd81351.tar.bz2 |
Avoid OOBE when AbsListView layout is out of sync with adapter
Views are not synchronized with adapter state until a layout pass
occurs, which may cause an OOBE if a list item is removed and an
accessibility node is obtained before the next layout pass.
This CL caches the item's enabled state on the bound view's layout
params, which allows us to avoid relying on the adapter to populate
accessibility nodes. It also aborts actions if the target position
is no longer valid.
Updates the documentation on AdapterView to reflect that the result
of getPositionForView() may not be synchronized with the adapter.
Bug: 23943664
Change-Id: I90ca946bead1d87a179eab1c2f482a54bcce9c38
-rw-r--r-- | core/java/android/widget/AbsListView.java | 46 | ||||
-rw-r--r-- | core/java/android/widget/AdapterView.java | 19 | ||||
-rw-r--r-- | core/java/android/widget/GridView.java | 2 | ||||
-rw-r--r-- | core/java/android/widget/ListView.java | 2 |
4 files changed, 54 insertions, 15 deletions
diff --git a/core/java/android/widget/AbsListView.java b/core/java/android/widget/AbsListView.java index ed858e7..6e9a418 100644 --- a/core/java/android/widget/AbsListView.java +++ b/core/java/android/widget/AbsListView.java @@ -2395,6 +2395,7 @@ public abstract class AbsListView extends AdapterView<ListAdapter> implements Te lp.itemId = mAdapter.getItemId(position); } lp.viewType = mAdapter.getItemViewType(position); + lp.isEnabled = mAdapter.isEnabled(position); if (lp != vlp) { child.setLayoutParams(lp); } @@ -2416,19 +2417,33 @@ public abstract class AbsListView extends AdapterView<ListAdapter> implements Te } final int position = getPositionForView(host); - final ListAdapter adapter = getAdapter(); - - if ((position == INVALID_POSITION) || (adapter == null)) { + if (position == INVALID_POSITION || mAdapter == null) { // Cannot perform actions on invalid items. return false; } - if (!isEnabled() || !adapter.isEnabled(position)) { - // Cannot perform actions on disabled items. + if (position >= mAdapter.getCount()) { + // The position is no longer valid, likely due to a data set + // change. We could fail here for all data set changes, since + // there is a chance that the data bound to the view may no + // longer exist at the same position within the adapter, but + // it's more consistent with the standard touch interaction to + // click at whatever may have moved into that position. return false; } - final long id = getItemIdAtPosition(position); + final boolean isItemEnabled; + final ViewGroup.LayoutParams lp = host.getLayoutParams(); + if (lp instanceof AbsListView.LayoutParams) { + isItemEnabled = ((AbsListView.LayoutParams) lp).isEnabled; + } else { + isItemEnabled = false; + } + + if (!isEnabled() || !isItemEnabled) { + // Cannot perform actions on disabled items. + return false; + } switch (action) { case AccessibilityNodeInfo.ACTION_CLEAR_SELECTION: { @@ -2445,11 +2460,13 @@ public abstract class AbsListView extends AdapterView<ListAdapter> implements Te } return false; case AccessibilityNodeInfo.ACTION_CLICK: { if (isClickable()) { + final long id = getItemIdAtPosition(position); return performItemClick(host, position, id); } } return false; case AccessibilityNodeInfo.ACTION_LONG_CLICK: { if (isLongClickable()) { + final long id = getItemIdAtPosition(position); return performLongPress(host, position, id); } } return false; @@ -2469,13 +2486,20 @@ public abstract class AbsListView extends AdapterView<ListAdapter> implements Te */ public void onInitializeAccessibilityNodeInfoForItem( View view, int position, AccessibilityNodeInfo info) { - final ListAdapter adapter = getAdapter(); - if (position == INVALID_POSITION || adapter == null) { + if (position == INVALID_POSITION) { // The item doesn't exist, so there's not much we can do here. return; } - if (!isEnabled() || !adapter.isEnabled(position)) { + final boolean isItemEnabled; + final ViewGroup.LayoutParams lp = view.getLayoutParams(); + if (lp instanceof AbsListView.LayoutParams) { + isItemEnabled = ((AbsListView.LayoutParams) lp).isEnabled; + } else { + isItemEnabled = false; + } + + if (!isEnabled() || !isItemEnabled) { info.setEnabled(false); return; } @@ -6315,6 +6339,9 @@ public abstract class AbsListView extends AdapterView<ListAdapter> implements Te */ long itemId = -1; + /** Whether the adapter considers the item enabled. */ + boolean isEnabled; + public LayoutParams(Context c, AttributeSet attrs) { super(c, attrs); } @@ -6340,6 +6367,7 @@ public abstract class AbsListView extends AdapterView<ListAdapter> implements Te encoder.addProperty("list:viewType", viewType); encoder.addProperty("list:recycledHeaderFooter", recycledHeaderFooter); encoder.addProperty("list:forceAdd", forceAdd); + encoder.addProperty("list:isEnabled", isEnabled); } } diff --git a/core/java/android/widget/AdapterView.java b/core/java/android/widget/AdapterView.java index 0cc1b25..2cfefba 100644 --- a/core/java/android/widget/AdapterView.java +++ b/core/java/android/widget/AdapterView.java @@ -600,13 +600,20 @@ public abstract class AdapterView<T extends Adapter> extends ViewGroup { } /** - * Get the position within the adapter's data set for the view, where view is a an adapter item - * or a descendant of an adapter item. + * Returns the position within the adapter's data set for the view, where + * view is a an adapter item or a descendant of an adapter item. + * <p> + * <strong>Note:</strong> The result of this method only reflects the + * position of the data bound to <var>view</var> during the most recent + * layout pass. If the adapter's data set has changed without a subsequent + * layout pass, the position returned by this method may not match the + * current position of the data within the adapter. * - * @param view an adapter item, or a descendant of an adapter item. This must be visible in this - * AdapterView at the time of the call. - * @return the position within the adapter's data set of the view, or {@link #INVALID_POSITION} - * if the view does not correspond to a list item (or it is not currently visible). + * @param view an adapter item, or a descendant of an adapter item. This + * must be visible in this AdapterView at the time of the call. + * @return the position within the adapter's data set of the view, or + * {@link #INVALID_POSITION} if the view does not correspond to a + * list item (or it is not currently visible) */ public int getPositionForView(View view) { View listItem = view; diff --git a/core/java/android/widget/GridView.java b/core/java/android/widget/GridView.java index f994d4a..607e955 100644 --- a/core/java/android/widget/GridView.java +++ b/core/java/android/widget/GridView.java @@ -1070,6 +1070,7 @@ public class GridView extends AbsListView { child.setLayoutParams(p); } p.viewType = mAdapter.getItemViewType(0); + p.isEnabled = mAdapter.isEnabled(0); p.forceAdd = true; int childHeightSpec = getChildMeasureSpec( @@ -1480,6 +1481,7 @@ public class GridView extends AbsListView { p = (AbsListView.LayoutParams) generateDefaultLayoutParams(); } p.viewType = mAdapter.getItemViewType(position); + p.isEnabled = mAdapter.isEnabled(position); if (recycled && !p.forceAdd) { attachViewToParent(child, where, p); diff --git a/core/java/android/widget/ListView.java b/core/java/android/widget/ListView.java index c5632ec..00d017f 100644 --- a/core/java/android/widget/ListView.java +++ b/core/java/android/widget/ListView.java @@ -1200,6 +1200,7 @@ public class ListView extends AbsListView { child.setLayoutParams(p); } p.viewType = mAdapter.getItemViewType(position); + p.isEnabled = mAdapter.isEnabled(position); p.forceAdd = true; final int childWidthSpec = ViewGroup.getChildMeasureSpec(widthMeasureSpec, @@ -1913,6 +1914,7 @@ public class ListView extends AbsListView { p = (AbsListView.LayoutParams) generateDefaultLayoutParams(); } p.viewType = mAdapter.getItemViewType(position); + p.isEnabled = mAdapter.isEnabled(position); if ((recycled && !p.forceAdd) || (p.recycledHeaderFooter && p.viewType == AdapterView.ITEM_VIEW_TYPE_HEADER_OR_FOOTER)) { |