Bug 173572

Summary:

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

Product:

WebKit

Reporter:

Carlos Alberto Lopez Perez <clopez>

Component:

WebKitGTK

Assignee:

Carlos Alberto Lopez Perez <clopez>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

beau.adkins, bugs-noreply, buildbot, cgarcia, mcatanzaro, rniwa, simon.fraser, thorton, zalan

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

174395, 175067

Bug Blocks:

Attachments:

Description	Flags
Patch	none
Patch	none
Archive of layout-test-results from ews103 for mac-elcapitan	none
Archive of layout-test-results from ews104 for mac-elcapitan-wk2	none
Archive of layout-test-results from ews126 for ios-simulator-wk2	none
Archive of layout-test-results from ews116 for mac-elcapitan	none
Patch	none
Patch	none
Archive of layout-test-results from ews102 for mac-elcapitan	none
Archive of layout-test-results from ews121 for ios-simulator-wk2	none
Archive of layout-test-results from ews107 for mac-elcapitan-wk2	none
Archive of layout-test-results from ews114 for mac-elcapitan	none
Patch	none
Patch	cgarcia: review+

Carlos Alberto Lopez Perez

Reported 2017-06-19 17:36:50 PDT

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 ?

Attachments
Patch (5.51 KB, patch) 2017-06-20 11:05 PDT, Carlos Alberto Lopez Perez	no flags	Details Formatted Diff Diff
Patch (41.47 KB, patch) 2017-06-22 07:30 PDT, Carlos Alberto Lopez Perez	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews103 for mac-elcapitan (2.17 MB, application/zip) 2017-06-22 08:04 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 (1.38 MB, application/zip) 2017-06-22 08:09 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews126 for ios-simulator-wk2 (1007.07 KB, application/zip) 2017-06-22 08:57 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews116 for mac-elcapitan (2.79 MB, application/zip) 2017-06-22 08:58 PDT, Build Bot	no flags	Details
Patch (151.18 KB, patch) 2017-06-28 11:36 PDT, Carlos Alberto Lopez Perez	no flags	Details Formatted Diff Diff
Patch (84.18 KB, patch) 2017-07-05 11:09 PDT, Carlos Alberto Lopez Perez	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews102 for mac-elcapitan (1.20 MB, application/zip) 2017-07-05 11:57 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews121 for ios-simulator-wk2 (1.19 MB, application/zip) 2017-07-05 12:15 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 (1.46 MB, application/zip) 2017-07-05 12:15 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews114 for mac-elcapitan (2.03 MB, application/zip) 2017-07-05 13:14 PDT, Build Bot	no flags	Details
Patch (49.67 KB, patch) 2017-07-10 20:24 PDT, Carlos Alberto Lopez Perez	no flags	Details Formatted Diff Diff
Patch (56.11 KB, patch) 2017-07-11 07:02 PDT, Carlos Alberto Lopez Perez	cgarcia: review+	Details Formatted Diff Diff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

Carlos Alberto Lopez Perez

Comment 1 2017-06-19 17:39:24 PDT

(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

Comment 2 2017-06-20 11:05:40 PDT

Created attachment 313415 [details] Patch initial patch: test EWS

Carlos Garcia Campos

Comment 3 2017-06-20 22:43:16 PDT

LGTM

Carlos Alberto Lopez Perez

Comment 4 2017-06-22 07:30:18 PDT

Created attachment 313616 [details] Patch add a test: test EWS again

Build Bot

Comment 5 2017-06-22 08:04:02 PDT

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

Comment 6 2017-06-22 08:04:04 PDT

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 7 2017-06-22 08:09:58 PDT

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

Comment 8 2017-06-22 08:09:59 PDT

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 9 2017-06-22 08:57:18 PDT

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

Comment 10 2017-06-22 08:57:19 PDT

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 11 2017-06-22 08:58:22 PDT

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

Comment 12 2017-06-22 08:58:23 PDT

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

Comment 13 2017-06-23 16:48:51 PDT

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 14 2017-06-23 16:51:20 PDT

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

Comment 15 2017-06-28 11:36:36 PDT

Created attachment 314044 [details] Patch patch v2. No asking for review. Test EWS

Carlos Alberto Lopez Perez

Comment 16 2017-07-05 11:09:13 PDT

Created attachment 314623 [details] Patch patch v3. No asking for review. Test EWS

Build Bot

Comment 17 2017-07-05 11:57:37 PDT

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

Comment 18 2017-07-05 11:57:38 PDT

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 19 2017-07-05 12:15:22 PDT

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

Comment 20 2017-07-05 12:15:24 PDT

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 21 2017-07-05 12:15:31 PDT

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

Comment 22 2017-07-05 12:15:33 PDT

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 23 2017-07-05 13:14:12 PDT

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

Comment 24 2017-07-05 13:14:13 PDT

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

Comment 25 2017-07-10 20:24:09 PDT

Created attachment 315068 [details] Patch Test EWS

Carlos Alberto Lopez Perez

Comment 26 2017-07-11 07:02:34 PDT

Created attachment 315106 [details] Patch patch: asking for review now

Carlos Garcia Campos

Comment 27 2017-07-11 08:05:00 PDT

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

Comment 28 2017-07-11 08:20:21 PDT

(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

Comment 29 2017-07-11 08:27:51 PDT

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

Comment 30 2017-07-11 08:34:50 PDT

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

Beau Adkins

Comment 31 2018-10-08 13:27:21 PDT

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

Comment 32 2018-10-09 12:13:24 PDT

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

Note You need to log in before you can comment on or make changes to this bug.