[ViewPager2] Focus events causing ViewPager2 to unexpectedly the change page [140656866]

Fixed

Bug

Status Update

No update yet.

Description

ne...@gmail.com

created issue #1

Sep 8, 2019 06:50AM

It doesn't matter if the keyboard is being hidden from code (hideSoftInputFromWindow) or from user interface by hitting back button quickly enough while the page transition is in progress.

I've added the keyboard hiding to my previously used (another bug report) example

https://github.com/AlmightyCZ/ViewPager2TextInputGravity
To replicate the issue, switch to second page(FORM), focus the editText, leave the keyboard open and use the tab to switch to the third page(LAST). The ViewPager jumps to LAST page for a split second and then immediately back to FORM. If the keyboard is not showing, then the page changes happens without any problem.

Using View/Handler.post for hideKeyboard invocation doesn't change the outcome, only postDelayed with long enough delay "works".

Comments

je...@google.com <je...@google.com> #2Sep 13, 2019 11:02PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 47ac9fee0e3d43b4da33f53b8a2ff590f0d46079
Author: Jakub Gielzak <jgielzak@google.com>
Date: Tue Aug 20 13:57:06 2019

Addressed issues with EditText causing scroll

EditText methods: bringPointIntoView, and handleFocusGainInternal
trigger requestChildRectangleOnScreen in ViewPager2's internal
RecyclerView.

This can cause:
- unwanted scrolling -- e.g. when typing on some API versions
- a jump to page 0 -- EditText sometimes reports over -1M px mScrollX

This fix bypasses requestChildRectangleOnScreen calculations taking
advantage of the fact that PagerSnapHelper constraints valid scroll
values to a snapped position ones, and as a result, the problem is
reduced to choosing the correct currentItem.

Bug: 138044582
Bug: 139432498
Test: ./gradlew viewpager2:connectedCheck

Change-Id: Ia4d3e4e6734183e64f261c7cc76d669deae78da5

A viewpager2/src/androidTest/java/androidx/viewpager2/widget/EditTextFocusTest.kt
M viewpager2/src/main/java/androidx/viewpager2/widget/ViewPager2.java

https://android-review.googlesource.com/1107013

https://goto.google.com/android-sha1/47ac9fee0e3d43b4da33f53b8a2ff590f0d46079

je...@google.com <je...@google.com> #3Sep 17, 2019 02:27PM

Fix [1] for this is planned to be released in 1.0.0-beta04.

[1]

http://aosp/1107013

je...@google.com <je...@google.com> #4Sep 17, 2019 03:53PM

beta04 with a fix for this was released today

je...@google.com <je...@google.com> #5Sep 17, 2019 05:00PM

Root cause:
After the keyboard is hidden, the focused TextView will ensure that it's cursor is visible on the screen, which leads to a call to requestChildRectangleOnScreen [1]. Since the EditText on the old page still has focus, ViewPager2 will initiate a scroll back to the old page.

Comparison with ViewPager:
Contrary to RecyclerView, ViewPager doesn't implement requestChildRectangleOnScreen() and the default implementation in ViewGroup does nothing.
Also, note that ViewPager transfers the focus to the new page during setCurrentItem [2].

Possible solutions:
1. Clear focus from old page and request focus on new page when updating mCurrentItem
2. Ignore calls to requestChildRectangleOnScreen like in ViewPager
3. Other..

I'm leaning towards solution 1.

[1]

https://android.googlesource.com/platform/frameworks/support/+/bfc1455f551ff1caf2766b3755361905a791e538/viewpager2/src/main/java/androidx/viewpager2/widget/ViewPager2.java#1013
[2]

https://android.googlesource.com/platform/frameworks/support/+/bfc1455f551ff1caf2766b3755361905a791e538/viewpager/src/main/java/androidx/viewpager/widget/ViewPager.java#1255

ap...@google.com <ap...@google.com> #6Sep 19, 2019 06:47PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit d4fa4242b0d945b8d3ff188602331b5124b3ace0
Author: Jakub Gielzak <jgielzak@google.com>
Date: Thu Sep 19 15:50:24 2019

Disabled requestChildRectangleOnScreen

It was a source of issues. Disabling it is on par with what ViewPager1
does. Users should use setCurrentItem instead.

Bug: 140656866
Test: EditTextFocusTest

Change-Id: I63c5f05fbc72174d7af044b6c7c689833f56ae11

M viewpager2/src/androidTest/java/androidx/viewpager2/widget/EditTextFocusTest.kt
M viewpager2/src/main/java/androidx/viewpager2/widget/ViewPager2.java

https://android-review.googlesource.com/1125359

https://goto.google.com/android-sha1/d4fa4242b0d945b8d3ff188602331b5124b3ace0

jg...@google.com <jg...@google.com> Sep 20, 2019 01:41PM

Accepted by je...@google.com.

ap...@google.com <ap...@google.com> #7Sep 20, 2019 05:06PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit f4ef0c4ea7c143931a166169ffdf499323f60842
Author: Jelle Fresen <jellefresen@google.com>
Date: Fri Sep 20 16:44:31 2019

Fix broken AccessibilityTest

performAccessibilityAction should be called on the UI thread. For some
reason it never surfaced before, presumably because the scrolling that
it triggers was performed in onAnimation and not as a direct result of
the action. Now we clear focus, however, this action triggers a change
in the view and the test crashes.

Bug: 140656866
Test: ./gradlew recyclerview:recyclerview:cC \
-Pandroid.testInstrumentationRunnerArguments.class=\
androidx.viewpager2.widget.AccessibilityTest

Change-Id: Ia7113fac1f9152ddaea4499a56220fea09822407

M viewpager2/src/androidTest/java/androidx/viewpager2/widget/AccessibilityTest.kt

https://android-review.googlesource.com/1126614

https://goto.google.com/android-sha1/f4ef0c4ea7c143931a166169ffdf499323f60842

ap...@google.com <ap...@google.com> #8Sep 20, 2019 05:06PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit d9000ec31ab4743ed07c8aa713d506ea57c37de2
Author: Jelle Fresen <jellefresen@google.com>
Date: Fri Sep 20 15:49:02 2019

Clear focus on page change in ViewPager2

Makes sure that a page can't keep focus when it is moving out of the
screen.

Bug: 140656866
Test: follow-up

Change-Id: I4e0fece0d4c438da76bfcb10939ba1bc880e27e7

M viewpager2/src/main/java/androidx/viewpager2/widget/ViewPager2.java

https://android-review.googlesource.com/1126610

https://goto.google.com/android-sha1/d9000ec31ab4743ed07c8aa713d506ea57c37de2

jg...@google.com <jg...@google.com> #9Sep 30, 2019 02:08PM

Fixes are in place now. Will come out in the next release.

ti...@gmail.com <ti...@gmail.com> #10Oct 1, 2019 09:20AM

Any plan when the next release is?

jg...@google.com <jg...@google.com> #11Oct 2, 2019 01:46PM

We are aiming to release beta05 with the fix on October 9, but we are unable to make promises as it depends on other AndroidX libraries.

ti...@gmail.com <ti...@gmail.com> #12Oct 2, 2019 03:09PM

Thanks, that's somewhat useful.

an...@wpost.com <an...@wpost.com> #13Oct 10, 2019 12:35PM

It looks like this change has broken the ability to use DPAD_LEFT or DPAD_RIGHT navigation by setting items as focusable.

https://android-review.googlesource.com/c/platform/frameworks/support/+/1125359/

Is there a workaround? I can make a sample if needed, and perhaps a new issue. Thanks!

jg...@google.com <jg...@google.com> #14Oct 11, 2019 05:29PM

Marked as fixed.

Fix for the original issue is now out (released on Oct 9 with beta05). Marking as Fixed.

Regarding

comment#13

(DPAD_*) -- moving to

http://issuetracker.google.com/142548536

pa...@google.com <pa...@google.com> #15Jul 5, 2022 10:40PM

This is still happening for me (see screencast) and seems to be caused by: https://android-review.googlesource.com/c/platform/frameworks/support/+/1126151

In our code, we have viewpager.setFocusability(false) but a child view is focusable. This combination causes the offending code to be called and the next item to show up briefly:

if (hasFocus()) { // if clear focus did not succeed
  mRecyclerView.requestFocus(View.FOCUS_FORWARD);
}

Reading the commit history it looks like the above code was added for API < 28. Is it possible to check for both hasFocus() && API < 28 here?

ap...@google.com <ap...@google.com> #16Dec 11, 2023 07:12PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 4e83b07b60706d954045f24b600338fae14abb2c
Author: Jakub Gielzak <jgielzak@google.com>
Date: Mon Sep 23 19:53:57 2019

Clear focus on page change on API < 28

Follow-up to: I4e0fece0d4c438da76bfcb10939ba1bc880e27e7

Bug: 140656866
Test: follow-up

Change-Id: I91bcd072e1812eeb20fbce6fb989557d60079bb7

M viewpager2/src/main/java/androidx/viewpager2/widget/ViewPager2.java

https://android-review.googlesource.com/1126151