1437213 - Revert top site pref to show 1 row instead of 2 rows for 59

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect)

Description

[Ed Lee :Mardak]

[Tracking Requested - why for this release]: The change to 2 rows happened in bug 1425489 but after analyzing the data and feedback, it was decided that we want 1 row especially because bug 1385944 switched 60 to have 1 row by default.

Comment 1

[Ed Lee :Mardak]

Attachment: Bug 1437213 - Revert top site pref to show 1 row instead of 2 rows for 59.

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

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 8949904 [details]
Bug 1437213 - Revert top site pref to show 1 row instead of 2 rows for 59.

https://reviewboard.mozilla.org/r/219212/#review225432

Just tested a beta build with this patch; it seems to be working nicely.  r=dmose

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 8949904 [details]
Bug 1437213 - Revert top site pref to show 1 row instead of 2 rows for 59.

https://reviewboard.mozilla.org/r/219212/#review225434

Comment 4

[Ed Lee :Mardak]

Comment on attachment 8949904 [details]
Bug 1437213 - Revert top site pref to show 1 row instead of 2 rows for 59.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1425489 switched to 2 rows in 59
[User impact if declined]: Firefox 59 will show 2 rows up from 1 in 58 then back to 1 in 60 (with bug 1385944)
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Kind of in bug 1385944 as the code is different for 59 uplift than the one that landed in nightly 60.
[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?]: It's a pref flip to show 1 row (6 top sites) instead of 2 rows (12 top sites) as shown in the original source commit https://github.com/mozilla/activity-stream/commit/93298009490f201eb9660cb549f77f73d34a068d Although the mozilla-beta export change is large because we generate different html to render 6 sites instead of 12 for each supported locale.
[String changes made/needed]: None

Comment 5

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

Comment on attachment 8949904 [details]
Bug 1437213 - Revert top site pref to show 1 row instead of 2 rows for 59.

Pref-flip, low risk, Beta59+

Hi Tom, Andrei, this NI is to ensure we have this tested. I do not think this change will land in m-c and therefore good test coverage on beta will help.

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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

Comment 7

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

I managed to conduct a brief functional testing on 59.0b10 build3 (20180215111455) around this change. I can confirm that the fix was properly implemented. Also, no UI or functional regressions were triggered. More details about the mentioned testing results can be found here: https://goo.gl/3Q5gd8.