Skip to content

Commit dd29f8c

Browse files
Amith YamasaniAndroid (Google) Code Review
authored andcommitted
Merge "Revert "Incorrect behavior of View clear focus.""
2 parents 9ae7631 + 73eb97f commit dd29f8c

File tree

5 files changed

+33
-207
lines changed

5 files changed

+33
-207
lines changed

core/java/android/view/View.java

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3766,14 +3766,6 @@ public void clearFocus() {
37663766
}
37673767

37683768
if ((mPrivateFlags & FOCUSED) != 0) {
3769-
// If this is the first focusable do not clear focus since the we
3770-
// try to give it focus every time a view clears its focus. Hence,
3771-
// the view that would gain focus already has it.
3772-
View firstFocusable = getFirstFocusable();
3773-
if (firstFocusable == this) {
3774-
return;
3775-
}
3776-
37773769
mPrivateFlags &= ~FOCUSED;
37783770

37793771
if (mParent != null) {
@@ -3782,24 +3774,9 @@ public void clearFocus() {
37823774

37833775
onFocusChanged(false, 0, null);
37843776
refreshDrawableState();
3785-
3786-
// The view cleared focus and invoked the callbacks, so now is the
3787-
// time to give focus to the the first focusable to ensure that the
3788-
// gain focus is announced after clear focus.
3789-
if (firstFocusable != null) {
3790-
firstFocusable.requestFocus(FOCUS_FORWARD);
3791-
}
37923777
}
37933778
}
37943779

3795-
private View getFirstFocusable() {
3796-
ViewRootImpl viewRoot = getViewRootImpl();
3797-
if (viewRoot != null) {
3798-
return viewRoot.focusSearch(null, FOCUS_FORWARD);
3799-
}
3800-
return null;
3801-
}
3802-
38033780
/**
38043781
* Called to clear the focus of a view that is about to be removed.
38053782
* Doesn't call clearChildFocus, which prevents this view from taking

core/java/android/view/ViewGroup.java

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -675,14 +675,11 @@ public void clearChildFocus(View child) {
675675
*/
676676
@Override
677677
public void clearFocus() {
678-
if (DBG) {
679-
System.out.println(this + " clearFocus()");
680-
}
681-
if (mFocused == null) {
682-
super.clearFocus();
683-
} else {
678+
super.clearFocus();
679+
680+
// clear any child focus if it exists
681+
if (mFocused != null) {
684682
mFocused.clearFocus();
685-
mFocused = null;
686683
}
687684
}
688685

@@ -694,12 +691,12 @@ void unFocus() {
694691
if (DBG) {
695692
System.out.println(this + " unFocus()");
696693
}
697-
if (mFocused == null) {
698-
super.unFocus();
699-
} else {
694+
695+
super.unFocus();
696+
if (mFocused != null) {
700697
mFocused.unFocus();
701-
mFocused = null;
702698
}
699+
mFocused = null;
703700
}
704701

705702
/**

core/java/android/view/ViewRootImpl.java

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,6 @@ public final class ViewRootImpl extends Handler implements ViewParent,
174174
View mView;
175175
View mFocusedView;
176176
View mRealFocusedView; // this is not set to null in touch mode
177-
View mOldFocusedView;
178177
int mViewVisibility;
179178
boolean mAppVisible = true;
180179
int mOrigWindowType = -1;
@@ -2273,34 +2272,33 @@ boolean scrollToRectOrFocus(Rect rectangle, boolean immediate) {
22732272

22742273
public void requestChildFocus(View child, View focused) {
22752274
checkThread();
2276-
2277-
if (DEBUG_INPUT_RESIZE) {
2278-
Log.v(TAG, "Request child focus: focus now " + focused);
2275+
if (mFocusedView != focused) {
2276+
mAttachInfo.mTreeObserver.dispatchOnGlobalFocusChange(mFocusedView, focused);
2277+
scheduleTraversals();
22792278
}
2280-
2281-
mAttachInfo.mTreeObserver.dispatchOnGlobalFocusChange(mOldFocusedView, focused);
2282-
scheduleTraversals();
2283-
22842279
mFocusedView = mRealFocusedView = focused;
2280+
if (DEBUG_INPUT_RESIZE) Log.v(TAG, "Request child focus: focus now "
2281+
+ mFocusedView);
22852282
}
22862283

22872284
public void clearChildFocus(View child) {
22882285
checkThread();
22892286

2290-
if (DEBUG_INPUT_RESIZE) {
2291-
Log.v(TAG, "Clearing child focus");
2292-
}
2293-
2294-
mOldFocusedView = mFocusedView;
2295-
2296-
// Invoke the listener only if there is no view to take focus
2297-
if (focusSearch(null, View.FOCUS_FORWARD) == null) {
2298-
mAttachInfo.mTreeObserver.dispatchOnGlobalFocusChange(mOldFocusedView, null);
2299-
}
2287+
View oldFocus = mFocusedView;
23002288

2289+
if (DEBUG_INPUT_RESIZE) Log.v(TAG, "Clearing child focus");
23012290
mFocusedView = mRealFocusedView = null;
2291+
if (mView != null && !mView.hasFocus()) {
2292+
// If a view gets the focus, the listener will be invoked from requestChildFocus()
2293+
if (!mView.requestFocus(View.FOCUS_FORWARD)) {
2294+
mAttachInfo.mTreeObserver.dispatchOnGlobalFocusChange(oldFocus, null);
2295+
}
2296+
} else if (oldFocus != null) {
2297+
mAttachInfo.mTreeObserver.dispatchOnGlobalFocusChange(oldFocus, null);
2298+
}
23022299
}
23032300

2301+
23042302
public void focusableViewAvailable(View v) {
23052303
checkThread();
23062304

@@ -2772,7 +2770,6 @@ private boolean enterTouchMode() {
27722770
mView.unFocus();
27732771
mAttachInfo.mTreeObserver.dispatchOnGlobalFocusChange(focused, null);
27742772
mFocusedView = null;
2775-
mOldFocusedView = null;
27762773
return true;
27772774
}
27782775
}

core/tests/coretests/src/android/widget/focus/RequestFocus.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@
2121
import android.app.Activity;
2222
import android.os.Bundle;
2323
import android.os.Handler;
24+
import android.widget.LinearLayout;
2425
import android.widget.Button;
26+
import android.view.View;
2527

2628
/**
2729
* Exercises cases where elements of the UI are requestFocus()ed.

core/tests/coretests/src/android/widget/focus/RequestFocusTest.java

Lines changed: 7 additions & 154 deletions
Original file line numberDiff line numberDiff line change
@@ -16,27 +16,21 @@
1616

1717
package android.widget.focus;
1818

19+
import android.widget.focus.RequestFocus;
20+
import com.android.frameworks.coretests.R;
21+
1922
import android.os.Handler;
20-
import android.test.ActivityInstrumentationTestCase2;
21-
import android.test.UiThreadTest;
23+
import android.test.ActivityInstrumentationTestCase;
2224
import android.test.suitebuilder.annotation.LargeTest;
2325
import android.test.suitebuilder.annotation.MediumTest;
24-
import android.util.AndroidRuntimeException;
25-
import android.view.View;
26-
import android.view.View.OnFocusChangeListener;
27-
import android.view.ViewTreeObserver.OnGlobalFocusChangeListener;
2826
import android.widget.Button;
29-
30-
import com.android.frameworks.coretests.R;
31-
32-
import java.util.ArrayList;
33-
import java.util.List;
27+
import android.util.AndroidRuntimeException;
3428

3529
/**
3630
* {@link RequestFocusTest} is set up to exercise cases where the views that
3731
* have focus become invisible or GONE.
3832
*/
39-
public class RequestFocusTest extends ActivityInstrumentationTestCase2<RequestFocus> {
33+
public class RequestFocusTest extends ActivityInstrumentationTestCase<RequestFocus> {
4034

4135
private Button mTopLeftButton;
4236
private Button mBottomLeftButton;
@@ -45,7 +39,7 @@ public class RequestFocusTest extends ActivityInstrumentationTestCase2<RequestFo
4539
private Handler mHandler;
4640

4741
public RequestFocusTest() {
48-
super(RequestFocus.class);
42+
super("com.android.frameworks.coretests", RequestFocus.class);
4943
}
5044

5145
@Override
@@ -100,145 +94,4 @@ public void testWrongThreadRequestFocusFails() throws Exception {
10094
e.getClass().getName());
10195
}
10296
}
103-
104-
/**
105-
* This tests checks the case in which the first focusable View clears focus.
106-
* In such a case the framework tries to give the focus to another View starting
107-
* from the top. Hence, the framework will try to give focus to the view that
108-
* wants to clear its focus. From a client perspective, the view does not loose
109-
* focus after the call, therefore no callback for focus change should be invoked.
110-
*
111-
* @throws Exception If an error occurs.
112-
*/
113-
@UiThreadTest
114-
public void testOnFocusChangeNotCalledIfFocusDoesNotMove() throws Exception {
115-
// Get the first focusable.
116-
Button button = mTopLeftButton;
117-
118-
// Make sure that the button is the first focusable and focus it.
119-
button.getRootView().requestFocus(View.FOCUS_DOWN);
120-
assertTrue(button.hasFocus());
121-
122-
// Attach on focus change listener that should not be called.
123-
button.setOnFocusChangeListener(new OnFocusChangeListener() {
124-
@Override
125-
public void onFocusChange(View v, boolean hasFocus) {
126-
throw new IllegalStateException("Unexpeced call to"
127-
+ "OnFocusChangeListener#onFocusChange");
128-
}
129-
});
130-
131-
// Attach on global focus change listener that should not be called.
132-
button.getViewTreeObserver().addOnGlobalFocusChangeListener(
133-
new OnGlobalFocusChangeListener() {
134-
@Override
135-
public void onGlobalFocusChanged(View oldFocus, View newFocus) {
136-
throw new IllegalStateException("Unexpeced call to"
137-
+ "OnFocusChangeListener#onFocusChange");
138-
}
139-
});
140-
141-
// Try to clear focus.
142-
button.clearFocus();
143-
}
144-
145-
/**
146-
* This tests check whether the on focus change callbacks are invoked in
147-
* the proper order when a View loses focus and the framework gives it to
148-
* the fist focusable one.
149-
*
150-
* @throws Exception
151-
*/
152-
@UiThreadTest
153-
public void testOnFocusChangeCallbackOrder() throws Exception {
154-
// Get the first focusable.
155-
Button clearingFocusButton = mTopRightButton;
156-
Button gainingFocusButton = mTopLeftButton;
157-
158-
// Make sure that the clearing focus is not the first focusable.
159-
View focusCandidate = clearingFocusButton.getRootView().getParent().focusSearch(null,
160-
View.FOCUS_FORWARD);
161-
assertNotSame("The clearing focus button is not the first focusable.",
162-
clearingFocusButton, focusCandidate);
163-
assertSame("The gaining focus button is the first focusable.",
164-
gainingFocusButton, focusCandidate);
165-
166-
// Focus the clearing focus button.
167-
clearingFocusButton.requestFocus();
168-
assertTrue(clearingFocusButton.hasFocus());
169-
170-
// Register the invocation order checker.
171-
CallbackOrderChecker checker = new CallbackOrderChecker(clearingFocusButton,
172-
gainingFocusButton);
173-
clearingFocusButton.setOnFocusChangeListener(checker);
174-
gainingFocusButton.setOnFocusChangeListener(checker);
175-
clearingFocusButton.getViewTreeObserver().addOnGlobalFocusChangeListener(checker);
176-
177-
// Try to clear focus.
178-
clearingFocusButton.clearFocus();
179-
180-
// Check that no callback was invoked since focus did not move.
181-
checker.verify();
182-
}
183-
184-
/**
185-
* This class check whether the focus change callback are invoked in order.
186-
*/
187-
private class CallbackOrderChecker implements OnFocusChangeListener,
188-
OnGlobalFocusChangeListener {
189-
190-
private class CallbackInvocation {
191-
final String mMethodName;
192-
final Object[] mArguments;
193-
194-
CallbackInvocation(String methodName, Object[] arguments) {
195-
mMethodName = methodName;
196-
mArguments = arguments;
197-
}
198-
}
199-
200-
private final View mClearingFocusView;
201-
private final View mGainingFocusView;
202-
203-
private final List<CallbackInvocation> mInvocations = new ArrayList<CallbackInvocation>();
204-
205-
public CallbackOrderChecker(View clearingFocusView, View gainingFocusView) {
206-
mClearingFocusView = clearingFocusView;
207-
mGainingFocusView = gainingFocusView;
208-
}
209-
210-
@Override
211-
public void onFocusChange(View view, boolean hasFocus) {
212-
CallbackInvocation invocation = new CallbackInvocation(
213-
"OnFocusChangeListener#onFocusChange", new Object[] {view, hasFocus});
214-
mInvocations.add(invocation);
215-
}
216-
217-
@Override
218-
public void onGlobalFocusChanged(View oldFocus, View newFocus) {
219-
CallbackInvocation invocation = new CallbackInvocation(
220-
"OnFocusChangeListener#onFocusChange", new Object[] {oldFocus, newFocus});
221-
mInvocations.add(invocation);
222-
}
223-
224-
public void verify() {
225-
assertSame("All focus change callback should be invoked.", 3, mInvocations.size());
226-
assertInvioked("Callback for View clearing focus explected.", 0,
227-
"OnFocusChangeListener#onFocusChange",
228-
new Object[] {mClearingFocusView, false});
229-
assertInvioked("Callback for View global focus change explected.", 1,
230-
"OnFocusChangeListener#onFocusChange", new Object[] {mClearingFocusView,
231-
mGainingFocusView});
232-
assertInvioked("Callback for View gaining focus explected.", 2,
233-
"OnFocusChangeListener#onFocusChange", new Object[] {mGainingFocusView, true});
234-
}
235-
236-
private void assertInvioked(String message, int order, String methodName,
237-
Object[] arguments) {
238-
CallbackInvocation invocation = mInvocations.get(order);
239-
assertEquals(message, methodName, invocation.mMethodName);
240-
assertEquals(message, arguments[0], invocation.mArguments[0]);
241-
assertEquals(message, arguments[1], invocation.mArguments[1]);
242-
}
243-
}
24497
}

0 commit comments

Comments
 (0)