1400920 - The pocket section briefly shows up and causes the scrollbar to make the whole page flicker

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect)

Description

[Florian Quèze [:florian]]

Attachment: Screen recording

See attached video. Somehow I can only reproduce this when opening new windows, I don't see it with the first window during startup.

There are 7 frames where the activity stream about:home page is displayed with an empty Pocket section, causing a vertical scrollbar. Then it disappears, the scrollbar too, and everything else shifts to the right due to the space freed by the scrollbar.

Comment 1

[Florian Quèze [:florian]]

Attachment: Screenshot

Comment 2

[Florian Quèze [:florian]]

(In reply to Florian Quèze [:florian] [:flo] from comment #0)
> Somehow I can only reproduce this when opening new
> windows, I don't see it with the first window during startup.

Actually I just saw it during startup now.

Comment 3

[Ed Lee :Mardak]

Fixed by https://github.com/mozilla/activity-stream/pull/3622 to be uplifted in this bug.

Comment 4

[Ed Lee :Mardak]

Attachment: Bug 1400920 - The pocket section briefly shows up and causes the scrollbar to make the whole page flicker.

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

Comment 5

[Ursula Sarracini (:ursula)]

Comment on attachment 8913266 [details]
Bug 1400920 - The pocket section briefly shows up and causes the scrollbar to make the whole page flicker.

https://reviewboard.mozilla.org/r/184648/#review189830

Thanks!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dd4eea6b8ae4
The pocket section briefly shows up and causes the scrollbar to make the whole page flicker. r=ursula

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 8

[Ursula Sarracini (:ursula)]

Verified fixed on Nightly 58.0a1 20170929100122

Comment 9

[Ed Lee :Mardak]

Comment on attachment 8913266 [details]
Bug 1400920 - The pocket section briefly shows up and causes the scrollbar to make the whole page flicker.

Approval Request Comment
[Feature/Bug causing the regression]: Activity Stream prerendering
[User impact if declined]: Users who don't see Pocket (only shown for US/CA/DE) might see content jumping around horizontally as scrollbars appear then disappear.
[Is this code covered by automated tests?]: No, this is just a css change
[Has the fix been verified in Nightly?]: Yes, 20170929100122
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Not really
[Why is the change risky/not risky?]: Only changing 1 line of css (and strings)
[String changes made/needed]: Pontoon localized strings were added that are not exposed for localization via mozilla-central

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Why do we have unrelated string changes in this patch?

Comment 11

[Ed Lee :Mardak]

flod suggested that we keep the strings up to date through the beta 57 cycle, but I can just manually edit the patch to not have strings, but the manual steps could introduce risk in the activity stream export process. Should I reattach a manually edited export patch without strings and request uplift a strings-only patch later in the beta process?

Comment 12

[Sylvestre Ledru [:Sylvestre]]

Yeah, clearly, I would like a bug == one issue being fixed, no unrelated changes.

Comment 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8913266 [details]
Bug 1400920 - The pocket section briefly shows up and causes the scrollbar to make the whole page flicker.

This is needed to fix Bug 1404345, Beta57+

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/46771933d922

Comment 15

[Andrei Vaida [:avaida]]

(In reply to Ed Lee :Mardak from comment #9)
> [Is this code covered by automated tests?]: No, this is just a css change
> [Has the fix been verified in Nightly?]: Yes, 20170929100122
> [Needs manual test from QE? If yes, steps to reproduce]: No

Setting qe-verify- based on Ed's assessment on manual testing needs and the fact that this fix has automated coverage.