If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Backout newtab changes including layout and styling changes for beta

VERIFIED FIXED in Firefox 31

Status

()

Firefox
New Tab Page
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: Mardak, Assigned: Mardak)

Tracking

Trunk
Firefox 31
Points:
---

Firefox Tracking Flags

(firefox31+ verified, firefox32+ verified, firefox-esr31+ verified)

Details

Attachments

(5 attachments, 3 obsolete attachments)

(Assignee)

Description

3 years ago
For 31 currently in beta, we want to backout all newtab related changes to avoid bug 1000097
(Assignee)

Comment 1

3 years ago
Created attachment 8456500 [details] [diff] [review]
v1

https://tbpl.mozilla.org/?tree=Try&rev=405f57f4722e
(Assignee)

Comment 2

3 years ago
For the latest attempt, I reverted newtab related directories to just before this commit:

* a682dda - Bug 895359 - Switch about:newtab to Promise.jsm and remove remaining callback r=jaws (6 months ago) <Tim Taubert>

git di --binary a682dda^ browser/{base/content{,/test},themes/{linux,osx,windows}}/newtab | git apply -R

Then added back in various commits:
https://github.com/Mardak/tiles-dev/pull/40


Tests are failing because drag/drop events aren't being synthesized correctly:
https://tbpl.mozilla.org/?tree=Try&rev=ff4cda08a684
https://tbpl.mozilla.org/php/getParsedLog.php?id=43899728&tree=Try
uncaught exception - TypeError: win is null at chrome://mochitests/content/browser/browser/base/content/test/newtab/head.js:465
https://hg.mozilla.org/try/file/a557e47d7c26/browser/base/content/test/newtab/head.js#l465
   462 function synthesizeNativeMouseEvent(aElement, aMsg, aOffsetX = 0, aOffsetY = 0) {

   463   let rect = aElement.getBoundingClientRect();

   464   let win = aElement.ownerDocument.defaultView;

   465   let x = aOffsetX + win.mozInnerScreenX + rect.left + rect.width / 2;
status-firefox31: --- → affected
tracking-firefox31: --- → +
(Assignee)

Comment 3

3 years ago
So backing out newtab related changes didn't seem to work and backing out even more still didn't work, so I tried to revert just bug 991210 bug 980014 bug 978338 and patch up search by making the search-form flex 5 and padding it on both sides with <div class="newtab-side-margin"/>.

https://tbpl.mozilla.org/?tree=Try&rev=b5e6c1fce07a
https://github.com/Mardak/tiles-dev/pull/41
Keywords: verifyme
(Assignee)

Comment 4

3 years ago
Created attachment 8456979 [details]
30 Release screenshot
(Assignee)

Comment 5

3 years ago
Created attachment 8456981 [details]
31 Beta backout screenshot

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/edilee@gmail.com-b5e6c1fce07a/try-macosx64/
(Assignee)

Comment 6

3 years ago
Created attachment 8456987 [details]
30/31 scrollbar difference overlap

With the backedout beta keeping the search bar, the scrollbars start appearing at a bigger height. See attached overlapped view of 31 vs 30.
status-firefox32: --- → affected
tracking-firefox32: --- → +
(Assignee)

Comment 7

3 years ago
Created attachment 8457026 [details] [diff] [review]
v2

There were 4 steps involved in this patch:
- Revert Bug 980014
- Revert Bug 978338
- Revert Bug 991210
- Fix up search form 
https://github.com/Mardak/tiles-dev/pull/41/commits
Assignee: nobody → edilee
Attachment #8456500 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8457026 - Flags: review?(adw)
(Assignee)

Comment 8

3 years ago
Created attachment 8457027 [details] [diff] [review]
release-to-backout.patch

Not sure if this helps, but these are the changes to browser newtab code since Release.

Updated

3 years ago
Attachment #8457026 - Flags: review?(adw) → review+
(Assignee)

Updated

3 years ago
Attachment #8457026 - Flags: approval-mozilla-release?
Attachment #8457026 - Flags: approval-mozilla-release? → approval-mozilla-release+
(Assignee)

Comment 9

3 years ago
https://hg.mozilla.org/releases/mozilla-release/rev/2552ea57100d
(In reply to Ed Lee :Mardak from comment #9)
> https://hg.mozilla.org/releases/mozilla-release/rev/2552ea57100d

Setting the status flag to fixed so it shows up in our RC2 verification query. 

Also marking ESR status flag since this needs to land for 31esr AFAIK.
status-firefox31: affected → fixed
status-firefox-esr31: --- → affected
tracking-firefox-esr31: --- → ?
https://hg.mozilla.org/releases/mozilla-esr31/rev/acedd56e2e08
status-firefox-esr31: affected → fixed
(Assignee)

Comment 12

3 years ago
Created attachment 8458055 [details] [diff] [review]
for aurora

I cherry-picked the patch to aurora, but bc1 is failing:
https://tbpl.mozilla.org/?tree=Try&rev=17ae17d2bcc7

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/general/browser_tabopen_reflows.js | Test timed out
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/general/browser_tabopen_reflows.js | application timed out after 330 seconds with no output
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/general/browser_tabopen_reflows.js | application terminated with exit code 5
PROCESS-CRASH | chrome://mochitests/content/browser/browser/base/content/test/general/browser_tabopen_reflows.js | application crashed [@ libSystem.B.dylib + 0xd7a]
Attachment #8457027 - Attachment is obsolete: true
(Assignee)

Comment 13

3 years ago
Actually, the test failures in comment 12 seem to be caused by bug 1039881.
(Assignee)

Updated

3 years ago
No longer blocks: 1000097
(Assignee)

Comment 14

3 years ago
Comment on attachment 8458055 [details] [diff] [review]
for aurora

Approval Request Comment
[Feature/regressing bug #]: revert bug 980014, bug 978338
[User impact if declined]: users will see a new tiles layout in 32 then a different one in 33
[Describe test coverage new/current, TBPL]: https://tbpl.mozilla.org/?tree=Try&rev=0768f53e2850
[Risks and why]: low risk - already landed for 31rc2
[String/UUID change made/needed]: none
Attachment #8458055 - Flags: approval-mozilla-aurora?
tracking-firefox-esr31: ? → +
Attachment #8458055 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
QA Contact: cornel.ionce
Verified that the mentioned newtab changes were successfully backed-out from:
1. Firefox 31 RC build 2 (build ID: 20140716183446)
2. Firefox 31 ESR (build ID: 20140717132905).

Changing the tracking flags accordingly. I will leave the "verifyme" keyword untouched until all branches will be fixed and verified.
status-firefox31: fixed → verified
status-firefox-esr31: fixed → verified
https://hg.mozilla.org/releases/mozilla-aurora/rev/238174823b7c
status-firefox32: affected → fixed
Backed out from Aurora due to tart crashes that appear to have been triggered by this.
https://hg.mozilla.org/releases/mozilla-aurora/rev/5bb486b0f89a

https://tbpl.mozilla.org/php/getParsedLog.php?id=44124951&tree=Mozilla-Aurora
status-firefox32: fixed → affected
(Assignee)

Comment 18

3 years ago
Created attachment 8458897 [details] [diff] [review]
for aurora checkin

This bug is clear of the tart crash: https://tbpl.mozilla.org/?tree=Try&rev=9a0ba6ff126f
Attachment #8458055 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Whiteboard: checkin-needed-aurora
https://hg.mozilla.org/releases/mozilla-aurora/rev/4e4056fa74af
status-firefox32: affected → fixed
Whiteboard: checkin-needed-aurora
(Assignee)

Comment 20

3 years ago
Fixed for both 31 (beta -> release) and 32 (aurora -> beta)
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Verified fixed on Firefox 32.0a2 Aurora, build ID: 20140721004001.
I'll keep an eye on this fix to make sure it landed properly in Firefox 31 release and Firefox 32 beta.
Status: RESOLVED → VERIFIED
status-firefox32: fixed → verified
Keywords: verifyme
Target Milestone: --- → Firefox 31
This landed properly in Firefox 31 release (build ID: 20140716183446) and Firefox 32 beta 1 (build ID: 20140722195627).
You need to log in before you can comment on or make changes to this bug.