Get |-moz-appearance:none| working again on <input type=number>

RESOLVED WORKSFORME

Status

()

RESOLVED WORKSFORME
5 years ago
5 years ago

People

(Reporter: jwatt, Assigned: jwatt)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Created attachment 8342422 [details] [diff] [review]
t

My entire approach to <input type=number> has been based on making the input look exactly like <input type=text>, but with a couple of spin buttons just inside its border on the right hand side. In native toolkits and in other browser implementations the two input types look basically the same, and get the same dimensions. Users expect this.

Unfortunately bug 935508 (native theming for <input type=number>) completely threw a spanner in the works. Getting <input type=number> and <input type=text> to look the same, on all platforms, both with and without |-moz-appearance:none|, has pretty much proved to be impossible when the spin buttons are inside the border of the control. I can get them to look the same on some combination of the above, but not all simultaneously, and believe me I've tried. For days. Fixing one thing, breaking another. Round and round.

Anyway, as a result I've had to change my approach. I'm now making the spin buttons appear to be outside the number control, against its right edge. The basic change here is that instead of styling the <input type=number> to look like a text field and removing styling from the anonymous text field, I'm now removing styling from the <input type=number> and making the anonymous text field actually appear as a text field.

One of the headaches that has fallen out of this change is that |-moz-appearance:none| on <input type=number> no longer works to change the appearance of the number control from native themed to CSS styled. This is because <input type=number> is no longer the themed thing, its anonymous text control is. This would be fine if we hadn't (correctly) decided not to hide <input type=number>'s pseudo-elements from content in bug 930010, since otherwise content would be able to style the anonymous text control with |-moz-appearance:none|. Unfortunately content cannot do that at this time though.

I've tried various approaches to get around this problem such as inheriting -moz-appearance through to the grandchild anonymous text control, but that turns into a nightmare since I then need hacks to prevent |-moz-appearance:textfield| applying to the text controls parent and grandparent <input type=number>.

Boris pointed me to http://hg.mozilla.org/mozilla-central/rev/5ab823ef8518 as a slightly hacky potential solution. That seems like the best thing to do, and seems to work.
Attachment #8342422 - Flags: review?(bzbarsky)
Hmm.  So a border style on the input will not change the user-visible border by default?  Or do we inherit the border styles to the anon textbox?
Comment on attachment 8342422 [details] [diff] [review]
t

>+    if (grandparentDisplay->mAppearance == NS_THEME_NONE) {

Need to null-check grandparentDisplay, right?

r=me with that fixed, though I'm not sure I like our styling story for this input in general.  :(

Would like dbaron to sign off on this hackery too.
Attachment #8342422 - Flags: review?(bzbarsky)
Attachment #8342422 - Flags: review+
Attachment #8342422 - Flags: feedback?(dbaron)
(Assignee)

Comment 3

5 years ago
[Having the spinner "on the outside" is more consistent with native toolkits, and it allows the spin buttons to be non-tiny, making them more user friendly.]
So adding something that depends on the grandparent conflicts with some work heycam is doing in bug 931668.  Is there a way it could just depend on the parent instead?  If not, what's heycam's strategy for dealing with these issues?
Flags: needinfo?(cam)
(Assignee)

Comment 5

5 years ago
Could the |-moz-appearance: number-input;| on the grandparent number control be leveraged here? If mAppearance is set to NS_THEME_NUMBER_INPUT, then update the grandchildren contexts too?
If Jonathan is computing nsStyleDisplay::mAppearance based on looking at the grandparent's value for that field, that's fine, as long as the new style context bit NS_STYLE_USES_GRANDANCESTOR_STYLE I'm adding in bug 931668 is set on the (grandchild) style context.  I'll add it if Jonathan lands first.
Flags: needinfo?(cam)
Attachment #8342422 - Flags: feedback?(dbaron) → feedback+
(Assignee)

Comment 7

5 years ago
In the end I decided to go back to having the spinner "inside" the number control instead of "outside" it. Both approaches have ugly problems, but after dholbert came up with some flexbox wizzardry to solve the worst of the issues with having it "inside" I want that route for now.

This patch may still come in useful for bug 947340.
Blocks: 947340
No longer blocks: 344616
(Assignee)

Updated

5 years ago
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.