Open Bug 947365 Opened 6 years ago Updated 5 months ago

Make <input type=number>'s spinner the same height as its text input control

Categories

(Core :: Layout: Form Controls, defect)

defect
Not set

Tracking

()

People

(Reporter: jwatt, Unassigned)

References

(Depends on 1 open bug, Blocks 2 open bugs, )

Details

Attachments

(4 files)

Ground rules: <input type=number> and <input type=text> are supposed to look identical (both native themed and not) with the exception of the former having a spinner slapped on it. That spinner should not change the dimensions of the number control though.

Background: nsNumberControlFrame creates a native anonymous content tree for <input type=number> that looks like this:

  <input type=number>
    <div>                - outer wrapper with "display:flex" by default
      <input type=text>  - text input field
      <div>              - spinner box wrapping up/down arrow buttons
        <div>            - spin up (up arrow button)
        <div>            - spin down (down arrow button)

Right now the spinner often isn't as tall as it could be (it's less than the height of the text input field), which makes its buttons smaller and harder to use than they ideally would be. This is happening because we have to set the height of the spinner to zero to center it vertically and to stop it from affecting the height of the number control if the combined height of its buttons is more than the height of the spinner's text field sibling. We have to do this because native theming forces the size of the buttons to a minimum size that is too tall, particularly if the number control itself is not themed. If the buttons are too tall, we want them to just overflow rather than for them to increase the height of the number control.

This works fine to prevent buttons that are too tall from increasing the height of the number control. However it's suboptimal when the buttons are less than half the height of the text field. In this latter case the buttons just take on their minimum height because their spinner parent has a height of zero. What we actually want to have happen is that the buttons size as big as possible up to but no bigger than the height of the text field.

In other words the height of the <input type=number> and wrapper <div> should shrink wrap to the height of the <input type=text>, and then the spinner <div> should expand to the resulting height of the wrapper <div>.

What I don't understand is why setting |height:100%| on the spinner doesn't set its height to the height we want. In fact whatever percentage value I set seems no be interpreted as the height of the text field plus 2px. If I try and use calc(100% - 2px) that fails too (still the same height).

I'm not sure, but I think that bug 527285 would allow us to fix this.
Depends on: 527285
(In reply to Jonathan Watt [:jwatt] from comment #0)
> What I don't understand is why setting |height:100%| on the spinner doesn't
> set its height to the height we want. In fact whatever percentage value I
> set seems no be interpreted as the height of the text field plus 2px. If I
> try and use calc(100% - 2px) that fails too (still the same height).

I think that's just because it doesn't have anything to resolve the percentage against, right? (since its containing block -- the flexbox -- doesn't have a fixed height. It derives its height from its children, and we're one of the children)

IsAutoHeight, defined here...
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsLayoutUtils.h?rev=9ceffd9514da&mark=1097-1097#1092
...says we treat percent heights against unresolved (nscoord_MAX) parent heights as "auto".

(I think the relevant caller of that function is
 http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsFrame.cpp?rev=9ceffd9514da#3973
which is what we end up using to populate the reflow state's mComputedHeight value.)
Per IRC discussion, I think something like this should satisfy the requirements in comment 0:
 http://jsfiddle.net/bsrP5/2/

In particular: We sort of want the spinner to have the default "align-self: stretch" (instead of "align-self: center" like it currently is in forms.css), so that it can grow when the control is large. But, we can't just directly make that change, or else the spin-buttons' required heights will impact the minimum size of the form control (which we don't want).

So, that fiddle wraps the spinner in an additional vertical flexbox, and *that wrapper* is "align-self:stretch". That wrapper doesn't impact the minimum size of the control, because it has only one child (the spinner) which I've given "flex: 1 0px" (i.e. start at 0, and absorb any space that we're given).

Then, *inside* the spinner, the spin buttons themselves can maintain their own minimum sizes, and they'll overflow from the spinner in a balanced way when the form control (and consequently the spinner) are short, due to "justify-content: center" on the spinner.
They aren't scaling to DPI either. The spin control is unusably tiny on Windows high DPI.

(The lorem ipsum is 9 point Arial, for reference.)
The current implementation seems to be completely broken. As can be seen in the attached picture (34.0.5, Linux, 64-bit), even in the default state it has wrong alignment (shifted slightly lower and significantly from the right edge). When the page is zoomed in or out (Ctrl++, Ctrl+-), the spinner just floats around unaffected.
Attachment #8532739 - Attachment description: appearance in 35.0.5 64-bit version for Linux at various zoom levels → appearance in 34.0.5 64-bit version for Linux at various zoom levels
> The current implementation seems to be completely broken.

The situation in Windows is not better: with the Win7 theme the spinner tries to scale horizontally (although the right margin is wrong) but not vertically, with the classic theme it does not scale at all and just has the right margin changing even worse than in Linux. In neither case the appearance is quite close to that of the native control (the Win7 case being worse).

Moreover, changing the default font size (Preferences → Content → Default font Size) apparently has no effect on the size of the spinner.
Having tried type=number for the first time, I would have thought this is a serious enough issue to disable the spinner buttons altogether until they're fixed. (As it stands, I have to hide them because of how broken they are. That's a pretty good sign it's not ready for release.)
Duplicate of this bug: 1108469
I added the simplistic example from bug 1108469. Regarding comment 9, bug 978320 seems to be related.

Sebastian
Component: DOM: Core & HTML → Layout: Form Controls
Blocks: 1169203
No longer blocks: 1169203
Blocks: 1169203
I believe this bug is also related to bug 1158435 and bug 1287681 -- maybe it would make sense to maintain a single issue to track?  Spinner arrows are totally unusable in their current state (on Windows, high-DPI).
See Also: → 1554573
You need to log in before you can comment on or make changes to this bug.