Setting focusableInTouchMode clears FOCUSABLE_AUTO

Bug: 35726873
Test: View_FocusHandlingTest#testHasFocusable
Change-Id: I10ec94684846dc2280ba0bd7922d9304bf8a95db

diff --git a/core/java/android/view/View.java b/core/java/android/view/View.java
index 2cfdf90..e349170 100644 (file)
--- a/core/java/android/view/View.java
+++ b/core/java/android/view/View.java
@@ -4500,6 +4500,8 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
                     break;
                 case com.android.internal.R.styleable.View_focusableInTouchMode:
                     if (a.getBoolean(attr, false)) {
+                        // unset auto focus since focusableInTouchMode implies explicit focusable
+                        viewFlagValues &= ~FOCUSABLE_AUTO;
                         viewFlagValues |= FOCUSABLE_IN_TOUCH_MODE | FOCUSABLE;
                         viewFlagMasks |= FOCUSABLE_IN_TOUCH_MODE | FOCUSABLE_MASK;
                     }
@@ -6471,12 +6473,18 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
                 }
             }
         }
+
+        // Invisible and gone views are never focusable.
         if ((mViewFlags & VISIBILITY_MASK) != VISIBLE) {
             return false;
         }
-        return (allowAutoFocus
-                ? getFocusable() != NOT_FOCUSABLE
-                : getFocusable() == FOCUSABLE) && isFocusable();
+
+        // Only use effective focusable value when allowed.
+        if ((allowAutoFocus || getFocusable() != FOCUSABLE_AUTO) && isFocusable()) {
+            return true;
+        }
+
+        return false;
     }
 
     /**
@@ -8666,7 +8674,10 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
         // which, in touch mode, will not successfully request focus on this view
         // because the focusable in touch mode flag is not set
         setFlags(focusableInTouchMode ? FOCUSABLE_IN_TOUCH_MODE : 0, FOCUSABLE_IN_TOUCH_MODE);
+
+        // Clear FOCUSABLE_AUTO if set.
         if (focusableInTouchMode) {
+            // Clears FOCUSABLE_AUTO if set.
             setFlags(FOCUSABLE, FOCUSABLE_MASK);
         }
     }
@@ -12264,12 +12275,13 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
         // If focusable is auto, update the FOCUSABLE bit.
         int focusableChangedByAuto = 0;
         if (((mViewFlags & FOCUSABLE_AUTO) != 0)
-                && (changed & (FOCUSABLE_MASK | CLICKABLE | FOCUSABLE_IN_TOUCH_MODE)) != 0) {
-            int newFocus = NOT_FOCUSABLE;
-            if ((mViewFlags & (CLICKABLE | FOCUSABLE_IN_TOUCH_MODE)) != 0) {
+                && (changed & (FOCUSABLE_MASK | CLICKABLE)) != 0) {
+            // Heuristic only takes into account whether view is clickable.
+            final int newFocus;
+            if ((mViewFlags & CLICKABLE) != 0) {
                 newFocus = FOCUSABLE;
             } else {
-                mViewFlags = (mViewFlags & ~FOCUSABLE_IN_TOUCH_MODE);
+                newFocus = NOT_FOCUSABLE;
             }
             mViewFlags = (mViewFlags & ~FOCUSABLE) | newFocus;
             focusableChangedByAuto = (old & FOCUSABLE) ^ (newFocus & FOCUSABLE);

diff --git a/core/java/android/view/ViewGroup.java b/core/java/android/view/ViewGroup.java
index ffa79bd..3dd3ba8 100644 (file)
--- a/core/java/android/view/ViewGroup.java
+++ b/core/java/android/view/ViewGroup.java
@@ -1148,18 +1148,21 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
 
     @Override
     boolean hasFocusable(boolean allowAutoFocus, boolean dispatchExplicit) {
+        // This should probably be super.hasFocusable, but that would change
+        // behavior. Historically, we have not checked the ancestor views for
+        // shouldBlockFocusForTouchscreen() in ViewGroup.hasFocusable.
+
+        // Invisible and gone views are never focusable.
         if ((mViewFlags & VISIBILITY_MASK) != VISIBLE) {
             return false;
         }
 
-        // TODO This should probably be super.hasFocusable, but that would change behavior.
-        // The below is a much simpler check than we do in the superclass implementation,
-        // but it's been this way for a long time and other code likely relies on it.
-        if ((allowAutoFocus ? getFocusable() != NOT_FOCUSABLE : getFocusable() == FOCUSABLE)
-                && isFocusable()) {
+        // Only use effective focusable value when allowed.
+        if ((allowAutoFocus || getFocusable() != FOCUSABLE_AUTO) && isFocusable()) {
             return true;
         }
 
+        // Determine whether we have a focused descendant.
         final int descendantFocusability = getDescendantFocusability();
         if (descendantFocusability != FOCUS_BLOCK_DESCENDANTS) {
             final int count = mChildrenCount;