1437284 - Spinbuttons in in-content pages looking weird

Status: RESOLVED INVALID

(Toolkit :: Themes, defect, P2)

Description

[Richard Marti (:Paenglab)]

Attachment: 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.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: spinbuttons.png

On top the issue on Windows, on bottom with background-position: center;. On Windows, the button border overlaps the textbox border.

Comment 2

[Richard Marti (:Paenglab)]

Attachment: Bug1437284.patch

I don't know why Windows is different with the padding.

Comment 3

[Dão Gottwald [::dao]]

(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.

Comment 4

[Richard Marti (:Paenglab)]

Attachment: Bug1437284.patch

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.

Comment 5

[Dão Gottwald [::dao]]

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.

Comment 6

[Dão Gottwald [::dao]]

> >--- 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?

Comment 7

[Richard Marti (:Paenglab)]

(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.

Comment 8

[Richard Marti (:Paenglab)]

Attachment: Bug1437284.patch

Okay, this should do it.

Comment 9

[Dão Gottwald [::dao]]

(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?

Comment 10

[Richard Marti (:Paenglab)]

Attachment: Bug1437284.patch

Comment 11

[Dão Gottwald [::dao]]

Comment on attachment 8951742 [details] [diff] [review]
Bug1437284.patch

Thanks!

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/32fd01f00b99
Fix the spinbuttons in in-content pages. r=dao

Comment 13

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/32fd01f00b99

Comment 14

[Oana Botisan, Desktop Release QA]

Attachment: port - buttons not centered.png

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.

Comment 15

[Richard Marti (:Paenglab)]

Attachment: spinbuttons with patch.png

Hmm, I don't see a problem here with Nightly 61.a1 on Linux Mint and Windows 10.

Comment 16

[Oana Botisan, Desktop Release QA]

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.

Comment 17

[Oana Botisan, Desktop Release QA]

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.

Comment 18

[Richard Marti (:Paenglab)]

Bug 1437302 removed the spinbuttons. When you want to check the buttons, you need to try a build from before 2018.04.11.

Comment 19

[Oana Botisan, Desktop Release QA]

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.