1633701 - Checkbox and text for [Prevent this page from creating additional dialogs] are not vertically aligned on MacOS

Status: NEW

(Toolkit :: Themes, defect, P3)

Description

[Cristian Fogel, QA [:cfogel]]

Attachment: Screenshot 0002-04-28 at 13.55.57.png

Affected versions

• 75.0, 75.0b7, 60.9esr, 77.0a1

Affected platforms

• macOS 10.15.3, Windows 10,

Steps to reproduce

1. Launch Firefox, access this link;
2. For the age-input prompt click to run;
3. Input any age-value, click on Ok

Expected result

• elements in modal are properly displayed;

Actual result

• checkbox is lower

Regression range

• will check if one asap, but at first glimpse looks like something old;

Additional notes

• attached screenshot with the issue;
• Ubuntu 18 Is not affected.

Comment 1

[abodenlosz]

Regression range:

• First bad: 2020-04-23
• Last good: 2020-04-22
• Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=961700e5d48a292ca8d15f974729946227374359&tochange=0aa77ee04caae3d4081bbc992f2d962d3c9ec449
• Possible regressor: https://bugzilla.mozilla.org/show_bug.cgi?id=1583696

Comment 2

[Matthew N. [:MattN]]

Is this fixed now that that was backed out?

Comment 3

[Cristian Fogel, QA [:cfogel]]

Same issue when checking with 78.0a1 (2020-05-04).

Comment 4

[Matthew N. [:MattN]]

Hmm… then maybe the regression range is wrong?

Comment 5

[Julien Cristau [:jcristau]]

Cristian, can you double check the regression range?

Comment 6

[Julien Cristau [:jcristau]]

Nevermind, comment 0 says 60.9 is affected, so definitely not bug 1583696.

Comment 7

[Firefox Bug Husbandry Bot]

Because this bug's Severity has not been changed from the default since it was filed, and it's Priority is -- (non,) indicating it has has not been previously triaged, the bug's Severity is being updated to -- (default, untriaged.)

Comment 8

[Release mgmt bot [:sylvestre / :calixte / :marco for bugbug]]

The priority flag is not set for this bug.
:MattN, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 9

[f94olivera]

Hi everyone! just created my account and was willing to help...
Is there any chance that I could be assigned this ticket?

Comment 10

[Matthew N. [:MattN]]

Sure thing, request information from the mentor if you need help (see the checkbox below the comment box)

Comment 11

[f94olivera]

Ok that was fast hahaha
Thanks!

Comment 12

[f94olivera]

Hi! I'm your mentee, was wondering if you could point me in the right direction...
I'd need the source code cause I can't seem to find it anywhere

Comment 13

[f94olivera]

(In reply to f94olivera from comment #12)

Hi! I'm your mentee, was wondering if you could point me in the right direction...
I'd need the source code cause I can't seem to find it anywhere

I think I found it, is this one by any chance?

This site ==> https://firefox-source-docs.mozilla.org/contributing/how_to_contribute_firefox.html
points to this ==> https://hg.mozilla.org/mozilla-central/

I've just started setting up my local environment

Comment 14

[Sam Foster [:sfoster] (he/him)]

(In reply to f94olivera from comment #13)

This site ==> https://firefox-source-docs.mozilla.org/contributing/how_to_contribute_firefox.html
points to this ==> https://hg.mozilla.org/mozilla-central/

Yeah that's the best place to start.
When you have a local build, try using the browser toolbox to inspect the dialog that has the bug. You should be able to find the CSS and markup that way. Note, you only need a "artifact build"[2], as this is a front-end bug.

1. https://developer.mozilla.org/en-US/docs/Tools/Browser_Toolbox
2. https://firefox-source-docs.mozilla.org/contributing/build/artifact_builds.html

Comment 15

[f94olivera]

Attachment: Bug 1633701 - Removed margin top and bottom from checkbox. r?sfoster

Comment 16

[f94olivera]

I think I've done it... Could you check it Sam?

By the way, did I referenced you correctly? cause I've realised after I commited that the previous one reference the reviewer like this "r=sfoster" instead of how I did which was "r?sfoster". I followed the instructions of this site

https://firefox-source-docs.mozilla.org/contributing/how_to_contribute_firefox.html#to-write-a-patch

Comment 17

[Sam Foster [:sfoster] (he/him)]

(In reply to f94olivera from comment #16)

I think I've done it... Could you check it Sam?

By the way, did I referenced you correctly? cause I've realised after I commited that the previous one reference the reviewer like this "r=sfoster" instead of how I did which was "r?sfoster".

Yeah, the patch came through to me as needing review, all that worked out. I've left a comment and request for changes on the patch. It looks like the windows version of this rule[1] does not already have margin-top/bottom so the same fix will not work there. If you are able to look at that and come up with a fix, that would be great. If not, I think I'll adjust this bug to be for the OSX rendering only and open a new one for the windows issue.

1. https://searchfox.org/mozilla-central/source/toolkit/themes/windows/global/checkbox.css#52-58

Comment 18

[f94olivera]

Yeah, the patch came through to me as needing review, all that worked out. I've left a comment and request for changes on the patch. It looks like the windows version of this rule[1] does not already have margin-top/bottom so the same fix will not work there. If you are able to look at that and come up with a fix, that would be great. If not, I think I'll adjust this bug to be for the OSX rendering only and open a new one for the windows issue.

I'm sorry, I've tried setting up my windows environment with no success, even though I'll keep an eye out for the new one you'll open, could you adjust this one so that is for MacOS only, please?

Comment 19

[Sam Foster [:sfoster] (he/him)]

:f94olivera, do you plan on updating this patch? If not I'll unassign you and someone else can take it.

Comment 20

[f94olivera]

Hi! I'm sorry but I've just switched jobs and had to return the Macbook so I won't be able to solve it since in my new job I'm expecting an HP laptop.
I'm sorry again
Cheers

Comment 21

[Clara Guerrero]

Hi, please let me know if you still need to find a regression range for this given comment comment 5

Best,
Clara

Comment 22

[Julien Cristau [:jcristau]]

Probably not needed at this point.

Comment 23

[Sanna Frese]

Hi! I'd love to take this on!

We have the opportunity to contribute to OS projects full time for a week at my job this week, and this seems like a good entry level cotribution for me. Could I be assigned?

Comment 24

[Sam Foster [:sfoster] (he/him)]

(In reply to Sanna Frese from comment #23)

Hi! I'd love to take this on!

We have the opportunity to contribute to OS projects full time for a week at my job this week, and this seems like a good entry level cotribution for me. Could I be assigned?

Apologies your message got lost in my inbox Sanna, I'm just seeing it now. Let me know if you are still interested in taking this on - though I realize that window has passed. You can check the "Request information from" box and select "mentor" to ensure the mentor on a bug is notified.

Comment 25

[Mathew Hodson]

Bug 1583696 landed with some style changes to the prompt. Is this fixed now?

Comment 26

[Cristian Fogel, QA [:cfogel]]

Did a quick check on macOS 10.15/Windows 10 with 85.0a1 (2020-11-19) and this issue still persists.

Comment 27

[Mathew Hodson]

I think this only affects macOS. The alignment was checked for Windows in bug 1641080, and it was reported working.

Comment 28

[Cristian Fogel, QA [:cfogel]]

Must have been a wrong build on my Windows machine.
Standing corrected, indeed on Windows it is fixed./

Checked and it's the same for both macOS 10.15 as well as 11.0.1.

Comment 29

[Kartik Gautam]

Hi,

Can I take this up. I am new here. And it will be my first contribution. I have setup the build in my mac running on OS11. I think this issue will be good for a newbie like me.
Please let me know, and also it will be great if you can give me a little hint about how to fix this.

Thanks,
Kartik Gautam

Comment 30

[Sam Foster [:sfoster] (he/him)]

(In reply to Kartik Gautam from comment #29)

Please let me know, and also it will be great if you can give me a little hint about how to fix this.

Hi there, I'm not sure what the correct fix for this bug is without doing some investigation. Bug 1583696 landed a new non-XUL-layout impementation of the dialog, which fixed the same issue observed in Windows. But on MacOS, the checkbox has different margins. It could be there is some specific workaround that needs to be removed. A previous incomplete patch for this identified and wanted to remove the top/bottom margin on all checkboxes for the MacOS platform. That's obviously something we should be extremely careful about if that turns out to be the root cause, as it could easily fix 1 problem and break 100 others. So I'm not 100% sure this is actually a good-first-bug any more, but you are welcome to investigate and propose a patch. See my suggestions in comment 14 for how to get started - you'll be using the browser toolbox alongside your study of that area of the codebase to understand the markup and CSS involved.

Clearing old need-info for Sanna/comment 24.

Comment 31

[Kartik Gautam]

(In reply to Sam Foster [:sfoster] (PTO thru' Jan 4th) (he/him) from comment #30)

(In reply to Kartik Gautam from comment #29)

Please let me know, and also it will be great if you can give me a little hint about how to fix this.

Hi there, I'm not sure what the correct fix for this bug is without doing some investigation. Bug 1583696 landed a new non-XUL-layout impementation of the dialog, which fixed the same issue observed in Windows. But on MacOS, the checkbox has different margins. It could be there is some specific workaround that needs to be removed. A previous incomplete patch for this identified and wanted to remove the top/bottom margin on all checkboxes for the MacOS platform. That's obviously something we should be extremely careful about if that turns out to be the root cause, as it could easily fix 1 problem and break 100 others.

Okay, actually i also thought of removing top and bottom margin, so i think this bug is not for a newbie like me, Thank You

Comment 32

[Dão Gottwald [::dao]]

Attachment: Bug 1633701 - Remove top margin from checkboxes on macOS and stop setting vertical-align. r=sfoster

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/416cce562ac2
Remove top margin from checkboxes on macOS and stop setting vertical-align. r=sfoster

Comment 34

[Natalia Csoregi [:nataliaCs]]

Backed out for fails on checkboxsize.xhtml

backout: https://hg.mozilla.org/integration/autoland/rev/8d91b346e6b5f5efbe347521f8c1bbcd5c10c88a

push: https://treeherder.mozilla.org/jobs?repo=autoland&revision=416cce562ac233753218454e22d1ff0e1e061815&group_state=expanded

failures firstly seen on a later push: https://treeherder.mozilla.org/jobs?repo=autoland&searchStr=os%2Cx%2C10.15%2Cwebrender%2Cdebug%2Creftests%2Ctest-macosx1015-64-qr%2Fdebug-reftest-e10s%2Cr4&revision=a119c6860f429847367a87405cf791da5110ef19

failure log: https://treeherder.mozilla.org/logviewer?job_id=337227649&repo=autoland&lineNumber=25213

[task 2021-04-20T23:20:01.259Z] 23:20:01 INFO - [Parent 1924, Main Thread] WARNING: Someone asked nsILookAndFeel for a color I don't know about: file /builds/worker/checkouts/gecko/widget/cocoa/nsLookAndFeel.mm:374
[task 2021-04-20T23:20:01.260Z] 23:20:01 INFO - [Parent 1924, Main Thread] WARNING: Someone asked nsILookAndFeel for a color I don't know about: file /builds/worker/checkouts/gecko/widget/cocoa/nsLookAndFeel.mm:374
[task 2021-04-20T23:20:01.262Z] 23:20:01 INFO - [Parent 1924, Main Thread] WARNING: Someone asked nsILookAndFeel for a color I don't know about: file /builds/worker/checkouts/gecko/widget/cocoa/nsLookAndFeel.mm:374
[task 2021-04-20T23:20:01.263Z] 23:20:01 INFO - [Parent 1924, Main Thread] WARNING: Someone asked nsILookAndFeel for a color I don't know about: file /builds/worker/checkouts/gecko/widget/cocoa/nsLookAndFeel.mm:374
[task 2021-04-20T23:20:01.310Z] 23:20:01 INFO - REFTEST TEST-UNEXPECTED-FAIL | toolkit/themes/osx/reftests/checkboxsize.xhtml == toolkit/themes/osx/reftests/checkboxsize-ref.xhtml | image comparison, max difference: 218, number of differing pixels: 760

Comment 35

[Dão Gottwald [::dao]]

Actually I think this may not be an issue anymore with Proton except for native checkboxes (checkbox[native]).