(In reply to csheany from comment #10)

Ever now and then the button is covered by the toolbar after exiting
fullscreen.

How are you exiting fullscreen? The test page does not provide a way to exit fullscreen.

The devices back button.

It happens when the page is zoomed in.

Should I clone to discuss?

(In reply to csheany from comment #16)

The devices back button.

Thanks.

It happens when the page is zoomed in.

Ah, ok. That sounds like it may be bug 1516471, then.

Should I clone to discuss?

Feel free to either mention this behaviour in a comment on bug 1516471, or file a bug that depends on bug 1516471.

How is this performing for you in Nightly 67.0a1 (2019-03-08)?

With the testcase and steps in comment 2, it's 100% reproducible on my phone.

Thank you for your response.

Do you mean that it still isn't hiding properly?

Yes, it still isn't hiding properly.

Have you been keeping an eye on it.

Do you know for how long?

If you are able to test the previous Nightly is the result the same?

It used to freeze with the button and toolbar visible at the same time...

But now I am seeing the test page reach the top of the screen under the toolbar.

(In reply to csheany from comment #22)

Have you been keeping an eye on it.

Do you know for how long?

No, I don't know.

(In reply to csheany from comment #23)

If you are able to test the previous Nightly is the result the same?

I am able to do that, but that takes time. Unless you have specific change in mind which may have changed the behavior recently, I'm not going to do that.

I don't remember the last time I checked but I thought it was happening fever times but now it is more.

The reason I was asking is...

I was testing Bug 1504865 with Bug 1158392 landed and as you can imagine made that difficult.

I was surprised because I didn't remeber it being so bad.

Yes it still wasn't smooth but it was for as long.

I cleared data for a fresh install and that may have caused Comment 24 but I'm not sure.

I mean consistent.

I just tested the second attachment and Bug 1504865 doesn't occur.

I was confused for a minute but looking at the page source it has meta data.

I wonder if that is a factor and may provide a clue as to resolving.

It was thought to be fixed by Bug 1158392 but that doesn't seem the case unless it depends on additonal patches.

I don't know if you have kept up with Bug 1504865 but it may have been cause by Bug 1298218.

Do you have any thoughts about that?

You don't have to check.

I guess I couldn't believe it was still an issue as the last comment (I made) was 2 months ago.

Do you still not know what caused the regression?

Is it about the unprefixed version of the Fullscreen API?

Thanks. That all makes sense.

So, it seems to me that bug 1158392 (containerless scrolling) is actually the reason which causes the url bar to consistently stay after entering fullscreen (as described in comment 19). If you go to about:config and change layout.scroll.root-frame-containers to 1 (from 0) and refresh the page, you would see url bar disappears automatically. (Also it seems to me even after doing this, bug 1504865 still doesn't occur for me, so maybe it has been fixed by something else as well.)

I guess maybe previously some mechanism triggers a repaint or something which removes the url bar, but now that is no longer triggered immediately.

I realize my comments in between Comment 25 were long winded and I was afraid you were in the process of testing the previous Nightly before reading Comment 30.

I also didn't want you to think it was out of the blue given the time frame from Comment 17 it just seemed related.

Would you mind posting your findings in Bug 1504865?

I think Bug 1528743 is related to the about:config pref.

Oh, well, it seems after changing layout.scroll.root-frame-containers I can reproduce bug 1504865 with https://bug1493976.bmoattachments.org/attachment.cgi?id=9018847 so it wasn't fixed without containerless scrolling... I guess nothing new worth reporting there, then.

Would you mind clariying the states in which it is fixed?

ni? Xidorn for Comment #35.

So, it seems that containerless scrolling makes this issue happen very consistently.

If I disable containerless scrolling (via setting layout.scroll.root-frame-containers to 1), this bug can reproduce but in a very low frequency.

This patch fixes the problem for me locally. I'm not familiar enough with the dynamic toolbar code to tell if this is the right approach.

:botond I still see the issue intermittently with your fix. Do you know what could have changed? Is the page not getting composited after it goes full screen anymore? I don't see this issue in beta or release so it seems to be a recent regression.

The page gets composited after it goes full screen, but at that time the compositor toolbar height, as seen in LayerManagerComposite::RenderToolbar() is still nonzero.

The toolbar height gets set to zero subsequently, and no place in the toolbar animator code is requesting another composite.

I think this is a latent bug that's been around for quite some time (likely since the latest dynamic toolbar rewrite), and it's just been masked by the fact that something else happened to be requesting a composite later. (Most of the time, anyways; if you read the comments further above, the issue is still reproducible at a low frequency on beta/release.)

It seems clear that the toolbar animator code should request its own composites after changing the toolbar height, and not rely on something else to coincidentally request a composite.

The only thing I'm really unsure about, is where in the toolbar animator code we should be requesting the composite - the place where I added it in the patch, or somewhere else in UpdateAnimation(), or in UpdateCompositorToolbarHeight() perhaps?

Does this fullscreen issue affect Fenix or just Fennec? If it affects Fenix, we might want to uplift it to GV 67 Beta.

I believe this is Fennec only.

See also Bug 1517809

https://hg.mozilla.org/mozilla-central/rev/0ebb51472ce4

(In reply to csheany from comment #46)

I believe this is Fennec only.

Thanks. If this bug is Fennec only, then we can let the fix ride with Fennec 68 instead of rushing to uplift to Fennec 67 Beta.

Uplifting might be worth it given Bug 1158392 which made this worse.

(In reply to csheany from comment #49)

Uplifting might be worth it given Bug 1158392 which made this worse.

Botond, should we uplift this fullscreen fix to Fenenc 67 Beta? csheany points out that containerless scrolling, which made this problem worse, rode the trains to Beta with Fennec 67 (bug 1137890).

The relationship to containerless scrolling is incidental, but given that this is a very low risk one-line patch, I support uplifting it to 67. Randall, any objection?

I don't think it can do any harm in uplifting it.

Comment on attachment 9052552 [details]
Bug 1500719 - Ensure toolbar is hidden when going fullscreen

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: Bug 1335895
• User impact if declined: When user enters fullscreen mode, the URL bar can remain visible until the user taps the page.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a very low risk patch, as all it does is request a composite at the end of a toolbar animation.
• String changes made/needed:

Comment on attachment 9052552 [details]
Bug 1500719 - Ensure toolbar is hidden when going fullscreen

Low risk patch for a visible Fennec regression, uplift approved for 67 beta 7, thanks.

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

Verified as fixed in Beta 67.0b7 build 1 on Android 6.0.1.

Also verified on the latest Nightly 68.0a1 (2019-03-31) on Galaxy S6 - Android 6.0.1.

Hi!

Due to Comment 57 and Comment 58 I'll mark this issue as Verified Fixed.

Thanks.