1791135 - Remove unnecessary rules from aboutwelcome.scss

Status: RESOLVED FIXED

(Firefox :: Messaging System, task, P2)

Description

[Shane Hughes [:aminomancer]]

We moved these rules for bug 1791088, but many of them are unnecessary.

• --newtab-button-secondary-color is definitely not needed,
• font-family doesn't seem to do anything on Windows 10. I haven't tested on every platform, but it looks like it was migrated over in bug 1618348 (the equivalent newtab rule having formerly been imported by aboutwelcome.scss). This commit is as far back as I could trace it. It looks like it's only needed on the new tab page because that doesn't get :root{font:message-box} from common.css (which is not the case for any users of aboutwelcome.css).
• font-size seems to have an extremely tiny (like, 1px) effect on layout of some surfaces, probably by changing the computed line-height for elements that don't have a fixed font-size,
• and position doesn't seem to have any effect anywhere. It was also added with bug 1618348, but I can't identify where it was migrated from (or if it was added anew then).

Comment 1

[Russell Chan]

Attachment: Bug 1791135 - Remove unnecessary rules from aboutwelcome.scss

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f0e6497690e9
Remove unnecessary rules from aboutwelcome.scss r=omc-reviewers,emcminn

Comment 3

[Sandor Molnar[:smolnar]]

Backed out for causing bc failures in browser/components/newtab/test/browser/browser_feature_callout_in_chrome.js

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

Push with failures

Failure log: TEST-UNEXPECTED-FAIL | browser/components/newtab/test/browser/browser_feature_callout_in_chrome.js | Callout custom position is rendered appropriately in RTL mode

Comment 4

[Shane Hughes [:aminomancer]]

These are the kind of tests you usually wanna run before pushing, and do mach try auto first as well. (an approving review doesn't necessarily mean you can skip testing, it just means the reviewer thinks the code looks good, but you'll still need testing to confirm or else the patch may be backed out) position: relative probably is required for feature callout RTL positioning. So you should add it to _feature-callout.scss or just don't remove it. Probably better to just not remove it, because a lot has changed to aboutwelcome positioning since this bug was initially filed.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f6dd294fc54e
Remove unnecessary rules from aboutwelcome.scss r=omc-reviewers,emcminn

Comment 6

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/f6dd294fc54e