Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5db64eb1 authored by Evan Rosky's avatar Evan Rosky
Browse files

Don't allow 0-sized views to be focused.

These zero-sized views tend to make keyboard navigation
difficult for users since focus "disappears" sometimes.

This takes a best-effort approach for focus requested
prior to layout: If a View hasn't been laid-out and is
asked to take focus, it will act as though this size
constraint doesn't exist. Then, upon layout, it will
defocus itself if it still has no size.

Bug: 32072305
Test: Added CTS View_FocusHandlingTest#testSizeHandling
Change-Id: I82fee246bdf4964fe744142876da88ae79e491f0
parent d87c6b51
Loading
Loading
Loading
Loading
+108 −17
Original line number Diff line number Diff line
@@ -4179,6 +4179,11 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
     */
    private static boolean sUseDefaultFocusHighlight;
    /**
     * True if zero-sized views can be focused.
     */
    private static boolean sCanFocusZeroSized;
    private String mTransitionName;
    static class TintInfo {
@@ -4763,6 +4768,8 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
            sThrowOnInvalidFloatProperties = targetSdkVersion >= Build.VERSION_CODES.P;
            sCanFocusZeroSized = targetSdkVersion < Build.VERSION_CODES.P;
            sCompatibilityDone = true;
        }
    }
@@ -6975,6 +6982,7 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
    void clearFocusInternal(View focused, boolean propagate, boolean refocus) {
        if ((mPrivateFlags & PFLAG_FOCUSED) != 0) {
            mPrivateFlags &= ~PFLAG_FOCUSED;
            clearParentsWantFocus();
            if (propagate && mParent != null) {
                mParent.clearChildFocus(this);
@@ -10010,6 +10018,13 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
        return (mPrivateFlags3 & PFLAG3_IS_LAID_OUT) == PFLAG3_IS_LAID_OUT;
    }
    /**
     * @return {@code true} if laid-out and not about to do another layout.
     */
    boolean isLayoutValid() {
        return isLaidOut() && ((mPrivateFlags & PFLAG_FORCE_LAYOUT) == 0);
    }
    /**
     * If this view doesn't do any drawing on its own, set this flag to
     * allow further optimizations. By default, this flag is not set on
@@ -10782,7 +10797,7 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
        if (views == null) {
            return;
        }
        if (!isFocusable() || !isEnabled()) {
        if (!canTakeFocus()) {
            return;
        }
        if ((focusableMode & FOCUSABLES_TOUCH_MODE) == FOCUSABLES_TOUCH_MODE
@@ -10996,8 +11011,9 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
     * descendants.
     *
     * A view will not actually take focus if it is not focusable ({@link #isFocusable} returns
     * false), or if it is focusable and it is not focusable in touch mode
     * ({@link #isFocusableInTouchMode}) while the device is in touch mode.
     * false), or if it can't be focused due to other conditions (not focusable in touch mode
     * ({@link #isFocusableInTouchMode}) while the device is in touch mode, not visible, not
     * enabled, or has no size).
     *
     * See also {@link #focusSearch(int)}, which is what you call to say that you
     * have focus, and you want your parent to look for the next one.
@@ -11104,9 +11120,7 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
    private boolean requestFocusNoSearch(int direction, Rect previouslyFocusedRect) {
        // need to be focusable
        if ((mViewFlags & FOCUSABLE) != FOCUSABLE
                || (mViewFlags & VISIBILITY_MASK) != VISIBLE
                || (mViewFlags & ENABLED_MASK) != ENABLED) {
        if (!canTakeFocus()) {
            return false;
        }
@@ -11121,10 +11135,21 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
            return false;
        }
        if (!isLayoutValid()) {
            mPrivateFlags |= PFLAG_WANTS_FOCUS;
        }
        handleFocusGainInternal(direction, previouslyFocusedRect);
        return true;
    }
    void clearParentsWantFocus() {
        if (mParent instanceof View) {
            ((View) mParent).mPrivateFlags &= ~PFLAG_WANTS_FOCUS;
            ((View) mParent).clearParentsWantFocus();
        }
    }
    /**
     * Call this to try to give focus to a specific view or to one of its descendants. This is a
     * special variant of {@link #requestFocus() } that will allow views that are not focusable in
@@ -13496,6 +13521,13 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
        mAttachInfo.mUnbufferedDispatchRequested = true;
    }
    private boolean canTakeFocus() {
        return ((mViewFlags & VISIBILITY_MASK) == VISIBLE)
                && ((mViewFlags & FOCUSABLE) == FOCUSABLE)
                && ((mViewFlags & ENABLED_MASK) == ENABLED)
                && (sCanFocusZeroSized || !isLayoutValid() || (mBottom > mTop) && (mRight > mLeft));
    }
    /**
     * Set flags controlling behavior of this view.
     *
@@ -13515,6 +13547,7 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
            return;
        }
        int privateFlags = mPrivateFlags;
        boolean shouldNotifyFocusableAvailable = false;
        // If focusable is auto, update the FOCUSABLE bit.
        int focusableChangedByAuto = 0;
@@ -13553,7 +13586,7 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
                            || focusableChangedByAuto == 0
                            || viewRootImpl == null
                            || viewRootImpl.mThread == Thread.currentThread()) {
                        mParent.focusableViewAvailable(this);
                        shouldNotifyFocusableAvailable = true;
                    }
                }
            }
@@ -13576,10 +13609,7 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
                // about in case nothing has focus.  even if this specific view
                // isn't focusable, it may contain something that is, so let
                // the root view try to give this focus if nothing else does.
                if ((mParent != null) && ((mViewFlags & ENABLED_MASK) == ENABLED)
                        && (mBottom > mTop) && (mRight > mLeft)) {
                    mParent.focusableViewAvailable(this);
                }
                shouldNotifyFocusableAvailable = true;
            }
        }
@@ -13588,17 +13618,18 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
                // a view becoming enabled should notify the parent as long as the view is also
                // visible and the parent wasn't already notified by becoming visible during this
                // setFlags invocation.
                if ((mViewFlags & VISIBILITY_MASK) == VISIBLE
                        && ((changed & VISIBILITY_MASK) == 0)) {
                    if ((mParent != null) && (mViewFlags & ENABLED_MASK) == ENABLED) {
                        mParent.focusableViewAvailable(this);
                    }
                }
                shouldNotifyFocusableAvailable = true;
            } else {
                if (hasFocus()) clearFocus();
            }
        }
        if (shouldNotifyFocusableAvailable) {
            if (mParent != null && canTakeFocus()) {
                mParent.focusableViewAvailable(this);
            }
        }
        /* Check if the GONE bit has changed */
        if ((changed & GONE) != 0) {
            needGlobalAttributesUpdate(false);
@@ -20125,15 +20156,58 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
            }
        }
        final boolean wasLayoutValid = isLayoutValid();
        mPrivateFlags &= ~PFLAG_FORCE_LAYOUT;
        mPrivateFlags3 |= PFLAG3_IS_LAID_OUT;
        if (!wasLayoutValid && isFocused()) {
            mPrivateFlags &= ~PFLAG_WANTS_FOCUS;
            if (canTakeFocus()) {
                // We have a robust focus, so parents should no longer be wanting focus.
                clearParentsWantFocus();
            } else if (!getViewRootImpl().isInLayout()) {
                // This is a weird case. Most-likely the user, rather than ViewRootImpl, called
                // layout. In this case, there's no guarantee that parent layouts will be evaluated
                // and thus the safest action is to clear focus here.
                clearFocusInternal(null, /* propagate */ true, /* refocus */ false);
                clearParentsWantFocus();
            } else if (!hasParentWantsFocus()) {
                // original requestFocus was likely on this view directly, so just clear focus
                clearFocusInternal(null, /* propagate */ true, /* refocus */ false);
            }
            // otherwise, we let parents handle re-assigning focus during their layout passes.
        } else if ((mPrivateFlags & PFLAG_WANTS_FOCUS) != 0) {
            mPrivateFlags &= ~PFLAG_WANTS_FOCUS;
            View focused = findFocus();
            if (focused != null) {
                // Try to restore focus as close as possible to our starting focus.
                if (!restoreDefaultFocus() && !hasParentWantsFocus()) {
                    // Give up and clear focus once we've reached the top-most parent which wants
                    // focus.
                    focused.clearFocusInternal(null, /* propagate */ true, /* refocus */ false);
                }
            }
        }
        if ((mPrivateFlags3 & PFLAG3_NOTIFY_AUTOFILL_ENTER_ON_LAYOUT) != 0) {
            mPrivateFlags3 &= ~PFLAG3_NOTIFY_AUTOFILL_ENTER_ON_LAYOUT;
            notifyEnterOrExitForAutoFillIfNeeded(true);
        }
    }
    private boolean hasParentWantsFocus() {
        ViewParent parent = mParent;
        while (parent instanceof ViewGroup) {
            ViewGroup pv = (ViewGroup) parent;
            if ((pv.mPrivateFlags & PFLAG_WANTS_FOCUS) != 0) {
                return true;
            }
            parent = pv.mParent;
        }
        return false;
    }
    /**
     * Called from layout when this view should
     * assign a size and position to each of its children.
@@ -20240,6 +20314,23 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
            mOverlay.getOverlayView().setRight(newWidth);
            mOverlay.getOverlayView().setBottom(newHeight);
        }
        // If this isn't laid out yet, focus assignment will be handled during the "deferment/
        // backtracking" of requestFocus during layout, so don't touch focus here.
        if (!sCanFocusZeroSized && isLayoutValid()) {
            if (newWidth <= 0 || newHeight <= 0) {
                if (hasFocus()) {
                    clearFocus();
                    if (mParent instanceof ViewGroup) {
                        ((ViewGroup) mParent).clearFocusedInCluster();
                    }
                }
                clearAccessibilityFocus();
            } else if (oldWidth <= 0 || oldHeight <= 0) {
                if (mParent != null && canTakeFocus()) {
                    mParent.focusableViewAvailable(this);
                }
            }
        }
        rebuildOutline();
    }
+12 −3
Original line number Diff line number Diff line
@@ -3215,22 +3215,31 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
        }
        int descendantFocusability = getDescendantFocusability();

        boolean result;
        switch (descendantFocusability) {
            case FOCUS_BLOCK_DESCENDANTS:
                return super.requestFocus(direction, previouslyFocusedRect);
                result = super.requestFocus(direction, previouslyFocusedRect);
                break;
            case FOCUS_BEFORE_DESCENDANTS: {
                final boolean took = super.requestFocus(direction, previouslyFocusedRect);
                return took ? took : onRequestFocusInDescendants(direction, previouslyFocusedRect);
                result = took ? took : onRequestFocusInDescendants(direction,
                        previouslyFocusedRect);
                break;
            }
            case FOCUS_AFTER_DESCENDANTS: {
                final boolean took = onRequestFocusInDescendants(direction, previouslyFocusedRect);
                return took ? took : super.requestFocus(direction, previouslyFocusedRect);
                result = took ? took : super.requestFocus(direction, previouslyFocusedRect);
                break;
            }
            default:
                throw new IllegalStateException("descendant focusability must be "
                        + "one of FOCUS_BEFORE_DESCENDANTS, FOCUS_AFTER_DESCENDANTS, FOCUS_BLOCK_DESCENDANTS "
                        + "but is " + descendantFocusability);
        }
        if (result && !isLayoutValid() && ((mPrivateFlags & PFLAG_WANTS_FOCUS) == 0)) {
            mPrivateFlags |= PFLAG_WANTS_FOCUS;
        }
        return result;
    }

    /**