1498911 - Find in page doesn't cycle through the matches with the Enter key

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Keyboards and IME, defect, P2)

Description

[jerome.debretagne]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:62.0) Gecko/20100101 Firefox/62.0

Steps to reproduce:

- Use an Android device with a hardware keyboard (tested on a Pixel C with its keyboard, using both Firefox 62.0.3 and Nightly 64.0a1 2018-10-14)
- Press Ctrl+F to open Find in page
- Type in a string that has multiple matches
- Press the Enter key to move to the next match
- Press Shift+Enter to move to the previous match


Actual results:

Nothing happens in both cases


Expected results:

- Pressing Enter should move to the next match
- Pressing Shift+Enter should move to the previous match

This would allow to circle through the matches using the Enter key on hardware keyboard, bringing consistency with the other platforms (Linux, Windows, etc.)

Comment 1

[Andrei Bodea[:andrei]]

Thank you for your report jerome!

I was able to reproduce this issue on the latest Nightly build 65.0a1(28-Nov-2018) with a Google Pixel C Android 8.0.0 with a hardware keyboard attached.
I will set this issue as new.

Comment 2

[Liz Henry (:lizzard) (relman/hg->git project)]

Calling this a P2 for now though it may not be a recent regression, find in page is a key feature and it would be nice for it to work.

Comment 3

[Sorina Florean [:sflorean]]

Andrei, since you were able to reproduce this, can you check for a regression window? Thanks

Comment 4

[jerome.debretagne]

Sorina, why are you looking for a regression window?
As far as I understand, this had certainly never been implemented on Android until now, so this will have to be added to match the other platforms.

Comment 5

[Andrei Bodea[:andrei]]

Sorina, I couldn't find a regression window.
Also, note that I was able to reproduce this issue on a Nightly build from 2016.

Comment 6

[Petru-Mugurel Lingurar [:petru]]

Shift + Enter is actually inserted in the "find text" box as the newline character.
First will need to block that insertion if the Finder is shown.

Comment 7

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1498911 - FindInPageBar will now handle `Enter` and `Shift+Enter`; r?JanH

Enter will search for the next occurrence of the "Find" string.
Shift+Enter will search for the previous occurrence of the "Find" string.

Shift+Enter would normally append a newline character to the "Find" string.
We'll programmatically remove this character and then will treat the
Shift+Enter case.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ee85b1ccaa18
FindInPageBar will now handle Enter and Shift+Enter; r=JanH

Comment 9

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/ee85b1ccaa18

Comment 10

[Andrei Bodea[:andrei]]

I confirm that the commands presented in Comment 7 are working: Enter, Shift+Enter.

But there is one more command that is not really working, when finding the word you wanna search and
use Ctr+F type the word and press and hold on the Enter button, the search is stuck and is not moving to the next word.

Note that this action is working correctly on desktop, when tapping and holding the Enter button, it goes faster and faster to the next word.

For more details please see the video, note that you can see the Enter and Shift+Enter working but tapping and holding on Enter not working.
The selection is changed to the next word after I release the Enter button(after trying to tap and hold to fast switch words on the last part of the video).
Build: latest Nightly 67.0a1 build with Google Pixel C (Android 8.0.0).

I will re-open this issue based on my comment as it is partially fixed.

Comment 11

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1498911 - Allow cycling through search results when keeping the buttons pressed; r?JanH

For every Enter or Shift+Enter ACTION_DOWN event a new next/previous search
will be made.
Keeping the buttons pressed will cycle through all the search results endlessly.

Depends on D17133

Comment 12

[Petru-Mugurel Lingurar [:petru]]

Note to sheriffs:

• D17133 is already merged in m-c
• D18203 is a follow-up patch that now needs to also be landed.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/33cac757cc2a
Allow cycling through search results when keeping the buttons pressed; r=JanH

Comment 14

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/33cac757cc2a

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Did something need to land here still? Not sure why checkin-needed was set on the bug again :)

Comment 16

[Petru-Mugurel Lingurar [:petru]]

From my mistake. Thanks Ryan, all good!

Comment 17

[Andrei Bodea[:andrei]]

Verified as fixed in the latest Nightly build 67.0a1 following the scenarios presented in Comment 10 with Google Pixel C(Android 8.0.0).