1123299 - <input type="number"> needs to be aware of vertical writing mode

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Jonathan Kew [:jfkthame]]

The element's default sizing is bad; it uses an explicit width. We'll fix that in bug 1119475. But also, the little double-arrow control for adjusting the value needs to be displayed in a vertical-aware way.

Comment 1

[Jonathan Kew [:jfkthame]]

Hmm - it turns out the anonymous content tree created for <input type="number"> uses display:flex, so this probably depends on bug 1079155 being implemented first.

Comment 2

[Jonathan Kew [:jfkthame]]

Note that the vertical portion of reftest ua-style-sheet-input-number-1.html is currently commented-out because support is not yet implemented, and it leads to assertions... once this bug is resolved, we should re-enable that test.

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: Allow <input type=number> to be displayed in vertical writing mode; but keep the spinner arrows arranged as for horizontal writing mode

[WIP] css changes to allow <input type=number> to be used vertically. Waiting on vertical-mode support for flexbox (bug 1079155) before we can actually use this.

Comment 4

[Daniel Holbert [:dholbert]]

(In reply to Jonathan Kew (:jfkthame) from comment #3)
> Waiting on vertical-mode support for flexbox (bug 1079155) before we can
> actually use this.

I think our existing partially-converted vertical-mode support for flexbox may already be good enough for this.  I do see issues here, but I don't think they're from flexbox; they're from nsNumberControlFrame.cpp.

e.g. when I load this testcase (which produces a text-entry frame that's ~nscoord_MAX wide)...
  data:text/html,<div style="writing-mode: vertical-lr"><input type="number" value="5">
...a frame dump shows that the internal flexbox stuff is all reasonably-sized. It's only the nsNumberControlFrame that's nscoord_MAX wide.

I think that comes from the "contentBoxHeight == NS_INTRINSICSIZE" checks in  nsNumberControlFrame::Reflow.  Those need to be converted to use BSize instead of Height.

Comment 5

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #4)
> e.g. when I load this testcase (which produces a text-entry frame that's
> ~nscoord_MAX wide)...

(loading that testcase with the current patch applied, I mean)

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: part 1: logicalize nsNumberControlFrame::Reflow [spun off as bug 1173662]

This patch (combined with the attached CSS patch) makes the test data URL in comment 4 render as expected, I think. (18px wide, instead of nscoord_MAX wide)

Comment 7

[Daniel Holbert [:dholbert]]

Comment on attachment 8614094 [details] [diff] [review]
Allow <input type=number> to be displayed in vertical writing mode; but keep the spinner arrows arranged as for horizontal writing mode

The spin buttons are slightly mispositioned (snapped to bottom-right) with current m-c & the patches here, but I've fixed that locally (it's a bug w/ flexbox code using the wrong writing mode, when flex container & flex item have different writing modes).

With that fixed, the spin-buttons get pushed such that the bottom half is sticking outside the control.  That's due to this styling:

>diff --git a/layout/style/forms.css b/layout/style/forms.css
> input[type=number]::-moz-number-spin-box {
>+  writing-mode: horizontal-tb;
[...]
>-  height: 0;
>+  block-size: 0;

(tangent: given the hardcoded writing-mode here, I'm not sure it makes sense to change 'height' to 'block-size'. Unless we're just doing this across the board for consistency, even when we know it'll always be a height.)
(end tangent.)

This needs a bit of massaging to avoid overflowing off the bottom of a <input type="number"> with a vertical writing-mode.

The spinbox is set up to be correctly positioned in a horizontal writing-mode <input type="number"> like so:
 - The spinbox has 0 height.
 - Its up-arrow overflows off of the top of its box.
 - Its down-arrow overflows off of the bottom of its box.
 - We center its 0-height box vertically in the parent using "align-self: center".  So, the uparrow/downarrow look vertically centered in a horizontal textbox.
 - The spinbox has a *nonzero width*, so it doesn't get pushed off the right side of the control by the other flex item (the actual text entry area)

However -- when we change to a vertical writing-mode, now the main axis of the outer flex container ("-moz-number-wrapper") is the vertical axis. So it gives the spinbox 0 height (as requested) and gives the other flex item (-moz-number-text) all of the height. This means the spinbox ends up getting pushed off of the bottom.

Really, we want to give it a content-sized main-size (i.e. shrinkwrap the buttons in the main axis), and we want to center the buttons in the cross axis. We might be able to get away with dropping height/width here altogether (aside from the %ifdef XP_WIN hack).

Comment 8

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #7)
> The spin buttons are slightly mispositioned (snapped to bottom-right) with
> current m-c & the patches here, but I've fixed that locally (it's a bug w/
> flexbox code using the wrong writing mode, when flex container & flex item
> have different writing modes).

(I filed bug 1173646 to cover this side-issue, BTW.)

Comment 9

[Daniel Holbert [:dholbert]]

Comment on attachment 8620555 [details] [diff] [review]
part 1: logicalize nsNumberControlFrame::Reflow [spun off as bug 1173662]

I spun off the nsNumberControlFrame::Reflow patch to bug 1173662, since it can land independently of the forms.css changes here. (and I'm not yet exactly clear what the forms.css changes should be, as noted in comment 7)

Comment 10

[Jonathan Kew [:jfkthame]]

Attachment: Allow <input type=number> to be displayed in vertical writing mode; but keep the spinner arrows arranged as for horizontal writing mode

With the flexbox and nsNumberControlFrame fixes that have landed now (thanks), we should be able to move forward here. Removing the height property from -moz-number-spin-box altogether (comment 7) doesn't quite work, as it then enlarges the overall height of the control in horizontal mode. But if we limit it to 1em, things seem to work pretty well. If UX design folk want to refine it further in some way, that's fine, but at least we'll have something that works.

Comment 11

[Daniel Holbert [:dholbert]]

Attachment: testcase with various font-sizes

Unfortunately, using "max-height:1em" means the controls' positioning will depend on the font-size, in a vertical writing-mode.

See attached testcase.  With the attached patch:
 - a small font-size makes the spinbox overflow off the bottom of the the textbox.
 - a large font-size makes the spinbox have some extra bottom-spacing (from taking on its full intrinsic height, due to its max-width:1em being larger & not actually clamping)

Maybe this is fine; it's better than the current behavior, at least.  If we can't quickly do anything better, could you file a followup on improving things here, and mention that bug number in a comment alongside the hacky 1em in forms.css?

Comment 12

[Daniel Holbert [:dholbert]]

I think we *really* want to set "block-size: 0" on the spinbox, so that it will have 0 height (and intrinsic width) in a horizontal control, and 0 width (but intrinsic height) in a vertical control.

That doesn't work right now, because the spinbox itself has a hard-coded writing-mode, so "block-size" is effectively equivalent to "height".  If we gave the spinbox an extra wrapper, though, which inherited the parent's writing-mode, then we could give *that wrapper* "block-size:0", I think...

There may be a more elegant way of doing things; this is the best I've come up with at the moment, though. Anyway, this is more complex than a simple CSS tweak, hence my suggestion of deferring a "real" fix to a followup.

Comment 13

[Daniel Holbert [:dholbert]]

Comment on attachment 8622060 [details] [diff] [review]
Allow <input type=number> to be displayed in vertical writing mode; but keep the spinner arrows arranged as for horizontal writing mode

r=me with a followup bug filed & noted in forms.css per above.

Please also include a reftest with a <input type="number"> that has a value and a vertical writing-mode. (probably with an abspos black box covering up the spinbox, as in the other number tests) This will at least let us test that the numeral is oriented correctly (with a vertical writing-mode, and snapped to the top).

Comment 14

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5b07aa1d76e2

Comment 15

[Jonathan Kew [:jfkthame]]

Pushed, including a reftest as requested:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b07aa1d76e2

And filed followup bug 1175074.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5b07aa1d76e2