From 3e14162fc655e7a0dae61318911f3e29c07d0bf0 Mon Sep 17 00:00:00 2001 From: Alan Viverette Date: Tue, 18 Feb 2014 17:05:13 -0800 Subject: Fix transient state, accessibility focus in ListView, GridView Reverts portions of "Use transient state to preserve accessibility focus in ListView," "Set transient state for focus container in ListView," and "Restore A11y and keyboard focus positions after GridView layout." Basically, using transient state here was bad and I should feel bad. Replaces reverted code with better handling of accessibility properties for scrapped views that are reused for the same stable ID. BUG: 12701797 Change-Id: I7afcb2fe14413828435d1476b4f0f40108edd2dd --- core/java/android/widget/AbsListView.java | 98 ++++++++++---------- core/java/android/widget/GridView.java | 71 +++++++++------ core/java/android/widget/ListView.java | 144 +++++++++++++++++++++++------- 3 files changed, 206 insertions(+), 107 deletions(-) diff --git a/core/java/android/widget/AbsListView.java b/core/java/android/widget/AbsListView.java index 63ce5a389490..ace9dda19408 100644 --- a/core/java/android/widget/AbsListView.java +++ b/core/java/android/widget/AbsListView.java @@ -2169,20 +2169,11 @@ public abstract class AbsListView extends AdapterView implements Te } /** - * @return the direct child that contains accessibility focus, or null if no + * @param focusedView view that holds accessibility focus + * @return direct child that contains accessibility focus, or null if no * child contains accessibility focus */ - View getAccessibilityFocusedChild() { - final ViewRootImpl viewRootImpl = getViewRootImpl(); - if (viewRootImpl == null) { - return null; - } - - View focusedView = viewRootImpl.getAccessibilityFocusedHost(); - if (focusedView == null) { - return null; - } - + View getAccessibilityFocusedChild(View focusedView) { ViewParent viewParent = focusedView.getParent(); while ((viewParent instanceof View) && (viewParent != this)) { focusedView = (View) viewParent; @@ -2329,12 +2320,6 @@ public abstract class AbsListView extends AdapterView implements Te } else { isScrap[0] = true; - // Clear any system-managed transient state so that we can - // recycle this view and bind it to different data. - if (child.isAccessibilityFocused()) { - child.clearAccessibilityFocus(); - } - child.dispatchFinishTemporaryDetach(); } } @@ -6190,18 +6175,12 @@ public abstract class AbsListView extends AdapterView implements Te void clear() { if (mViewTypeCount == 1) { final ArrayList scrap = mCurrentScrap; - final int scrapCount = scrap.size(); - for (int i = 0; i < scrapCount; i++) { - removeDetachedView(scrap.remove(scrapCount - 1 - i), false); - } + clearScrap(scrap); } else { final int typeCount = mViewTypeCount; for (int i = 0; i < typeCount; i++) { final ArrayList scrap = mScrapViews[i]; - final int scrapCount = scrap.size(); - for (int j = 0; j < scrapCount; j++) { - removeDetachedView(scrap.remove(scrapCount - 1 - j), false); - } + clearScrap(scrap); } } @@ -6302,7 +6281,7 @@ public abstract class AbsListView extends AdapterView implements Te if (mViewTypeCount == 1) { return retrieveFromScrap(mCurrentScrap, position); } else { - int whichScrap = mAdapter.getItemViewType(position); + final int whichScrap = mAdapter.getItemViewType(position); if (whichScrap >= 0 && whichScrap < mScrapViews.length) { return retrieveFromScrap(mScrapViews[whichScrap], position); } @@ -6374,13 +6353,6 @@ public abstract class AbsListView extends AdapterView implements Te mScrapViews[viewType].add(scrap); } - // Clear any system-managed transient state. - if (scrap.isAccessibilityFocused()) { - scrap.clearAccessibilityFocus(); - } - - scrap.setAccessibilityDelegate(null); - if (mRecyclerListener != null) { mRecyclerListener.onMovedToScrapHeap(scrap); } @@ -6454,7 +6426,6 @@ public abstract class AbsListView extends AdapterView implements Te lp.scrappedFromPosition = mFirstActivePosition + i; scrapViews.add(victim); - victim.setAccessibilityDelegate(null); if (hasListener) { mRecyclerListener.onMovedToScrapHeap(victim); } @@ -6558,23 +6529,52 @@ public abstract class AbsListView extends AdapterView implements Te } } } - } - static View retrieveFromScrap(ArrayList scrapViews, int position) { - int size = scrapViews.size(); - if (size > 0) { - // See if we still have a view for this position. - for (int i=0; i scrapViews, int position) { + final int size = scrapViews.size(); + if (size > 0) { + // See if we still have a view for this position or ID. + for (int i = 0; i < size; i++) { + final View view = scrapViews.get(i); + final AbsListView.LayoutParams params = + (AbsListView.LayoutParams) view.getLayoutParams(); + + if (mAdapterHasStableIds) { + final long id = mAdapter.getItemId(position); + if (id == params.itemId) { + return scrapViews.remove(i); + } + } else if (params.scrappedFromPosition == position) { + final View scrap = scrapViews.remove(i); + clearAccessibilityFromScrap(scrap); + return scrap; + } } + final View scrap = scrapViews.remove(size - 1); + clearAccessibilityFromScrap(scrap); + return scrap; + } else { + return null; } - return scrapViews.remove(size - 1); - } else { - return null; + } + + private void clearScrap(final ArrayList scrap) { + final int scrapCount = scrap.size(); + for (int j = 0; j < scrapCount; j++) { + removeDetachedView(scrap.remove(scrapCount - 1 - j), false); + } + } + + private void clearAccessibilityFromScrap(View view) { + if (view.isAccessibilityFocused()) { + view.clearAccessibilityFocus(); + } + view.setAccessibilityDelegate(null); + } + + private void removeDetachedView(View child, boolean animate) { + child.setAccessibilityDelegate(null); + AbsListView.this.removeDetachedView(child, animate); } } diff --git a/core/java/android/widget/GridView.java b/core/java/android/widget/GridView.java index 67430028cbbb..4ed48ff08f90 100644 --- a/core/java/android/widget/GridView.java +++ b/core/java/android/widget/GridView.java @@ -30,13 +30,13 @@ import android.view.SoundEffectConstants; import android.view.View; import android.view.ViewDebug; import android.view.ViewGroup; +import android.view.ViewRootImpl; import android.view.accessibility.AccessibilityEvent; import android.view.accessibility.AccessibilityNodeInfo; +import android.view.accessibility.AccessibilityNodeProvider; import android.view.accessibility.AccessibilityNodeInfo.CollectionInfo; import android.view.accessibility.AccessibilityNodeInfo.CollectionItemInfo; import android.view.animation.GridLayoutAnimationController; -import android.widget.AbsListView.AbsPositionScroller; -import android.widget.ListView.ListViewPositionScroller; import android.widget.RemoteViews.RemoteView; import java.lang.annotation.Retention; @@ -1222,22 +1222,32 @@ public class GridView extends AbsListView { setSelectedPositionInt(mNextSelectedPosition); - // Remember which child, if any, had accessibility focus. - final int accessibilityFocusPosition; - final View accessFocusedChild = getAccessibilityFocusedChild(); - if (accessFocusedChild != null) { - accessibilityFocusPosition = getPositionForView(accessFocusedChild); - accessFocusedChild.setHasTransientState(true); - } else { - accessibilityFocusPosition = INVALID_POSITION; - } + AccessibilityNodeInfo accessibilityFocusLayoutRestoreNode = null; + View accessibilityFocusLayoutRestoreView = null; + int accessibilityFocusPosition = INVALID_POSITION; + + // Remember which child, if any, had accessibility focus. This must + // occur before recycling any views, since that will clear + // accessibility focus. + final ViewRootImpl viewRootImpl = getViewRootImpl(); + if (viewRootImpl != null) { + final View focusHost = viewRootImpl.getAccessibilityFocusedHost(); + if (focusHost != null) { + final View focusChild = getAccessibilityFocusedChild(focusHost); + if (focusChild != null) { + if (!dataChanged || focusChild.hasTransientState() + || mAdapterHasStableIds) { + // The views won't be changing, so try to maintain + // focus on the current host and virtual view. + accessibilityFocusLayoutRestoreView = focusHost; + accessibilityFocusLayoutRestoreNode = viewRootImpl + .getAccessibilityFocusedVirtualView(); + } - // Ensure the child containing focus, if any, has transient state. - // If the list data hasn't changed, or if the adapter has stable - // IDs, this will maintain focus. - final View focusedChild = getFocusedChild(); - if (focusedChild != null) { - focusedChild.setHasTransientState(true); + // Try to maintain focus at the same position. + accessibilityFocusPosition = getPositionForView(focusChild); + } + } } // Pull all children into the RecycleBin. @@ -1324,13 +1334,22 @@ public class GridView extends AbsListView { mSelectorRect.setEmpty(); } - if (accessFocusedChild != null) { - accessFocusedChild.setHasTransientState(false); - - // If we failed to maintain accessibility focus on the previous - // view, attempt to restore it to the previous position. - if (!accessFocusedChild.isAccessibilityFocused() - && accessibilityFocusPosition != INVALID_POSITION) { + // Attempt to restore accessibility focus, if necessary. + final View newAccessibilityFocusedView = viewRootImpl.getAccessibilityFocusedHost(); + if (newAccessibilityFocusedView == null) { + if (accessibilityFocusLayoutRestoreView != null + && accessibilityFocusLayoutRestoreView.isAttachedToWindow()) { + final AccessibilityNodeProvider provider = + accessibilityFocusLayoutRestoreView.getAccessibilityNodeProvider(); + if (accessibilityFocusLayoutRestoreNode != null && provider != null) { + final int virtualViewId = AccessibilityNodeInfo.getVirtualDescendantId( + accessibilityFocusLayoutRestoreNode.getSourceNodeId()); + provider.performAction(virtualViewId, + AccessibilityNodeInfo.ACTION_ACCESSIBILITY_FOCUS, null); + } else { + accessibilityFocusLayoutRestoreView.requestAccessibilityFocus(); + } + } else if (accessibilityFocusPosition != INVALID_POSITION) { // Bound the position within the visible children. final int position = MathUtils.constrain( accessibilityFocusPosition - mFirstPosition, 0, getChildCount() - 1); @@ -1341,10 +1360,6 @@ public class GridView extends AbsListView { } } - if (focusedChild != null) { - focusedChild.setHasTransientState(false); - } - mLayoutMode = LAYOUT_NORMAL; mDataChanged = false; if (mPositionScrollAfterLayout != null) { diff --git a/core/java/android/widget/ListView.java b/core/java/android/widget/ListView.java index cef2138a53b1..63e13589b3b8 100644 --- a/core/java/android/widget/ListView.java +++ b/core/java/android/widget/ListView.java @@ -39,10 +39,12 @@ import android.view.View; import android.view.ViewDebug; import android.view.ViewGroup; import android.view.ViewParent; +import android.view.ViewRootImpl; import android.view.accessibility.AccessibilityEvent; import android.view.accessibility.AccessibilityNodeInfo; import android.view.accessibility.AccessibilityNodeInfo.CollectionInfo; import android.view.accessibility.AccessibilityNodeInfo.CollectionItemInfo; +import android.view.accessibility.AccessibilityNodeProvider; import android.widget.RemoteViews.RemoteView; import java.util.ArrayList; @@ -1567,22 +1569,58 @@ public class ListView extends AbsListView { setSelectedPositionInt(mNextSelectedPosition); - // Remember which child, if any, had accessibility focus. - final int accessibilityFocusPosition; - final View accessFocusedChild = getAccessibilityFocusedChild(); - if (accessFocusedChild != null) { - accessibilityFocusPosition = getPositionForView(accessFocusedChild); - accessFocusedChild.setHasTransientState(true); - } else { - accessibilityFocusPosition = INVALID_POSITION; + AccessibilityNodeInfo accessibilityFocusLayoutRestoreNode = null; + View accessibilityFocusLayoutRestoreView = null; + int accessibilityFocusPosition = INVALID_POSITION; + + // Remember which child, if any, had accessibility focus. This must + // occur before recycling any views, since that will clear + // accessibility focus. + final ViewRootImpl viewRootImpl = getViewRootImpl(); + if (viewRootImpl != null) { + final View focusHost = viewRootImpl.getAccessibilityFocusedHost(); + if (focusHost != null) { + final View focusChild = getAccessibilityFocusedChild(focusHost); + if (focusChild != null) { + if (!dataChanged || isDirectChildHeaderOrFooter(focusChild) + || focusChild.hasTransientState() || mAdapterHasStableIds) { + // The views won't be changing, so try to maintain + // focus on the current host and virtual view. + accessibilityFocusLayoutRestoreView = focusHost; + accessibilityFocusLayoutRestoreNode = viewRootImpl + .getAccessibilityFocusedVirtualView(); + } + + // If all else fails, maintain focus at the same + // position. + accessibilityFocusPosition = getPositionForView(focusChild); + } + } } - // Ensure the child containing focus, if any, has transient state. - // If the list data hasn't changed, or if the adapter has stable - // IDs, this will maintain focus. + View focusLayoutRestoreDirectChild = null; + View focusLayoutRestoreView = null; + + // Take focus back to us temporarily to avoid the eventual call to + // clear focus when removing the focused child below from messing + // things up when ViewAncestor assigns focus back to someone else. final View focusedChild = getFocusedChild(); if (focusedChild != null) { - focusedChild.setHasTransientState(true); + // TODO: in some cases focusedChild.getParent() == null + + // We can remember the focused view to restore after re-layout + // if the data hasn't changed, or if the focused position is a + // header or footer. + if (!dataChanged || isDirectChildHeaderOrFooter(focusedChild)) { + focusLayoutRestoreDirectChild = focusedChild; + // Remember the specific view that had focus. + focusLayoutRestoreView = findFocus(); + if (focusLayoutRestoreView != null) { + // Tell it we are going to mess with it. + focusLayoutRestoreView.onStartTemporaryDetach(); + } + } + requestFocus(); } // Pull all children into the RecycleBin. @@ -1656,20 +1694,24 @@ public class ListView extends AbsListView { recycleBin.scrapActiveViews(); if (sel != null) { - final boolean shouldPlaceFocus = mItemsCanFocus && hasFocus(); - final boolean maintainedFocus = focusedChild != null && focusedChild.hasFocus(); - if (shouldPlaceFocus && !maintainedFocus && !sel.hasFocus()) { - if (sel.requestFocus()) { - // Successfully placed focus, clear selection. - sel.setSelected(false); - mSelectorRect.setEmpty(); - } else { - // Failed to place focus, clear current (invalid) focus. + // The current selected item should get focus if items are + // focusable. + if (mItemsCanFocus && hasFocus() && !sel.hasFocus()) { + final boolean focusWasTaken = (sel == focusLayoutRestoreDirectChild && + focusLayoutRestoreView != null && + focusLayoutRestoreView.requestFocus()) || sel.requestFocus(); + if (!focusWasTaken) { + // Selected item didn't take focus, but we still want to + // make sure something else outside of the selected view + // has focus. final View focused = getFocusedChild(); if (focused != null) { focused.clearFocus(); } positionSelector(INVALID_POSITION, sel); + } else { + sel.setSelected(false); + mSelectorRect.setEmpty(); } } else { positionSelector(INVALID_POSITION, sel); @@ -1687,15 +1729,30 @@ public class ListView extends AbsListView { mSelectedTop = 0; mSelectorRect.setEmpty(); } - } - if (accessFocusedChild != null) { - accessFocusedChild.setHasTransientState(false); + // Even if there is not selected position, we may need to + // restore focus (i.e. something focusable in touch mode). + if (hasFocus() && focusLayoutRestoreView != null) { + focusLayoutRestoreView.requestFocus(); + } + } - // If we failed to maintain accessibility focus on the previous - // view, attempt to restore it to the previous position. - if (!accessFocusedChild.isAccessibilityFocused() - && accessibilityFocusPosition != INVALID_POSITION) { + // Attempt to restore accessibility focus, if necessary. + final View newAccessibilityFocusedView = viewRootImpl.getAccessibilityFocusedHost(); + if (newAccessibilityFocusedView == null) { + if (accessibilityFocusLayoutRestoreView != null + && accessibilityFocusLayoutRestoreView.isAttachedToWindow()) { + final AccessibilityNodeProvider provider = + accessibilityFocusLayoutRestoreView.getAccessibilityNodeProvider(); + if (accessibilityFocusLayoutRestoreNode != null && provider != null) { + final int virtualViewId = AccessibilityNodeInfo.getVirtualDescendantId( + accessibilityFocusLayoutRestoreNode.getSourceNodeId()); + provider.performAction(virtualViewId, + AccessibilityNodeInfo.ACTION_ACCESSIBILITY_FOCUS, null); + } else { + accessibilityFocusLayoutRestoreView.requestAccessibilityFocus(); + } + } else if (accessibilityFocusPosition != INVALID_POSITION) { // Bound the position within the visible children. final int position = MathUtils.constrain( accessibilityFocusPosition - mFirstPosition, 0, getChildCount() - 1); @@ -1706,8 +1763,11 @@ public class ListView extends AbsListView { } } - if (focusedChild != null) { - focusedChild.setHasTransientState(false); + // Tell focus view we are done mucking with it, if it is still in + // our view hierarchy. + if (focusLayoutRestoreView != null + && focusLayoutRestoreView.getWindowToken() != null) { + focusLayoutRestoreView.onFinishTemporaryDetach(); } mLayoutMode = LAYOUT_NORMAL; @@ -1733,6 +1793,30 @@ public class ListView extends AbsListView { } } + /** + * @param child a direct child of this list. + * @return Whether child is a header or footer view. + */ + private boolean isDirectChildHeaderOrFooter(View child) { + final ArrayList headers = mHeaderViewInfos; + final int numHeaders = headers.size(); + for (int i = 0; i < numHeaders; i++) { + if (child == headers.get(i).view) { + return true; + } + } + + final ArrayList footers = mFooterViewInfos; + final int numFooters = footers.size(); + for (int i = 0; i < numFooters; i++) { + if (child == footers.get(i).view) { + return true; + } + } + + return false; + } + /** * Obtain the view and add it to our list of children. The view can be made * fresh, converted from an unused view, or used as is if it was in the -- cgit v1.2.3-59-g8ed1b