1908330 - [Toolbar Redesign] The Navigation Bar flickers when dismissing the keyboard

Status: ASSIGNED

(Fenix :: Toolbar, defect, P2)

Description

[Delia Pop]

Attachment: screen-20240717-115924.mp4

Steps to reproduce

1. Open the latest Nightly 130.0a1 with the toolbar redesign enabled.
2. From homepage or a opened website, tap the address toolbar or the search button from the navigation bar.
3. Once the keyboard is invoked, tap the back button or tap anywhere on the screen to dismiss the keyboard.
4. Observe the bottom of the screen.

Expected behavior

The keyboard is dismissed without visual defects.

Actual behavior

A visual defect/flicker can be observed on the navigation bar while the keyboard is dismissed.

Device information

• Firefox version: Nightly 130.0a1 from 07/17
• Android device model: Google Pixel 8 Pro
• Android OS version: Android 14

Comment 1

[Petru-Mugurel Lingurar [:petru]]

I'm not sure what is the reported flicker. Might help to reduce "Animator duration scale" from developer settings.
@Delia Would this be the same as what was reported in bug 1911042?

Comment 2

[Delia Pop]

Attachment: media_20240828_173926_7542705587697213836_Slomo.mp4

The flicker I'm referring to in this ticket can be seen on the nav bar (or more exactly in it's location) right after the keyboard is removed: the address bar is displayed on homepage, and it is then pushed up when the navigation bar appears.

Shouldn't the navigation bar be already displayed on the homepage when the keyboard is dismissed?
If not, even if this is actually the expected behavior, the user can still see this animation as more of a visual defect.
Attaching a slow motion video recording and an image with the transition framed.

Comment 3

[Delia Pop]

Attachment: transitions.png

Comment 4

[Petru-Mugurel Lingurar [:petru]]

Thanks!
Think this will be fixed by the patch from bug 1911042 (only on Android 10+)

Comment 5

[Joe M [:jmahon]]

Marking this as "depends on" Bug 1911042. This should be retested after that is complete; we don't currently expect there to be additional work for this bug beyond that.

Comment 6

[Petru-Mugurel Lingurar [:petru]]

With the patches for bug 1911042 landed it would be great to know if we can consider this done also.

Comment 7

[Delia Pop]

Attachment: GooglePixel8Pro.mp4

I can still reproduce this issue in the latest Nightly 13.0a1 from 10/17 with Samsung Galaxy S22 Ultra (Android 14) and Google Pixel 8 Pro (Android 14).
However, I cannot see this flicker/animation on OPPO A15s (Android 10).

Comment 8

[Petru-Mugurel Lingurar [:petru]]

Attachment: ToolbarFlicker.mov

I'm not sure what the reported flicker is.
Based on the latest video - are you referring at what happens just in the beginning of the video (portion slowed down in the attached file)?
There we switch from the homescreen which has navbar + addressbar
To the search screen which has just the addressbar (another one)
So users yes, will see the update.

But based on the latest video the issue from https://bugzilla.mozilla.org/attachment.cgi?id=9421320 is no more, right?

Comment 9

[Delia Pop]

Attachment: slow motion.mp4

Hi, Petru,
yes, the issue from Comment 3 is fixed, the toolbar are no longer overlapping on homepage. Instead, we can now see the homepage exposed without showing any toolbar, in the timeframe between the keyboard being dismissed and default homepage displayed. In normal speed, this animation/flicker is the issue I'm referring to.

Comment 10

[Petru-Mugurel Lingurar [:petru]]

Thanks for the update!

Looking into this specific issue more it seems like we specifically went with hiding the navbar when users enter search to protect against the issue from bug 1895573.
This problem stems from the fact that in homescreen/browserscreen we show addressbar + navbar
While in search we show only the addressbar
So we can be left with either of

• both the navbar from home and the addressbar from search be shown at the same time (fixed in bug 1895573 by hiding the navbar)
• none of the navbar from home and the addressbar from search be shown at time (seen here as a result of bug 1895573)

Without a reliable way to sync the hiding/showing the toolbars

One thing we can try is having the two toolbars use shared element transition.

Comment 11

[Petru-Mugurel Lingurar [:petru]]

Attachment: SlowedDownSearchUXWithoutNavbar.mp4

Having looked into a shared toolbar between home and search it seems like we cannot rely on that now since SearchDialogFragment being a dialog has a separate window than that of HomeFragment.

Looking at how things function without the navbar we can see that the issue does not exist because we keep the addressbar showing.
Think we can replicate the same for when using the navbar with instead of hiding the entire bottom toolbar (addressbar + navbar) when entering search to hide just the navbar.

Comment 12

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1908330 - part 1 - Hide the navbar when entering search from browser r=#android-reviewers

This applies to when we navigate to the homescreen which initializes it's navbar and then
immediately opens the search dialog.
Previously we would hide the entire bottom toolbar (addressbar + navbar) which could lead
to situations in which there is no toolbar shown on the screen until the one from search
is visible.

Comment 13

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1908330 - part 2 - Hide the navbar when entering search from home screen r=#android-reviewers

This applies to when the user is on the home screen and the navbar is shown.
Currently in most situations we hide the home screen toolbar (addressbar + navbar) too early
and so for a split second there is no toolbar on the screen until the one for search is shown.

Moved to use the result listener framework which in tests is much faster than relying on store
updates to know if search is visible or not (instant vs ~60ms on a Pixel 6) which allows with
greater reliability to transition between the toolbar from the home screen and the toolbar
from the search screen.

Comment 14

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1908330 - part 3 - Show the navbar after the searching ends r=#android-reviewers

Since when entering search we hide the navbar but keep the addressbar visible when
exiting search we need to show back the navbar - if applicable.

Using the result listener framework is much faster than relying on store updates to
know if search is visible or not (instant vs ~60ms on a Pixel 6) allows to transition
from the search toolbar to the home screen toolbar in just one visual frame.

Comment 15

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1908330 - part 4 - Stop controlling the toolbar visibility from BottomToolbarContainerIntegration r=#android-reviewers

This was initially meant to only control the scrolling behavior of the toolbar.
The later added functionality to also control the toolbar functionality depending
on if search is active or not is suboptimal with the toolbar visibility updates being
too slow:

• when search is active this should hide the toolbar
  This was too slow to happen leading to two toolbar shown on screen, issue for which
  another patch was needed as part of bug 1895573.
• when search is closed this should show the toolbar
  This is too slow and leads to having no toolbar on the screen immediately after search
  is closed.
  We get the visibility update after a few tens of milliseconds so on the homescreen the
  toolbar will be shown but a flicker can still be seen and the visibility change happens
  too late - in the case of a search started from browser we'd try showing the homescreen
  toolbar after the user gets back to the browser.

Comment 16

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1908330 - part 5 - Remove BottomToolbarContainerIntegration from HomeFragment r=#android-reviewers

The BottomToolbarContainerIntegration through it's internal ToolbarBehaviorController
controls on whether the toolbar should be dynamic or not depending on the state of the
current browser tab.
Since on the home screen there in no active browser tab and the toolbar from the home screen is
always fixed in place and not dynamic there is no need for BottomToolbarContainerIntegration.