Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(30)

Issue 230193002: Spin button in input[type=number] should not ignore paddings. (Closed)

Created:
6 years, 8 months ago by tkent
Modified:
6 years, 8 months ago
Reviewers:
keishi
CC:
blink-reviews, zoltan1, dsinclair, bemjb+rendering_chromium.org, eae+blinkwatch, ed+blinkwatch_opera.com, leviw+renderwatch, dglazkov+blink, apavlov+blink_chromium.org, jchaffraix+rendering, darktears, pdr., rune+blink, rwlbuis
Visibility:
Public.

Description

Spin button in input[type=number] should not ignore paddings. The old behavior was to emulate Windows spin button appearance. Now we don't use Windows-native controls and don't need to emulate it. Implementation: This CL removes special layout code for spin buttons in RenderTextControlSingleLine, and the spin button layout relies on CSS flexible box. As for Mac, we can't stretch NSStepper to arbitrary sizes. We need to apply align-item:center. Also, adjust control size threshold because spin buttons need to fit into the font height. Test: * We need to do rebaseline for some layout tests. * This CL removes fast/forms/number/number-large-padding.html, which made sense for the special layout. * Add font-size to number-spinbutton-in-multi-column.html because OSX spin button height depends on font size, and we don't stretch it. BUG=128957, 330117, 356939 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=171315

Patch Set 1 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+111 lines, -160 lines) Patch
M LayoutTests/TestExpectations View 2 chunks +6 lines, -2 lines 0 comments Download
D LayoutTests/fast/forms/number/number-large-padding.html View 1 chunk +0 lines, -29 lines 0 comments Download
D LayoutTests/fast/forms/number/number-large-padding-expected.txt View 1 chunk +0 lines, -10 lines 0 comments Download
M LayoutTests/fast/forms/number/number-spinbutton-in-multi-column.html View 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/platform/win/fast/forms/number/number-appearance-datalist-expected.png View Binary file 0 comments Download
M LayoutTests/platform/win/fast/forms/number/number-appearance-datalist-expected.txt View 2 chunks +6 lines, -8 lines 0 comments Download
M LayoutTests/platform/win/fast/forms/number/number-appearance-rtl-expected.png View Binary file 0 comments Download
M LayoutTests/platform/win/fast/forms/number/number-appearance-rtl-expected.txt View 2 chunks +15 lines, -16 lines 0 comments Download
M LayoutTests/platform/win/fast/forms/number/number-appearance-spinbutton-disabled-readonly-expected.png View Binary file 0 comments Download
M LayoutTests/platform/win/fast/forms/number/number-appearance-spinbutton-disabled-readonly-expected.txt View 2 chunks +5 lines, -6 lines 0 comments Download
M LayoutTests/platform/win/fast/forms/number/number-appearance-spinbutton-layer-expected.txt View 1 chunk +4 lines, -4 lines 0 comments Download
M LayoutTests/platform/win/fast/speech/input-appearance-numberandspeech-expected.txt View 1 chunk +64 lines, -64 lines 0 comments Download
M Source/core/css/html.css View 2 chunks +4 lines, -9 lines 0 comments Download
M Source/core/css/themeMac.css View 1 chunk +4 lines, -0 lines 0 comments Download
M Source/core/rendering/RenderTextControlSingleLine.cpp View 1 chunk +0 lines, -10 lines 0 comments Download
M Source/platform/mac/ThemeMac.mm View 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
tkent
Keishi, would you review this please?
6 years, 8 months ago (2014-04-10 03:04:48 UTC) #1
keishi
lgtm
6 years, 8 months ago (2014-04-11 05:02:43 UTC) #2
tkent
The CQ bit was checked by tkent@chromium.org
6 years, 8 months ago (2014-04-11 05:09:23 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tkent@chromium.org/230193002/60001
6 years, 8 months ago (2014-04-11 05:09:25 UTC) #4
commit-bot: I haz the power
6 years, 8 months ago (2014-04-11 05:24:43 UTC) #5
Message was sent while issue was closed.
Change committed as 171315

Powered by Google App Engine
This is Rietveld 408576698