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: Ic79eaa2e26bec9cd8d90fdab434271bc4f3d8a68
diff --git a/core/java/android/widget/AbsListView.java b/core/java/android/widget/AbsListView.java
index 389fc0a..5724f52 100644
--- a/core/java/android/widget/AbsListView.java
+++ b/core/java/android/widget/AbsListView.java
@@ -2398,6 +2398,7 @@
lp.itemId = mAdapter.getItemId(position);
}
lp.viewType = mAdapter.getItemViewType(position);
+ lp.isEnabled = mAdapter.isEnabled(position);
if (lp != vlp) {
child.setLayoutParams(lp);
}
@@ -2419,19 +2420,33 @@
}
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: {
@@ -2447,12 +2462,14 @@
}
} return false;
case AccessibilityNodeInfo.ACTION_CLICK: {
- if (isItemClickable(host, position)) {
+ if (isItemClickable(host)) {
+ 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;
@@ -2472,13 +2489,20 @@
*/
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;
}
@@ -2490,7 +2514,7 @@
info.addAction(AccessibilityAction.ACTION_SELECT);
}
- if (isItemClickable(view, position)) {
+ if (isItemClickable(view)) {
info.addAction(AccessibilityAction.ACTION_CLICK);
info.setClickable(true);
}
@@ -2501,9 +2525,8 @@
}
}
- private boolean isItemClickable(View view, int position) {
- return mAdapter != null && view != null &&
- mAdapter.isEnabled(position) && !view.hasFocusable();
+ private boolean isItemClickable(View view) {
+ return !view.hasFocusable();
}
/**
@@ -6326,6 +6349,9 @@
*/
long itemId = -1;
+ /** Whether the adapter considers the item enabled. */
+ boolean isEnabled;
+
public LayoutParams(Context c, AttributeSet attrs) {
super(c, attrs);
}
@@ -6351,6 +6377,7 @@
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 @@
}
/**
- * 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 @@
child.setLayoutParams(p);
}
p.viewType = mAdapter.getItemViewType(0);
+ p.isEnabled = mAdapter.isEnabled(0);
p.forceAdd = true;
int childHeightSpec = getChildMeasureSpec(
@@ -1480,6 +1481,7 @@
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 be83974..53ca6d1 100644
--- a/core/java/android/widget/ListView.java
+++ b/core/java/android/widget/ListView.java
@@ -1200,6 +1200,7 @@
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 @@
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)) {