Default focus order now tries to adhere to view hierarchy

Specifically, tab focus should travel completely through all
views within a given viewgroup before traveling to a sibling
viewgroup. At each hierarchy level, viewgroups are sorted
according to the existing rectangle-based algorithm.

This is only the first of a couple focus-order tweaks to
adhere to the new keyboard navigation ux spec.

Bug: 33848452
Bug: 34084573
Test: Checked in a sample-app that focus order adheres much
      more closely to our keyboard navigation spec. Also
      verified that existing FocusFinder CTS tests still pass.
Change-Id: Ia0e59a2e0dd8b9773796e0aaebfca0deb3c14a3c

diff --git a/core/java/android/view/FocusFinder.java b/core/java/android/view/FocusFinder.java
index 41a13cf..7fde8a6 100644 (file)
--- a/core/java/android/view/FocusFinder.java
+++ b/core/java/android/view/FocusFinder.java
@@ -52,7 +52,9 @@ public class FocusFinder {
     final Rect mFocusedRect = new Rect();
     final Rect mOtherRect = new Rect();
     final Rect mBestCandidateRect = new Rect();
-    final SequentialFocusComparator mSequentialFocusComparator = new SequentialFocusComparator();
+    private final UserSpecifiedFocusComparator mUserSpecifiedFocusComparator =
+            new UserSpecifiedFocusComparator();
+    private final FocusComparator mFocusComparator = new FocusComparator();
 
     private final ArrayList<View> mTempList = new ArrayList<View>();
 
@@ -225,12 +227,10 @@ public class FocusFinder {
             View focused, Rect focusedRect, int direction) {
         try {
             // Note: This sort is stable.
-            mSequentialFocusComparator.setRoot(root);
-            mSequentialFocusComparator.setIsLayoutRtl(root.isLayoutRtl());
-            mSequentialFocusComparator.setFocusables(focusables);
-            Collections.sort(focusables, mSequentialFocusComparator);
+            mUserSpecifiedFocusComparator.setFocusables(focusables);
+            Collections.sort(focusables, mUserSpecifiedFocusComparator);
         } finally {
-            mSequentialFocusComparator.recycle();
+            mUserSpecifiedFocusComparator.recycle();
         }
 
         final int count = focusables.size();
@@ -703,36 +703,80 @@ public class FocusFinder {
         return id != 0 && id != View.NO_ID;
     }
 
-    /**
-     * Sorts views according to their visual layout and geometry for default tab order.
-     * If views are part of a focus chain (nextFocusForwardId), then they are all grouped
-     * together. The head of the chain is used to determine the order of the chain and is
-     * first in the order and the tail of the chain is the last in the order. The views
-     * in the middle of the chain can be arbitrary order.
-     * This is used for sequential focus traversal.
-     */
-    private static final class SequentialFocusComparator implements Comparator<View> {
+    static FocusComparator getFocusComparator(ViewGroup root, boolean isRtl) {
+        FocusComparator comparator = getInstance().mFocusComparator;
+        comparator.setRoot(root);
+        comparator.setIsLayoutRtl(isRtl);
+        return comparator;
+    }
+
+    static final class FocusComparator implements Comparator<View> {
         private final Rect mFirstRect = new Rect();
         private final Rect mSecondRect = new Rect();
-        private ViewGroup mRoot;
+        private ViewGroup mRoot = null;
         private boolean mIsLayoutRtl;
+
+        public void setIsLayoutRtl(boolean b) {
+            mIsLayoutRtl = b;
+        }
+
+        public void setRoot(ViewGroup root) {
+            mRoot = root;
+        }
+
+        public int compare(View first, View second) {
+            if (first == second) {
+                return 0;
+            }
+
+            getRect(first, mFirstRect);
+            getRect(second, mSecondRect);
+
+            if (mFirstRect.top < mSecondRect.top) {
+                return -1;
+            } else if (mFirstRect.top > mSecondRect.top) {
+                return 1;
+            } else if (mFirstRect.left < mSecondRect.left) {
+                return mIsLayoutRtl ? 1 : -1;
+            } else if (mFirstRect.left > mSecondRect.left) {
+                return mIsLayoutRtl ? -1 : 1;
+            } else if (mFirstRect.bottom < mSecondRect.bottom) {
+                return -1;
+            } else if (mFirstRect.bottom > mSecondRect.bottom) {
+                return 1;
+            } else if (mFirstRect.right < mSecondRect.right) {
+                return mIsLayoutRtl ? 1 : -1;
+            } else if (mFirstRect.right > mSecondRect.right) {
+                return mIsLayoutRtl ? -1 : 1;
+            } else {
+                // The view are distinct but completely coincident so we consider
+                // them equal for our purposes.  Since the sort is stable, this
+                // means that the views will retain their layout order relative to one another.
+                return 0;
+            }
+        }
+
+        private void getRect(View view, Rect rect) {
+            view.getDrawingRect(rect);
+            mRoot.offsetDescendantRectToMyCoords(view, rect);
+        }
+    }
+
+    /**
+     * Sorts views according to any explicitly-specified focus-chains. If there are no explicitly
+     * specified focus chains (eg. no nextFocusForward attributes defined), this should be a no-op.
+     */
+    private static final class UserSpecifiedFocusComparator implements Comparator<View> {
         private final SparseArray<View> mFocusables = new SparseArray<View>();
         private final SparseBooleanArray mIsConnectedTo = new SparseBooleanArray();
         private final ArrayMap<View, View> mHeadsOfChains = new ArrayMap<View, View>();
+        private final ArrayMap<View, Integer> mOriginalOrdinal = new ArrayMap<>();
 
         public void recycle() {
-            mRoot = null;
             mFocusables.clear();
             mHeadsOfChains.clear();
             mIsConnectedTo.clear();
-        }
-
-        public void setRoot(ViewGroup root) {
-            mRoot = root;
-        }
-
-        public void setIsLayoutRtl(boolean b) {
-            mIsLayoutRtl = b;
+            mOriginalOrdinal.clear();
         }
 
         public void setFocusables(ArrayList<View> focusables) {
@@ -755,6 +799,10 @@ public class FocusFinder {
                     setHeadOfChain(view);
                 }
             }
+
+            for (int i = 0; i < focusables.size(); ++i) {
+                mOriginalOrdinal.put(focusables.get(i), i);
+            }
         }
 
         private void setHeadOfChain(View head) {
@@ -793,44 +841,22 @@ public class FocusFinder {
                     return 1; // first is end of chain
                 }
             }
+            boolean involvesChain = false;
             if (firstHead != null) {
                 first = firstHead;
+                involvesChain = true;
             }
             if (secondHead != null) {
                 second = secondHead;
+                involvesChain = true;
             }
 
-            // First see if they belong to the same focus chain.
-            getRect(first, mFirstRect);
-            getRect(second, mSecondRect);
-
-            if (mFirstRect.top < mSecondRect.top) {
-                return -1;
-            } else if (mFirstRect.top > mSecondRect.top) {
-                return 1;
-            } else if (mFirstRect.left < mSecondRect.left) {
-                return mIsLayoutRtl ? 1 : -1;
-            } else if (mFirstRect.left > mSecondRect.left) {
-                return mIsLayoutRtl ? -1 : 1;
-            } else if (mFirstRect.bottom < mSecondRect.bottom) {
-                return -1;
-            } else if (mFirstRect.bottom > mSecondRect.bottom) {
-                return 1;
-            } else if (mFirstRect.right < mSecondRect.right) {
-                return mIsLayoutRtl ? 1 : -1;
-            } else if (mFirstRect.right > mSecondRect.right) {
-                return mIsLayoutRtl ? -1 : 1;
+            if (involvesChain) {
+                // keep original order between chains
+                return mOriginalOrdinal.get(first) < mOriginalOrdinal.get(second) ? -1 : 1;
             } else {
-                // The view are distinct but completely coincident so we consider
-                // them equal for our purposes.  Since the sort is stable, this
-                // means that the views will retain their layout order relative to one another.
                 return 0;
             }
         }
-
-        private void getRect(View view, Rect rect) {
-            view.getDrawingRect(rect);
-            mRoot.offsetDescendantRectToMyCoords(view, rect);
-        }
     }
 }

diff --git a/core/java/android/view/ViewGroup.java b/core/java/android/view/ViewGroup.java
index b135bef..862a842 100644 (file)
--- a/core/java/android/view/ViewGroup.java
+++ b/core/java/android/view/ViewGroup.java
@@ -59,6 +59,7 @@ import com.android.internal.R;
 import com.android.internal.util.Predicate;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
@@ -1138,31 +1139,42 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
         final int focusableCount = views.size();
 
         final int descendantFocusability = getDescendantFocusability();
+        final boolean focusSelf = (isFocusableInTouchMode() || !shouldBlockFocusForTouchscreen());
 
-        if (descendantFocusability != FOCUS_BLOCK_DESCENDANTS) {
-            if (shouldBlockFocusForTouchscreen()) {
-                focusableMode |= FOCUSABLES_TOUCH_MODE;
+        if (descendantFocusability == FOCUS_BLOCK_DESCENDANTS) {
+            if (focusSelf) {
+                super.addFocusables(views, direction, focusableMode);
             }
+            return;
+        }
 
-            final int count = mChildrenCount;
-            final View[] children = mChildren;
+        if (shouldBlockFocusForTouchscreen()) {
+            focusableMode |= FOCUSABLES_TOUCH_MODE;
+        }
 
-            for (int i = 0; i < count; i++) {
-                final View child = children[i];
-                if ((child.mViewFlags & VISIBILITY_MASK) == VISIBLE) {
-                    child.addFocusables(views, direction, focusableMode);
-                }
+        if ((descendantFocusability == FOCUS_BEFORE_DESCENDANTS) && focusSelf) {
+            super.addFocusables(views, direction, focusableMode);
+        }
+
+        int count = 0;
+        final View[] children = new View[mChildrenCount];
+        for (int i = 0; i < mChildrenCount; ++i) {
+            View child = mChildren[i];
+            if ((child.mViewFlags & VISIBILITY_MASK) == VISIBLE) {
+                children[count++] = child;
             }
         }
+        Arrays.sort(children, 0, count, FocusFinder.getFocusComparator(this, false));
+        for (int i = 0; i < count; ++i) {
+            children[i].addFocusables(views, direction, focusableMode);
+        }
 
-        // we add ourselves (if focusable) in all cases except for when we are
-        // FOCUS_AFTER_DESCENDANTS and there are some descendants focusable.  this is
+        // When set to FOCUS_AFTER_DESCENDANTS, we only add ourselves if
+        // there aren't any focusable descendants.  this is
         // to avoid the focus search finding layouts when a more precise search
         // among the focusable children would be more interesting.
-        if ((descendantFocusability != FOCUS_AFTER_DESCENDANTS
-                // No focusable descendants
-                || (focusableCount == views.size())) &&
-                (isFocusableInTouchMode() || !shouldBlockFocusForTouchscreen())) {
+        if ((descendantFocusability == FOCUS_AFTER_DESCENDANTS) && focusSelf
+                && focusableCount == views.size()) {
             super.addFocusables(views, direction, focusableMode);
         }
     }