1379628 - Title bar obscures top of page / clicks (taps) need to be done about 1cm below

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Vincent Lefevre]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0
Build ID: 20170623152303

Steps to reproduce:

Select open tabs or open new pages.


Actual results:

The title bar obscures the top of the page; this occurs for any page. Moreover, when I want to select a link or an input text field, I need to tap about 1cm below (about the height of the title bar, as I think these problems are related).

Comment 1

[Vincent Lefevre]

Note: I had to report the bug from a desktop machine (the Firefox on my phone is hardly usable).

Comment 2

[Bogdan Surd, QA [:bsurd, NI]]

Hello, 

 Is this issue persistent even after doing a clear data or a fresh install of Firefox on your device? Could you also please provide some more information? Device type and android version? Did you install any addon recently that might have caused this? Or did you change any setting?

Thank you.

Comment 3

[Vincent Lefevre]

(In reply to Bogdan Surd, QA [:BogdanS] from comment #2)
>  Is this issue persistent even after doing a clear data or a fresh install
> of Firefox on your device?

It disappears after just quitting Firefox and restarting it. Until it reappears...

> Could you also please provide some more
> information? Device type and android version?

Samsung Galaxy S7 with Samsung's Android 7.0 (device not rooted).

> Did you install any addon recently that might have caused this?

No add-on (except the default one), but I could reproduce the problem just after using YouTube full-screen; however, I'm not sure how to reproduce it exactly. So, it seems to be similar to bug 998929 (but which is marked as fixed).

Comment 4

[Vincent Lefevre]

Something like that:
1. Set to landscape view.
2. Start a YouTube video.
3. Switch to full screen.
4. Back from full screen.
5. Open the list of tabs.
6. Set to portrait view.
7. Select another tab from the list of tabs. This triggers the issue.

Comment 5

[Vincent Lefevre]

I forgot to mention the exact Firefox version: Firefox Beta 55.0b7

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Randall, sounds like a dynamic toolbar bug

Comment 7

[Randall Barker [:rbarker]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #6)
> Randall, sounds like a dynamic toolbar bug

Yeah, It's something I've been investigating. Thought it was some how related to chrome custom tabs. The biggest problem is there aren't any good steps to reproduce yet.

Comment 8

[Randall Barker [:rbarker]]

I can reproduce now. Must be a race condition because it is very intermittent on my testing device.

Comment 9

[Randall Barker [:rbarker]]

The issue seems to be that when multiple REQUEST_SHOW_TOOLBAR_IMMEDIATELY messages are received, if the first request is still being process when the second one is received, a static snapshot update is requested. If the toolbar is not visible yet, this causes the snap shot pixel to never get consumed so the snapshot ready message is never generated. With out the ready message, the second REQUEST_SHOW_TOOLBAR_IMMEDIATELY gets stuck in an animation pending state.

Comment 10

[Randall Barker [:rbarker]]

Attachment: Bug 1379628 - Ensure pixels for Android dynamic toolbar snapshot get processed even if the toolbar is not visible so pending animations may run

Review commit: https://reviewboard.mozilla.org/r/156636/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/156636/

Comment 11

[Randall Barker [:rbarker]]

I created Bug 1380447 so that the multiple REQUEST_SHOW_TOOLBAR_IMMEDIATELY may be investigated.

Comment 12

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8885867 [details]
Bug 1379628 - Ensure pixels for Android dynamic toolbar snapshot get processed even if the toolbar is not visible so pending animations may run

https://reviewboard.mozilla.org/r/156636/#review161758

Not great to have a "GetXXX" function have such important side effects. Maybe rename GetToolbarEffect to something that is more representative of what it's really doing?

Comment 13

[Randall Barker [:rbarker]]

Comment on attachment 8885867 [details]
Bug 1379628 - Ensure pixels for Android dynamic toolbar snapshot get processed even if the toolbar is not visible so pending animations may run

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/156636/diff/1-2/

Comment 14

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8885867 [details]
Bug 1379628 - Ensure pixels for Android dynamic toolbar snapshot get processed even if the toolbar is not visible so pending animations may run

https://reviewboard.mozilla.org/r/156636/#review161770

LGTM, thanks!

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e6e712904806
Ensure pixels for Android dynamic toolbar snapshot get processed even if the toolbar is not visible so pending animations may run r=kats

Comment 16

[Carsten Book [:Tomcat]]

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

Comment 17

[Randall Barker [:rbarker]]

Comment on attachment 8885867 [details]
Bug 1379628 - Ensure pixels for Android dynamic toolbar snapshot get processed even if the toolbar is not visible so pending animations may run

Approval Request Comment
[Feature/Bug causing the regression]: Dynamic Toolbar V3
[User impact if declined]: When toggling between fullscreen it is possible for the toolbar to get stuck in a pending animation state and stops animating. In this state the toolbar covers the top part of the screen and all touch events are offset by the toolbar height.
[Is this code covered by automated tests?]: Sadly no.
[Has the fix been verified in Nightly?]:no
[Needs manual test from QE? If yes, steps to reproduce]: Toggling between fullscreen will intermittently trigger it. 
[List of other uplifts needed for the feature/fix]: none.
[Is the change risky?]: low risk
[Why is the change risky/not risky?]: patch removes early return so a function gets called every render instead of only when toolbar has a height.
[String changes made/needed]: none

Comment 19

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 8885867 [details]
Bug 1379628 - Ensure pixels for Android dynamic toolbar snapshot get processed even if the toolbar is not visible so pending animations may run

fix regression from fennec dynamic toolbar, beta55+

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ea2563ac86db

Comment 21

[Bogdan Surd, QA [:bsurd, NI]]

Devices:
 - Samsung Galaxy S7 Edge (Android 7.0)
 - Huawei MediaPad M2 (Adnroid 5.1.1)


Hello, I've verified this using the steps provided in both Comment 4 & Comment 17. The issue did not reproduce for me on any of the devices. Marking this as verified.

@Vincent if you have the time please also verify this is fixed on your end. Thanks!

Notes:
After exiting firescreen the page was scrolled up a bit.

Comment 22

[Vincent Lefevre]

I've tried to reproduce the bug with the latest beta55, and I couldn't. So, the bug seems really fixed.