New issue
Advanced search Search tips

Issue 719737 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Input bullets are variable sizes

Reported by sara.rah...@collectivehealth.com, May 8 2017

Issue description

Chrome Version       : 58.0.3029.96
OS Version: OS X 10.11.6
URLs (if applicable) :https://codepen.io/anon/pen/LyejLr
Other browsers tested:
  Add OK or FAIL after other browsers where you have tested this issue:
     Safari 5: Fail
  Firefox 4.x: OK
     IE 7/8/9: Don't know

What steps will reproduce the problem?
1. Having an input with multiline text causes the input sizes to vary
2. Wrapping the input in a <span> tag fixes the variable size problem.
3.

What is the expected result?All inputs should be the same size. 


What happens instead of that?


Please provide any additional information below. Attach a screenshot if
possible.

UserAgentString: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.96 Safari/537.36



 
Components: Blink>Layout
Labels: -OS-Mac OS-All
Status: Untriaged (was: Unconfirmed)
I can repro on Mac and Linux w/Chrome 60 dev.

Comment 2 by e...@chromium.org, May 9 2017

Labels: -Pri-3 Pri-2
Owner: cbiesin...@chromium.org
Status: Assigned (was: Untriaged)
Christian, would you mind looking into this? The element itself seems to have the right size but the inner control does not.
Owner: chrishtr@chromium.org
Owner: cbiesin...@chromium.org
Christian figured out the bug. It's that the native controls theme code does not
set a minimum width equal to width, which means that the control can become smaller.

On Mac, there is a platform theme, which leads to I think this code getting called:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/mac/ThemeMac.mm?type=cs&q=MinimumControlSize&sq=package:chromium&l=434

On Linux, there is no platform theme, which leads to this code getting called:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/LayoutThemeDefault.cpp?q=layoutthemedefault&sq=package:chromium&dr=CSs&l=206

For radio buttons, I think should set a min-width and min-height equal to width and height.


Finally, the reason that wrapping in a <span> fixes the issue is that spans have a computed
"minimum content scale" that includes the sizes (*not* min-sizes) of children. The flex algorithm
takes into account this minimum content scale.
To elaborate on the last paragraph, we basically call ComputedStyle::SetWidth using the theme-provided width. When the radio button is a direct child we do not take the width property into account for the min-content width, but when it is an indirect descendant we do take the width into account for the flex item's min-width.
Status: Started (was: Assigned)
patch: https://chromium-review.googlesource.com/c/581753/
Cc: keishi@chromium.org cbiesin...@chromium.org chrishtr@chromium.org erikc...@chromium.org
Labels: OS-Mac
Owner: ----
OK, that patch will fix Linux/Windows/ChromeOS/Android. However, it does not fix Mac because I don't know the Mac code enough.

Fixing Mac will likely require changing ThemeMac::MinimumControlSize to return something reasonable for kRadioPart here:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/mac/ThemeMac.mm?rcl=f044d8fb15f2369921b2b8c88f548501b7384e6f&l=434

cc'ing some people who may (?) have some experience in this area based on the git log, and moving myself to CC.
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f49b0411204af67d51d054dd4c67161f4c1f9429

commit f49b0411204af67d51d054dd4c67161f4c1f9429
Author: Christian Biesinger <cbiesinger@chromium.org>
Date: Tue Jul 25 20:09:09 2017

Set a minimum size on checkboxes and radio buttons

We really don't want them to shrink below their default size
unless explicitly specified. This matters when they are used
as flex items, because we would otherwise compute their
min-content size to zero and shrink it to a tiny size.

Bug:  719737 
Change-Id: Ic5d6cbc134e079e76c307d049c5ec48d408214e0
Reviewed-on: https://chromium-review.googlesource.com/581753
Reviewed-by: Morten Stenshorne <mstensho@opera.com>
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489407}
[modify] https://crrev.com/f49b0411204af67d51d054dd4c67161f4c1f9429/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/f49b0411204af67d51d054dd4c67161f4c1f9429/third_party/WebKit/LayoutTests/css3/flexbox/radiobutton-min-size.html
[modify] https://crrev.com/f49b0411204af67d51d054dd4c67161f4c1f9429/third_party/WebKit/Source/core/layout/LayoutTheme.cpp
[modify] https://crrev.com/f49b0411204af67d51d054dd4c67161f4c1f9429/third_party/WebKit/Source/core/layout/LayoutTheme.h
[modify] https://crrev.com/f49b0411204af67d51d054dd4c67161f4c1f9429/third_party/WebKit/Source/core/layout/LayoutThemeDefault.cpp

Comment 9 by chrishtr@google.com, Jul 25 2017

Owner: chrishtr@chromium.org
I'll fix the Mac path.
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 4 2017

Status: Fixed (was: Started)

Sign in to add a comment