1898059 - Back gesture is not consistently dismissing the edit toolbar

Status: NEW

(Fenix :: Toolbar, defect, P2)

Description

[Roger Yang [:royang]]

Attachment: Screen_recording_20240521_121141.webm

Currently using back gesture, we are not consistently dismissing the the edit toolbar and showing the display toolbar with the navigation toolbar.

Comment 1

[Mike a [:mavduevskiy]]

Context

The handling of back-pressed gestures has derived from initial intention with introduction of gesture-based navigation system. To see the intended behaviour, switch the device to 3-button navigation and play with the address bar: when engaging with the toolbar in the edit mode, any back gesture would close the edit mode and switch the toolbar into display mode.

Problem

Switching to Gesture navigation changes the behaviour: a back gesture closes the keyboard, but doesn't cancel the edit mode. It's still there, focused, but in the edit state, hiding important UI elements. User has to guess to do the back gesture again to actually close the edit mode.

Solution

We want to keep sync the behaviour between gesture and button navigation systems, by making gesture navigation behave in a similar way to the button one.

Now that I wrote it down, the question popped in my head why not the other way around. In case I am not the only one who got that question:
– the intent is to do a (hopefully) small fix to make the component to behave in line with the original intent, since there is already infrastructure around catching onback press and cancelling edit mode;
– it's in line with the behaviour of other browsers (in chrome, back gesture hides the keyboard and makes the addressbar lose its focus;
– it streamlines visibility states of toolbar/navigationbar/search.

Comment 2

[Chris Peterson [:cpeterson]]

We want to fix this bug in phase 1, but it doesn't block Nightly testing.

Comment 3

[Chris Peterson [:cpeterson]]

Increasing priority to P1 now that we're fixing toolbar phase 1's beta blockers.

Comment 4

[Joe M [:jmahon]]

Marking as P2 for the purpose of the half-sprint starting on 7/30.

Comment 5

[Petru-Mugurel Lingurar [:petru]]

Attachment: NavigationButtonsVsGesturesInSearchScreen.mp4

Adding a new recording showing the different behaviour between tapping back and swiping back.

Comment 6

[Petru-Mugurel Lingurar [:petru]]

Aarjav provided the direction for this:
"The .. behaviour of going back to the homepage should be applied when the user executes the gesture back or OS nav bar back."

Comment 7

[Mike a [:mavduevskiy]]

Attachment: WIP: Bug 1898059 - Close search fragment on onBackPressed

Some additional context: with the introduction of gesture navigation, onBackPressed behaviour became inconsistent when working with editviews. Back button, part of the iconic system navigation buttons, would call onBackPressed when pressed with edittext in focus and keyboard shown. But when the system navigation was set to gestures, the first back gesture wouldn't get registered at all.

The gesture is invisible for dispatchKeyEvent listener and to onBackPress calls. It hides the keyboard and clears the focus from the editview. The solution in this patch is based around the cleared focus of the edittext: when the editview loses its focus, we trigger onBackPressed.

Another aproach we could go for here was to listen to the keyboard state through a viewTreeObserver, dismissing the search fragment once the keyboard is hidden, but that would introduce an unwanted side-effect. With gesture navigation, keyboard has a didecated hide-keyboard button, that hides the keyboard but keeps the element in focus.

Comment 8

[Petru-Mugurel Lingurar [:petru]]

I am curious on which devices this was tested.
On an Android 14 device I see the issue described in the ticket while on an Android 11 device I don't.
Which would mean different framework behaviour for the back button.
Which would mean that we should maybe focus on not using the deprecated onBackPressed method and not relying on KEYCODE_BACK anymore with Android clearly stating that intercepting back events from KeyEvent.KEYCODE_BACK is no longer supported
While accepting this as a minor inconvenient for now.

Comment 9

[Petru-Mugurel Lingurar [:petru]]

As discussed in triage with Channing we'll move this to a release blocker since more changes are needed to support this.