[MacOS] "Show you how" text is not centered inside the button
Categories
(Firefox :: General, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr78 | --- | unaffected |
firefox-esr91 | --- | unaffected |
firefox93 | --- | unaffected |
firefox94 | --- | unaffected |
firefox95 | --- | verified |
firefox96 | --- | verified |
People
(Reporter: snegritas, Assigned: sfoster)
References
(Regression)
Details
(Keywords: regression)
Attachments
(3 files)
58.22 KB,
image/png
|
Details | |
48 bytes,
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details | Review |
32.03 KB,
image/png
|
Details |
Affected versions
- 95.0a1 (2021-10-24)
Affected platforms
- MacOS 10.15
Steps to reproduce
- Open Firefox with a new profile.
- Open multiple tabs.
- Quit firefox.
- Reopen firefox with the profile from step 1.
Expected result
- Infobar is shown and the "Show you how" text is aligned in the center of the button.
Actual Result
- Infobar is shown and the "Show you how" text is not aligned in the center of the button.
Regression range
-
last good build: https://hg.mozilla.org/mozilla-central/rev/ef7b596bb385f46280eb1492009bf8fb541e17db
-
first bad build: https://hg.mozilla.org/mozilla-central/rev/3cf700ab977a298e19a00f1ec66d14939766955b
-
Here is the regression range: "https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=ef7b596bb385f46280eb1492009bf8fb541e17db&tochange=3cf700ab977a298e19a00f1ec66d14939766955b"
Additional notes
- Only the nightly version is affected by this issue.
Reporter | ||
Updated•3 years ago
|
Reporter | ||
Updated•3 years ago
|
Reporter | ||
Updated•3 years ago
|
Updated•3 years ago
|
Comment 1•3 years ago
|
||
Are we sure this is about one of my patches? The pushlog in comment 0 has a lot of other patches that are more likely culprits...
ni?ing sfoster too since he might know what's going on right away, I'm not particularly familiar with this infobox.
Assignee | ||
Comment 2•3 years ago
|
||
Well I think we can rule out Gijs' changes in Bug 1734246 as that patch literally just updated the string. I don't know off the top of my head what's going on here, and it seems odd that only this button would be affected if it was a layout change. Does this reproduce for any of the other infobars? I can investigate, but my first step would be to keep bisecting that pushlog down to a shorter list of candidates.
It looks like there's a (pre-existing) rule for that button that sets some padding at: https://searchfox.org/mozilla-central/source/toolkit/content/widgets/message-bar.css#232-242, I'm not sure if that has significance here?
Comment 3•3 years ago
|
||
:sergiu.negritas, since this bug is a regression, could you fill (if possible) the regressed_by field?
For more information, please visit auto_nag documentation.
Comment 4•3 years ago
|
||
Yeah if I remove this declaration then it looks much better, fwiw.
Comment 5•3 years ago
|
||
Apologies, :emilio, looks like I mistakenly pinged you.
Assignee | ||
Comment 6•3 years ago
|
||
I had a go at getting mozregression to winnow down that pushlog to a likely culprit, but without success yet. From the front-end point of view I'm happy to just land a patch with the changes :emilio pointed out. However it seems like it might be useful for the Layout team to know if there was a regression that caused this to show up, so if anyone is able to pinpoint the regressor that would be great.
Comment 7•3 years ago
|
||
Sergiu, could you try and get a narrower regression range? Thanks
Reporter | ||
Comment 8•3 years ago
|
||
Hey Pascal! Sorry for the long reply time but unfortunately I could not find a narrower regression range unfortunately.
After I place the dates of the regression it just freezes on my mac and it doesn't download any new build.
Comment 9•3 years ago
|
||
:sfoster what is the status of the patch you were planning to land?
Assignee | ||
Comment 10•3 years ago
|
||
I can make a patch from :emilio's suggestion in comment #4. That does seem to fix the symptom, but leaves an open question as to why this needing fixing. But answering that question doesn't need to block us here.
Assignee | ||
Comment 11•2 years ago
|
||
Updated•2 years ago
|
Assignee | ||
Comment 12•2 years ago
|
||
This is from MacOS. Its the same on Linux (Ubuntu) and Windows. LGTM.
Comment 13•2 years ago
|
||
https://hg.mozilla.org/integration/autoland/rev/f3cf9c079ebb1ad2b5995bb4c99e299229b6e000 looks to be the cause when looking at that regression range.
Comment 14•2 years ago
•
|
||
So the fix is correct because before that patch the selector changed in comment 13 had more specificity, right?
Updated•2 years ago
|
Assignee | ||
Comment 15•2 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #14)
So the fix is correct because before that patch the selector changed in comment 13 had more specificity, right?
Yeah that's exactly right. Before Bug 1698349, the small button (button.small
) rule had higher specificity so we got the block padding from it rather than the .notification-button
rule in message-bar.css. So, I think the fix here is to add the button
back to this selector in common.inc.css. There's a possibility this change has already been worked-around elsewhere but I'm not seeing anything yet.
I'll update my patch. Thanks :jaws for spotting this.
Comment 16•2 years ago
|
||
I think that removing the padding is preferable? It never had an effect so it's just dead code.
Updated•2 years ago
|
Comment 17•2 years ago
|
||
Agree with emilio here :)
Comment 18•2 years ago
|
||
Set release status flags based on info from the regressing bug 1698349
Comment 19•2 years ago
|
||
Pushed by sfoster@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ca306f7eb991 Restore specificity for small buttons rule to fix infobar notification call-to-action buttons. r=jaws,desktop-theme-reviewers,dao
Assignee | ||
Comment 20•2 years ago
|
||
(In reply to Tim Nguyen :ntim from comment #17)
Agree with emilio here :)
Gah, sorry. I did phabricator then bugmail so I didn't see this until I pushed.
Comment 21•2 years ago
|
||
bugherder |
Comment 22•2 years ago
|
||
The patch landed in nightly and beta is affected.
:sfoster, is this bug important enough to require an uplift?
If not please set status_beta
to wontfix
.
For more information, please visit auto_nag documentation.
Assignee | ||
Comment 23•2 years ago
|
||
Comment on attachment 9251275 [details]
Bug 1737754 - Remove padding override for notification buttons. r?jaws
Beta/Release Uplift Approval Request
- User impact if declined: Text is misaligned in the "Restore session" infobar button.
- Is this code covered by automated tests?: No
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: Yes
- If yes, steps to reproduce: * Reset the
browser.startup.couldRestoreSession.count
pref to0
using either a new profile, or about:config. - Start Firefox, close it and start it again
- You should get a infobar prompt indicating how to enable session restore.
ER: The "Show me how" text on the button should be correctly aligned (vertically centered .)
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): The patch reverts a 1-line inadvertent CSS change which only applies to these infobar buttons
- String changes made/needed: None
Assignee | ||
Updated•2 years ago
|
Updated•2 years ago
|
Comment 24•2 years ago
|
||
Comment on attachment 9251275 [details]
Bug 1737754 - Remove padding override for notification buttons. r?jaws
Low risk, approved for our last 95 beta, thanks.
Comment 25•2 years ago
|
||
bugherder uplift |
Reporter | ||
Comment 26•2 years ago
|
||
Hello! I have managed to reproduce the issue with firefox 95.0a1(2021-10-24) on MacOS 10.15, I can confirm that the issue is fixed in firefox 96.0a1(2021-11-24).
I will check the beta fix this friday when the next beta release is scheduled.
Thank you
Reporter | ||
Comment 27•2 years ago
|
||
Hello! Since firefox 95.0b12 has been released today I can confirm that the issue is fixed with this version on macOS 10.15.
Marking this issue as VERIFIED->FIXED and I will update the corresponding flags accordingly.
Thank you!
Description
•