Bug 173572 – [GTK] Spin buttons on input type number appear over the value itself for small widths

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Carlos Alberto Lopez Perez]

On https://browserperfdash.igalia.com/ the input type number to select the number of rows or days (the first and the last inputs over the tables) don't show correctly on WebKitGTK+.

The GtkSpinButtons drawn to allow adjusting the values appear over the value of the input itself.


This is easily reproducible with something like:

What is your age? <input type="number" style="width: 50px;">


Not sure what should be the right fix here .... 

Maybe we should not draw the spin buttons when the size of the input element is too small to accommodate both the input values and the spin button itself ?

[Carlos Alberto Lopez Perez]

(In reply to Carlos Alberto Lopez Perez from comment #0)
> 
> This is easily reproducible with something like:
> 
> What is your age? <input type="number" style="width: 50px;">
> 

Quick example: https://people.igalia.com/clopez/wkbug/173572/

[Carlos Alberto Lopez Perez]

Created attachment 313415 [details]
Patch

initial patch: test EWS

[Carlos Garcia Campos]

LGTM

[Carlos Alberto Lopez Perez]

Created attachment 313616 [details]
Patch

add a test: test EWS again

[Build Bot]

Comment on attachment 313616 [details]
Patch

Attachment 313616 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3978163

New failing tests:
fast/forms/number/number-minimum-size-spinbutton-not-clover.html

[Build Bot]

Created attachment 313623 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 313616 [details]
Patch

Attachment 313616 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3978185

New failing tests:
fast/forms/number/number-minimum-size-spinbutton-not-clover.html

[Build Bot]

Created attachment 313625 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 313616 [details]
Patch

Attachment 313616 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3978289

New failing tests:
fast/forms/number/number-minimum-size-spinbutton-not-clover.html

[Build Bot]

Created attachment 313631 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[Build Bot]

Comment on attachment 313616 [details]
Patch

Attachment 313616 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3978274

New failing tests:
fast/forms/number/number-minimum-size-spinbutton-not-clover.html

[Build Bot]

Created attachment 313632 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Michael Catanzaro]

Yeah, I'm not sure if it's caused by a change in Adwaita or a change in RenderThemeGtk, but it's a recent regression. We already have another bug report about this, but I have not managed to find it after a quick search.

[Michael Catanzaro]

Comment on attachment 313616 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=313616&action=review

Nice layout test!

The 4 and 5 combo boxes don't look great, but it's surely a lot better than before.

> LayoutTests/fast/forms/number/number-minimum-size-spinbutton-not-clover.html:5
> +<p>Test that spin buttons don't clover the value of input type numbers.

clover -> cover

[Carlos Alberto Lopez Perez]

Created attachment 314044 [details]
Patch

patch v2. No asking for review. Test EWS

[Carlos Alberto Lopez Perez]

Created attachment 314623 [details]
Patch

patch v3. No asking for review. Test EWS

[Build Bot]

Comment on attachment 314623 [details]
Patch

Attachment 314623 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/4057691

New failing tests:
fast/forms/number/number-minimum-size-spinbutton-not-clover-controled-style.html
fast/forms/number/number-minimum-size-spinbutton-not-clover.html

[Build Bot]

Created attachment 314630 [details]
Archive of layout-test-results from ews102 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 314623 [details]
Patch

Attachment 314623 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4057679

New failing tests:
fast/forms/number/number-minimum-size-spinbutton-not-clover-controled-style.html
fast/forms/number/number-minimum-size-spinbutton-not-clover.html

[Build Bot]

Created attachment 314635 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[Build Bot]

Comment on attachment 314623 [details]
Patch

Attachment 314623 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4057734

New failing tests:
fast/forms/number/number-minimum-size-spinbutton-not-clover-controled-style.html
fast/forms/number/number-minimum-size-spinbutton-not-clover.html

[Build Bot]

Created attachment 314636 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 314623 [details]
Patch

Attachment 314623 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4057989

New failing tests:
fast/forms/number/number-minimum-size-spinbutton-not-clover-controled-style.html
fast/forms/number/number-minimum-size-spinbutton-not-clover.html

[Build Bot]

Created attachment 314646 [details]
Archive of layout-test-results from ews114 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Carlos Alberto Lopez Perez]

Created attachment 315068 [details]
Patch

Test EWS

[Carlos Alberto Lopez Perez]

Created attachment 315106 [details]
Patch

patch: asking for review now

[Carlos Garcia Campos]

Comment on attachment 315106 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=315106&action=review

> Source/WebCore/ChangeLog:10
> +        This ensures that we don't end clovering the input values with

I'm not an English expert at all, but I think here we should say "end up"

> Source/WebCore/rendering/RenderThemeGtk.cpp:939
> +    // Input field need a minimum dimensions to render the spinbuttons correctly (without clovering the values).

need a minimum dimensions -> need minimum dimensions

> Source/WebCore/rendering/RenderThemeGtk.cpp:957
> +    // renders on WebKitGTK+. To ensure that spin buttons don't end clovering the values of the input

Same comment here about "end up"

[Carlos Alberto Lopez Perez]

(In reply to Carlos Garcia Campos from comment #27)
> Comment on attachment 315106 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=315106&action=review
> 
> > Source/WebCore/ChangeLog:10
> > +        This ensures that we don't end clovering the input values with
> 
> I'm not an English expert at all, but I think here we should say "end up"
> 
> > Source/WebCore/rendering/RenderThemeGtk.cpp:939
> > +    // Input field need a minimum dimensions to render the spinbuttons correctly (without clovering the values).
> 
> need a minimum dimensions -> need minimum dimensions
> 
> > Source/WebCore/rendering/RenderThemeGtk.cpp:957
> > +    // renders on WebKitGTK+. To ensure that spin buttons don't end clovering the values of the input
> 
> Same comment here about "end up"


Right... 

I'm also not sure from where I got the word "clover / clovering" but I think it should be saying "cover / covering" instead. I will fix it.

[Michael Catanzaro]

This is a very important fix. Thanks!

(In reply to Carlos Garcia Campos from comment #27)
> > Source/WebCore/ChangeLog:10
> > +        This ensures that we don't end clovering the input values with
> 
> I'm not an English expert at all, but I think here we should say "end up"

Yes

(In reply to Carlos Alberto Lopez Perez from comment #28) 
> I'm also not sure from where I got the word "clover / clovering" but I think
> it should be saying "cover / covering" instead. I will fix it.

Yes. See also: https://en.wikipedia.org/wiki/Clover :)

[Carlos Alberto Lopez Perez]

Committed r219332: <http://trac.webkit.org/changeset/219332>

[Beau Adkins]

This caused a display regression on http://www.paradiso-design.net/ntsc_pal_conversion_faq.html

This is strange because this site doesn't even have spinner buttons at all. Why is this code growing the text entry field if it doesn't have spin buttons?

Probably a better long term solution is to use a spin button style that more closely matches other browsers. The default GTK side-by-side buttons is just way too wide. Any chance this is in the works?

[Michael Catanzaro]

We should try fixing this in bug #175067, following the solution proposed in comment 11 there.