Closed Bug 1437284 Opened 2 years ago Closed 2 years ago

Spinbuttons in in-content pages looking weird

Categories

(Toolkit :: Themes, defect, P2)

defect

Tracking

()

RESOLVED INVALID
mozilla60
Tracking Status
firefox-esr52 --- unaffected
firefox58 --- unaffected
firefox59 --- unaffected
firefox60 --- fixed

People

(Reporter: Paenglab, Assigned: Paenglab)

References

Details

(Keywords: regression)

Attachments

(5 files, 3 obsolete files)

Attached image spinbuttons-Linux.png
The in bug 1429573 changed spinbuttons are looking weird in in-content pages like the prefs.

On Windows the arrow icons aren't centered in the buttons and on Mac on the right is a 10px gap to the border. Linux has both issues together.
Attached image spinbuttons.png
On top the issue on Windows, on bottom with background-position: center;. On Windows, the button border overlaps the textbox border.
Component: XUL Widgets → Themes
Keywords: regression
Priority: -- → P2
Attached patch Bug1437284.patch (obsolete) — Splinter Review
I don't know why Windows is different with the padding.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8949999 - Flags: review?(dao+bmo)
(In reply to Richard Marti (:Paenglab) from comment #2)
> Created attachment 8949999 [details] [diff] [review]
> Bug1437284.patch
> 
> I don't know why Windows is different with the padding.

It appears to be because of margin-inline-start here, although I don't know why it only has that effect on Windows:

https://searchfox.org/mozilla-central/rev/d03ad8843e3bf2e856126bc53b0475c595e5183b/toolkit/themes/shared/in-content/common.inc.css#361

Let's stop messing with that margin.
Attachment #8949999 - Flags: review?(dao+bmo)
Attached patch Bug1437284.patch (obsolete) — Splinter Review
Removing the margin-inline-start made it more consistent. Windows still needs some margin to the spinbuttons to have a gap to the input border.
Attachment #8949999 - Attachment is obsolete: true
Attachment #8951076 - Flags: review?(dao+bmo)
Comment on attachment 8951076 [details] [diff] [review]
Bug1437284.patch

>--- a/toolkit/themes/shared/in-content/common.inc.css
>+++ b/toolkit/themes/shared/in-content/common.inc.css

>+xul|textbox[type="number"] {
>+  padding-inline-end: 0;
>+}
>+
>+html|input[type="number"]::-moz-number-text {
>+  margin-inline-end: 10px;
>+}

Please move this after the rule that sets padding-left and padding-right on the textbox.

>--- a/toolkit/themes/windows/global/in-content/common.css
>+++ b/toolkit/themes/windows/global/in-content/common.css

>+html|input[type="number"]::-moz-number-spin-box {
>+  margin-inline-end: 3px;
>+}

I don't understand why this is needed... I mean, I read your comment, but I don't understand why Windows is different.
Attachment #8951076 - Flags: review?(dao+bmo) → review+
> >--- a/toolkit/themes/windows/global/in-content/common.css
> >+++ b/toolkit/themes/windows/global/in-content/common.css
> 
> >+html|input[type="number"]::-moz-number-spin-box {
> >+  margin-inline-end: 3px;
> >+}
> 
> I don't understand why this is needed... I mean, I read your comment, but I
> don't understand why Windows is different.

Maybe we need to stop setting any horizontal margin on ::-moz-number-spin-up and ::-moz-number-spin-down. With this patch it's still 1px on either side.

Also, existing rules use html|*.numberbox-input, you're patch uses html|input[type="number"]. Can you make this consistent?
(In reply to Dão Gottwald [::dao] from comment #6)
> 
> Maybe we need to stop setting any horizontal margin on ::-moz-number-spin-up
> and ::-moz-number-spin-down. With this patch it's still 1px on either side.
> 
> Also, existing rules use html|*.numberbox-input, you're patch uses
> html|input[type="number"]. Can you make this consistent?

This could work. Then I can in shared/common.css give to ::-moz-number-spin-box a margin-inline-end: 1px; and hopefully all platforms look the same.

I'll try it this evening.
Attached patch Bug1437284.patch (obsolete) — Splinter Review
Okay, this should do it.
Attachment #8951076 - Attachment is obsolete: true
Attachment #8951416 - Flags: review?(dao+bmo)
(In reply to Dão Gottwald [::dao] from comment #6)
> Also, existing rules use html|*.numberbox-input, you're patch uses
> html|input[type="number"]. Can you make this consistent?
Flags: needinfo?(richard.marti)
Attachment #8951416 - Flags: review?(dao+bmo)
Attached patch Bug1437284.patchSplinter Review
Attachment #8951416 - Attachment is obsolete: true
Flags: needinfo?(richard.marti)
Attachment #8951742 - Flags: review?(dao+bmo)
Comment on attachment 8951742 [details] [diff] [review]
Bug1437284.patch

Thanks!
Attachment #8951742 - Flags: review?(dao+bmo) → review+
Keywords: checkin-needed
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/32fd01f00b99
Fix the spinbuttons in in-content pages. r=dao
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/32fd01f00b99
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
I managed to reproduce the bug using an older version of Nightly 2018-02-10 on Windows 10 x64.
I retested everything on latest Nightly 61.0a1 and beta 60.0b5 using Windows 10 x64, Ubuntu 16.04 x64 and macOS 10.13 and the bug is not reproducing anymore. The arrow icons are centered on the buttons, but I noticed something else.

On Windows 10 x64 (Nightly 61.0a1) and on Ubuntu 16.04 x64 (both beta 60.0b5 and Nightly 61.0a1) the two buttons are not perfectly centered in the field. Please look at the attached image to see the issue. MacOS doesn't seem to be affected by this issue.
Flags: needinfo?(richard.marti)
Hmm, I don't see a problem here with Nightly 61.a1 on Linux Mint and Windows 10.
Flags: needinfo?(richard.marti)
I verified it again using latest Nightly and beta 60.0b6 on Windows 10 x64 and Ubuntu 16.04. The issue is not reproducing anymore.

On Ubuntu the buttons are not centered in the field if the browser is restarted. But if the browser is closed and opened again, that issue is not reproducing. Windows is not affected by this.
I've noticed that on the latest Nightly using Windows 10 x64, Ubuntu 16.04 x64 and macOS 10.13, the buttons are not present anymore in the "Connection Settings" pop-up in about:preferences. Is that expected behaviour? 

And if the answer is yes, can you please tell me where can I verify the fix for this bug?

Thank you.
Flags: needinfo?(richard.marti)
Bug 1437302 removed the spinbuttons. When you want to check the buttons, you need to try a build from before 2018.04.11.
Flags: needinfo?(richard.marti)
Considering the fact that the fix for bug 1437302 is already lifted in Firefox 60, I think that this bug is not applicable anymore. I will mark it as invalid.
Flags: qe-verify+
Resolution: FIXED → INVALID
You need to log in before you can comment on or make changes to this bug.