1912988 - [toolbar redesign] the YouTube playlist table disappears when the toolbar is hidden

Status: VERIFIED FIXED

(Fenix :: Toolbar, defect, P1)

Description

[eclaudiu64]

Attachment: mobizen_20240813_191055.mp4

User Agent: Mozilla/5.0 (Android 11; Mobile; rv:131.0) Gecko/131.0 Firefox/131.0

Steps to reproduce:

Setting: Scroll to hide address bar and toolbar activated and after we enter any playlist on m.youtube.com, for example with music and scroll down, it can be seen that the table showing the list of videos in the playlist disappears.

Actual results:

The playlist table disappears when the toolbar is hidden.

Expected results:

The table should remain on the screen regardless of the status of the toolbar.

Comment 1

[eclaudiu64]

Can it be set to the errors for the new toolbar?

Comment 2

[eclaudiu64]

And this error is related to this error: https://bugzilla.mozilla.org/show_bug.cgi?id=1888410
Because there is a problem with that table and the old toolbar.

Comment 3

[Chris Peterson [:cpeterson]]

Thanks for reporting this bug and finding the related bug 1888410 for the old toolbar. This is an interesting bug because the new toolbar shouldn't affect the page content's layout when the toolbar is closed, but it does.

Comment 4

[[:harrisono]]

Attachment: nightly131ScreenCapture.mp4

Screen capture of nightly v131 showing the playlist m.youtube.com banner disappearing on scroll with "scroll to hide" enabled and the new navigation toolbar enabled.

Comment 5

[[:harrisono]]

Attachment: release129ScreenCapture.mp4

Release v129 showing the playlist m.youtube.com banner not disappearing on scroll with "scroll to hide" enabled and no new navigation toolbar.

Comment 6

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1912988 - part 1 - Ensure conversions of dp to px are not floored r=#android-reviewers

On a real device with a density of 2.75 1dp is measured as 3px while with using toInt()
we'd calculate it to be 2px.
roundToInt() gives the expected result (closest integer) and is also what is used
internally by compose for similar operations.

Comment 7

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1912988 - part 2 - Ensure we show a divider only when the microsurvey is showing r=#android-reviewers

The navbar layout always has a top divider which was previously not included in the computed
height of the toolbar leading to errors when compared to the measured height.

Comment 8

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1912988 - part 3 - Round values for vertical clipping instead of always flooring r=#android-reviewers

The behavior applied when using just the address bar was already doing this, now
the behavior applied when using the navbar will do the same.

Comment 9

[Petru-Mugurel Lingurar [:petru]]

Attachment: TranslatingFixedElements.mp4

The above patches would help getting the same behavior when using the navbar to when we'd use just the addressbar.
This issue per se would be fixed but I think another still remains: this particular element - the youtube playlist is translated faster / further than other fixed elements like youtube's bottom bar.

@Botond - is this something that the APZ team can help with / do you know who else owns this functionality?

Comment 10

[Petru-Mugurel Lingurar [:petru]]

To describe the issue seen: we were calling setVerticalClipping with a value bigger than that set through setDynamicToolbarMaxHeight.
The above patches ensure this will not happen anymore.

Comment 11

[Botond Ballo [:botond]]

(In reply to Petru-Mugurel Lingurar [:petru] from comment #9)

Created attachment 9422231 [details]
TranslatingFixedElements.mp4

The above patches would help getting the same behavior when using the navbar to when we'd use just the addressbar.
This issue per se would be fixed but I think another still remains: this particular element - the youtube playlist is translated faster / further than other fixed elements like youtube's bottom bar.

@Botond - is this something that the APZ team can help with / do you know who else owns this functionality?

Yes, the issue is reminiscent of bug 1894136.

Please go ahead and file a new issue for this in the Core :: Panning and Zooming component. Thanks!

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1fe11f79d109
part 1 - Ensure conversions of dp to px are not floored r=android-reviewers,mavduevskiy
https://hg.mozilla.org/integration/autoland/rev/63b1685c86c6
part 2 - Ensure we show a divider only when the microsurvey is showing r=android-reviewers,skhan
https://hg.mozilla.org/integration/autoland/rev/8c38504b6e67
part 3 - Round values for vertical clipping instead of always flooring r=android-reviewers,mavduevskiy

Comment 13

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/1fe11f79d109
https://hg.mozilla.org/mozilla-central/rev/63b1685c86c6
https://hg.mozilla.org/mozilla-central/rev/8c38504b6e67

Comment 14

[Delia Pop]

Attachment: YT_Playlist.mp4

Verified as fixed on latest Nightly 132.0a1 from 09/11 with the following devices:

• Samsung Galaxy A53 5G (Android 14)
• Xiaomi 12 Pro (Android 13)
• LG G7 fit (Android 9)