1488846 - Pocket cta for logged out users flickers a bit before displaying the logged in version (topics) and shifting content

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect, P1)

Description

[Scott [:thecount] Downe]

A followup bug found after landing https://bugzilla.mozilla.org/show_bug.cgi?id=1483655

And another small bug found by me caused by the same above ticket, found while I was fixing the other.

1. Flicker of logged in/out state while the page loads. You can see this in an attachment. Example: https://user-images.githubusercontent.com/197334/45060065-6ad37680-b06c-11e8-9341-f2a6cf17bba3.gif

2. The height of the cta was actually a few pixels larger than the topics, which potentially causes a slight shift in content as it loads. I made sure on max screen sizes the content doesn't move.

Initial pr fix here: https://github.com/mozilla/activity-stream/pull/4401

Comment 1

[Ed Lee :Mardak]

https://github.com/mozilla/activity-stream/pull/4401

Comment 2

[Ed Lee :Mardak]

https://hg.mozilla.org/mozilla-central/rev/8dde92f89a24

Comment 3

[Ed Lee :Mardak]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/d2e41f2f964d
Backed out changeset 8dde92f89a24 for browser_asrouter_cfr.js failures. a=backout

Relanded:
https://hg.mozilla.org/mozilla-central/rev/581019e9ea70

Comment 4

[Ciprian Muresan [:cmuresan], Ecosystem QA]

I have verified that the Pocket explainer no longer flickers between its logged in/out states when the page is refreshed on the latest Nightly 64.0a1 (Build ID 20180920220102) on Windows 10 x64, Mac 10.13, and Arch Linux x64.

Comment 5

[Pascal Chevrel:pascalc (PTO until April 26)]

Scott, why are you asking tracking for Firefox 63? Will you request an uplift of this patch for our last 63 beta this week? Thanks

Comment 6

[Scott [:thecount] Downe]

Yeah, it is my plan to get this uplifted, but I hit a snag around moving the changes over that I need to work through.

It's related to the build process and not the code itself. Once I get it building the correct diff, it should be easy.

Comment 7

[Scott [:thecount] Downe]

Attachment: Bug 1488846 - Ensure pocket cta doesn't flash between logged in and out while loading, and ensure content doesn't shift when adding cta.

Comment 8

[Scott [:thecount] Downe]

Comment on attachment 9015332 [details]
Bug 1488846 - Ensure pocket cta doesn't flash between logged in and out while loading, and ensure content doesn't shift when adding cta.

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1483655

User impact if declined: Bad user experience

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: Yes

If yes, steps to reproduce: You can see it in action here: https://user-images.githubusercontent.com/197334/45060065-6ad37680-b06c-11e8-9341-f2a6cf17bba3.gif

Really just load the about:home page without being logged into pocket.

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Has automated tests, small code changes, and has been verified on nightly for some time now.

String changes made/needed: None

Comment 9

[Scott [:thecount] Downe]

Correction, List of other uplifts needed: 1496246

Comment 10

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9015332 [details]
Bug 1488846 - Ensure pocket cta doesn't flash between logged in and out while loading, and ensure content doesn't shift when adding cta.

On nightly for 3 weeks and verified by QA, uplift approved for 63 beta 14, thanks.

Comment 11

[Stefan Hindli [:stefan_hindli]]

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

Comment 12

[Marius Coman [:mcoman], Ecosystem QA]

I have verified that this issue is no longer reproducible with the latest Firefox Beta (63.0b14 Build ID - 20181011200118) installed, on Windows 10 x64, Arch Linux and Mac 10.13.3. Now, the Pocket explainer no longer flickers between its logged in/out states when the page is refreshed.