1829039 - Updated embedded migration wizard styles to match figma

Status: VERIFIED FIXED

(Firefox :: Messaging System, enhancement, P1)

Description

[Punam Dahiya [:pdahiya]]

Scope of this bug is to implement UI fixes - embedded migration wizard UI in shorter window width responsive layout, font sizes as per figma

https://www.figma.com/file/c83OEio9DhCQKuabx2tIEl/NUO-Experiments-2023?node-id=976-31535

and UI specific feedback from base patch https://phabricator.services.mozilla.com/D175945#5802326

https://phabricator.services.mozilla.com/D175945#inline-973585

Comment 1

[Mike Conley (:mconley) (:⚙️)]

I believe we can get the sizes we want, proportional to the default font size settings in about:welcome, by setting font-size: 0.83em on the migration-wizard element within the about:welcome CSS.

Bug 1829733 should also bring the checkbox spacing closer to the spec.

Comment 2

[Negin]

Attachment: Bug 1829039 - Updates embedded migration wizard styles to match figma

Comment 3

[Shane Hughes [:aminomancer]]

Attachment: WIP: Bug 1829039 - Individually animate AW embedded migration wizard elements. r=negin

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cdaa6a0312f0
Updates embedded migration wizard styles to match figma r=aminomancer,mconley

Comment 5

[Cosmin Sabou [:CosminS]]

Backed out for causing bc failures on browser_aboutwelcome_multistage_mr.js.

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Csuccess%2Crunning%2Cpending%2Crunnable&fromchange=9f2b8ebce4cb12f450fda7fe0f23fe26692d56b1&searchStr=os%2Cx%2C10.15%2Cwebrender%2Cdebug%2Cmochitests%2Cwith%2Cnetworking%2Con%2Csocket%2Cprocess%2Ctest-macosx1015-64-qr%2Fdebug-mochitest-browser-chrome-spi-nw%2Cbc1&tochange=39c38caea8df91143c5eac952126223df57bb435&selectedTaskRun=R-evbUSjRAuw04TBzbhIRw.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=414502784&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/a587a3ddf3ec2e2d513b537a38d588c3408113ea

From what I've seen they're OS X only: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&fromchange=908855299d543f0f0f2499eebd343596f413b959&searchStr=bc&tochange=a835c99f230beb6a5eee12e8bd9e1480fbf84fc9&test_paths=browser%2Fcomponents%2Fnewtab%2Ftest%2Fbrowser%2Fbrowser.ini&selectedTaskRun=DA5KZBxjSgCiW8hfbltJ3Q.0

Comment 6

[Norisz Fay [:noriszfay]]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/a587a3ddf3ec

Comment 7

[Negin]

(In reply to Cosmin Sabou [:CosminS] from comment #5)

Backed out for causing bc failures on browser_aboutwelcome_multistage_mr.js.

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Csuccess%2Crunning%2Cpending%2Crunnable&fromchange=9f2b8ebce4cb12f450fda7fe0f23fe26692d56b1&searchStr=os%2Cx%2C10.15%2Cwebrender%2Cdebug%2Cmochitests%2Cwith%2Cnetworking%2Con%2Csocket%2Cprocess%2Ctest-macosx1015-64-qr%2Fdebug-mochitest-browser-chrome-spi-nw%2Cbc1&tochange=39c38caea8df91143c5eac952126223df57bb435&selectedTaskRun=R-evbUSjRAuw04TBzbhIRw.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=414502784&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/a587a3ddf3ec2e2d513b537a38d588c3408113ea

From what I've seen they're OS X only: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&fromchange=908855299d543f0f0f2499eebd343596f413b959&searchStr=bc&tochange=a835c99f230beb6a5eee12e8bd9e1480fbf84fc9&test_paths=browser%2Fcomponents%2Fnewtab%2Ftest%2Fbrowser%2Fbrowser.ini&selectedTaskRun=DA5KZBxjSgCiW8hfbltJ3Q.0

Sorry about that! Test failure should be resolved now.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/72f882230f63
Updates embedded migration wizard styles to match figma r=aminomancer,mconley

Comment 9

[Serban Stanca [:SerbanS]]

Backed out for causing mochitests failures in browser_aboutwelcome_multistage_mr.js.

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | browser/components/newtab/test/browser/browser_aboutwelcome_multistage_mr.js | Panel should be tightly anchored to the bottom of the button shadow node. - 275.2833251953125 == 0 - got 275.2833251953125, expected 0 (operator ==)

Now also seems to be only on OS X.

Comment 10

[Negin]

Sorry about that again! I'm unable to replicate the issue locally and it didn't catch the failure on my try run. Will be landing a (hopefully) fix for this failure shortly.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/743c4f059b79
Updates embedded migration wizard styles to match figma r=aminomancer,mconley

Comment 12

[Shane Hughes [:aminomancer]]

The patch should not be backed out actually, this seems to be an earlier regression that just wasn't caught previously (bug 1831174)

Comment 13

[Cristian Tuns]

Backed out for causing bc failures in browser_aboutwelcome_multistage_mr.js

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | browser/components/newtab/test/browser/browser_aboutwelcome_multistage_mr.js | Panel should be tightly anchored to the bottom of the button shadow node. - 284.5333557128906 == 0 - got 284.5333557128906, expected 0 (operator ==)

Comment 14

[Shane Hughes [:aminomancer]]

The failure is not caused by this patch, see bug 1831174.

Comment 15

[Cristian Tuns]

I will wait for the backfills and reland it if necessary in a bit.

Comment 16

[Cristian Tuns]

The backfills shows that the perma fail is caused by Bug 1829039 : https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&fromchange=439228f47bdde69dc2be1c7ea2f37528f4a412b8&searchStr=windows%2C7%2Cwebrender%2Copt%2Cmochitests%2Ctest-windows7-32-qr%2Fopt-mochitest-browser-chrome%2Cbc15&tochange=b8a3e39ead7360cc33ba17b03f0bb998b55059d3
The failures on Bug 1831174 are on mac mostly and Intermittent , https://treeherder.mozilla.org/jobs?repo=autoland&revision=57b92e69fc6f57bca1c3bef6dc20385b6660f33b&selectedTaskRun=esOgy6VHRFiPSe1nAcbCMQ.0&searchStr=OS%2CX%2C10.15%2CWebRender%2Copt%2CMochitests%2Cwith%2Cnetworking%2Con%2Csocket%2Cprocess%2Ctest-macosx1015-64-qr%2Fopt-mochitest-browser-chrome-spi-nw%2Cbc3&group_state=expanded as you can see here. But this one is on win 7 and it is perma failing (14/14), Can you please take another look? I can't reland this with the parma failures.

Comment 17

[Shane Hughes [:aminomancer]]

Ah, I should have looked more closely at it. We'll get to the bottom of it, thanks.

Interesting that between Win 7 and macOS, the difference is consistently around 280. I wonder where that's coming from, could that be the height of the migration-wizard itself? Maybe removing the setpref that disables transitions is worth a try. Instead of disabling transitions, just wait for the transition to finish. Otherwise we could try just waiting for an arbitrary length of time.

Comment 18

[Negin]

(In reply to Shane Hughes [:aminomancer] from comment #17)

Ah, I should have looked more closely at it. We'll get to the bottom of it, thanks.

Interesting that between Win 7 and macOS, the difference is consistently around 280. I wonder where that's coming from, could that be the height of the migration-wizard itself? Maybe removing the setpref that disables transitions is worth a try. Instead of disabling transitions, just wait for the transition to finish. Otherwise we could try just waiting for an arbitrary length of time.

Sounds good, I'll give that a shot! (And my mistake didn't catch the perma failure either)

Comment 20

[Negin]

Perma failure for win7 should be addressed in the most recent patch update, sorry for the back and forth with this one!

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/67d785ab3cb3
Updates embedded migration wizard styles to match figma r=aminomancer

Comment 22

[Norisz Fay [:noriszfay]]

Backed out for causing node-newtab failure on aboutwelcome.scss

Backout link

Push with failures

Failure log

Comment 23

[Negin]

Sorry about that, linting failure should be fixed now.

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f1e19e0fa144
Updates embedded migration wizard styles to match figma r=aminomancer

Comment 25

[Cosmin Sabou [:CosminS]]

Backed out changeset f1e19e0fa144 (Bug 1829039) as requested by aminomancer.

Backout link: https://hg.mozilla.org/integration/autoland/rev/d91b5b3708383805f3997838c0093d4d221d034a

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/86f74f61adc0
Updates embedded migration wizard styles to match figma r=aminomancer

Comment 27

[Narcis Beleuzu [:NarcisB]]

Backed out for newtab failure on aboutwelcome.scss

Backout link: https://hg.mozilla.org/integration/autoland/rev/46592604836d07bdecebab1ac02301da24d56ba5
Log link: https://treeherder.mozilla.org/logviewer?job_id=415130268&repo=autoland&lineNumber=4326

Comment 28

[Negin]

So sorry about that, I did not push up the correct linting failure changes! This should hopefully be the last push.

Comment 29

[Negin]

Just want to callout that the macOS intermittent was not resolved in this patch that was filed in 1831974 and seems unrelated to this patch's changes.

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7de75b83935f
Updates embedded migration wizard styles to match figma r=aminomancer,mconley

Comment 31

[Negin]

[Tracking Requested - why for this release]:
This patch is required for embedded import wizard for new user onboarding flow in Fx114

Comment 32

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/7de75b83935f

Comment 33

[Negin]

Comment on attachment 9330050 [details]
Bug 1829039 - Updates embedded migration wizard styles to match figma

Beta/Release Uplift Approval Request

• User impact if declined: This adds necessary CSS styling for the embedded migration wizard, which is blocking its release for Fx114
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: Bug 1829043
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch introduces simple CSS changes and updates to the applicable browser test for the embedded migration wizard which is currently behind a pref
• String changes made/needed: No
• Is Android affected?: No

Comment 34

[Pascal Chevrel:pascalc]

Comment on attachment 9330050 [details]
Bug 1829039 - Updates embedded migration wizard styles to match figma

Approved for 114 beta 3, thanks.

Comment 35

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-beta/rev/997530f68e38

Comment 36

[Marius Coman [:mcoman], Ecosystem QA]

I have verified this enhancement using the "browser.migrate.content-modal.about-welcome-behavior" pref set to "embedded" and I can confirm the following:

• The Variant 1 of the Import flow from the Figma specification is displayed if the "browser.migrate.content-modal.enabled" pref is set to "true"
• The Variant 2 of the Import flow from the Figma specification is displayed if the "browser.migrate.content-modal.import-all.enabled" pref is set to "true"

Verified using the latest Firefox Nightly (115.0a1 Build ID - 20230512094914) and latest Firefox Beta (114.0b3 Build ID - 20230511191645) installed on Windows 10 x64, macOS 12.6.1, and Ubuntu 22.04 x64.

Comment 37

[Intermittent Failures Robot]

14 failures in 4077 pushes (0.003 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 13
• mozilla-central: 1

Platform and build breakdown:

• macosx1015-64-qr: 13
  • debug: 7
  • opt: 6
• windows11-64-2009-ccov-qr: 1
  • opt: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1829039&startday=2023-05-08&endday=2023-05-14&tree=all