1211016 - backout Bug 1194895 changes from beta release to avoid regression in Bug 1210091

Status: RESOLVED FIXED

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

Description

[maxim zhilyaev]

Following discussion in Bug 1210091, Bug 1194895 changes must be backed out from beta to avoid Bug 1210091 behavior

Comment 1

[maxim zhilyaev]

Attachment: Bug-1211016.beta.v1.patch

Approval Request Comment

This is a reverse patch of 1194895 patch.  However, I kept some of 1194895 changes to the newtab mochitests tests, as their removal causes cascading test failures.  

[Feature/regressing bug #]:
Bug 1211016 - backout Bug 1194895 changes from beta release to avoid regression in Bug 1210091

[User impact if declined]:

Very significant, as 1194895 changes caused 1211016 failure mode.
This is not a new bug - bug 1000097 was identified to have spotted identical issues, but it was dropped due to rare reproduction.  Fix to 1194895 caused conditions described in 1000097 appear far more often because the scrollbar on newtab allows bottom row of tiles to be partially visible. 
We need to remove newtab scrollbar in beta per dougt request.

[Describe test coverage new/current, TreeHerder]:

Since beta treeherder is noisy, I ran two try builds with and without the patch.
Try push with the patch:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cf441ec4dffe

Try push without the patch:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=01cbd26274aa

I could not find significant differences, and the ones existing are not related to newtab.

I further tested manually on OsX, and verified that the scrollbar does not show up as expected.

[Risks and why]: 

Moderate risk - we are removing a significant UX function. However, the negative user impact outweighs the risks in this case.

[String/UUID change made/needed]:
None

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8669517 [details] [diff] [review]
Bug-1211016.beta.v1.patch

OK, taking it for 42. Should be in beta 4.
What is the plan for 43 & 44?
Thanks

Comment 3

[maxim zhilyaev]

to the best if my knowledge the plan is to fix Bug 1210091 and have scrollbar on newtab in 43 and 44

Comment 4

[Sylvestre Ledru [:Sylvestre]]

Taking in account Doug's comment in https://bugzilla.mozilla.org/show_bug.cgi?id=1210091#c1
I am considering this as an r+ from him and we can take it in 42.
Should be in beta 6

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/2743c9ab7748