Toolbar does not scroll off-screen on first page load after session restoration

mcomella commented 5 years ago

Steps to reproduce

Make sure a page is ready to be restored
- "Echo, open Firefox"
- Navigate to page (e.g. WIkipedia)
- Click a link
- "Echo, go home"
- adb shell am force-stop org.mozilla.connect.firefox or wait for timeout
"Echo, open Firefox"
- Verify page is restored. If following the instructions above, the last loaded page will be loaded and there will be no back stack
Scroll page around

Expected behavior

Toolbar moves on and off screen

Actual behavior

Toolbar stays on screen. When the user fullscreens a video, the toolbar remains on the screen. This state is fixed when the user:

opens the navigation overlay (e.g. to load a new page)
loads a url via the url bar

But clicking links to load new pages doesn't fix the issue. This is problematic if they user starts on YouTube and only clicks on links in YouTube to navigate.

This problem is pronounced on the ES gen 5 because of its small screen and will combine with our solutions to #284 to be even worse.

Device information

Echo Show device (1st gen, 2nd gen, or 5): verified on 5; probably affects all of them
Latest affected Firefox version: up/master ef4acaf3
Earliest affected Firefox version: ?

My assumption is that we set an initial toolbar state (canScroll = false) when starting the app on the Navigation Overlay. However, when we implemented session restore, we never updated this state.

If it helps, I changed the code in BrowserAppBarViewModel to this:

    val isAppBarHidden = MutableLiveData<Boolean>().apply {
        value = true
    }

And it seemed to fix the issue, sort of. It's not the right fix but maybe it'll help debugging.

nojunpark commented 5 years ago

I wasn't able to repro this on my Echo Show (gen 1) with FFTV 1.3. (OS Version: 641574920) Perhaps it's only reliably reproducible on ES gen 5?

dnarcese commented 5 years ago

I am also unable to reproduce on gen 2 but I also noticed that it only has v1.2. @mcomella If this is only reproducible on gen 5 and we don't have that device here in TOR, could someone in SF take this over?

mcomella commented 5 years ago

QA: please verify that:

When a session is restored (e.g. by following the above steps), the toolbar can be scrolled off the screen
When a session is not restored (on first run, or if app closes when initial homescreen is open), the toolbar does not scroll off the screen (I think that's the intended behavior but I might be wrong)

mcomella commented 5 years ago

@npark-mozilla I see this was moved to the Done column: should we change the label now to qa-verified (or whatever)?

nojunpark commented 5 years ago

Odd that we don't have qa-verified here, but yeah I should have removed the label, which should be sufficient

mozilla-mobile / firefox-echo-show