841942 - Consider displaying tick marks for <input type=range> when @list/<datalist> is used

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, enhancement, P3)

Description

[Jonathan Watt [:jwatt]]

We should figure out what we want to do for @list support for <input type=range>, if anything.

Comment 1

[Jonathan Watt [:jwatt]]

Everyone should feel free to comment on what we should be doing for @list/<datalist> with <input type=range> and how much content authors would care about having this bug fixed relative to other things we could be doing. Also please do CC anyone you think might have an opinion.

I don't think fixing this should block us shipping <input type=range>. (To my mind it would be better to have this as an enhancement at some later date, assuming it's even worth working on this rather than other things on our list of high priority work.) If anyone disagrees, please do speak up.

Comment 2

[Mounir Lamouri (:mounir)]

My concern is that there is no way to feature-detect this for a specific type. Except that, I agree it iss more a nice to have than a must have. I think we should at least have a look and see how hard implementing this would be.

Comment 3

[Jonathan Watt [:jwatt]]

What does "implementing this" involve? What is the desired behavior in your opinion?

Comment 4

[Mounir Lamouri (:mounir)]

(In reply to Jonathan Watt [:jwatt] from comment #3)
> What does "implementing this" involve? What is the desired behavior in your
> opinion?

Would say that the minimum is:
- showing a tick for each valid values in the associated datalist;

Maybe there could be:
- have a "magnet effect" when the user is around those values;
- show the values (the numbers in the UI).

I would be perfectly fine with simply the ticks being shown.

Comment 5

[Jonathan Watt [:jwatt]]

This sounds like it could get very involved.

* We'd need UI design for each individual native theming backend,
  and -moz-appearance:none, for major and minor ticks, and rendering
  of the data point labels.

* We'd (presumably) need to provide some sort of pseudo elements for
  long and short ticks, yet those elements wouldn't really be
  reflowed and rendered themselves, but rather would have to be used
  as templates for repeating, somehow.

* We'd need to decide what should happen when some of the data points
  in the list are too close together for the available space. We'd
  need logic to measure tick metrics for the various themes and for
  unthemed styled pseudo elements, detect when this is the case and
  act "appropriately", whatever that means.

* Laying out and painting data point labels at the various ticks
  would make the above even more complex given they would add to
  the metrics, layout and fitting issue.

* We'd have to take transforms into account when figuring out when
  ticks and labels are "too close" to visually make sense, and
  acting on that.

* If we have a "magnet effect", how does that interact with @step
  and the clamp-to-values that @step introduces, especially when
  the @step and @list data points conflict?

* Throw vertical vs horizontal range into the mix for even more
  fun.

* A range with ticks [and labels] is going to take up quite a bit
  of space - what are the layout rules for positioning and alignment
  of the whole thing, and how does that fit into the flow of a page?

Those are just the issues I can think of off the top of my head. Even without including labels in the UI, this sounds like it could take quite a lot of time to hash out the details for, and then another significant chunk of time to implement.

It's worth mentioning that none of the other browsers seem to do anything with datalist for type=range (probably because of the complexities listed above, or maybe because sliders with ticks look ugly).

I've also not seen anyone asking for this when I was researching type=range before starting this project FWIW.

Comment 6

[Anne (:annevk)]

Lack of feature-detection does not strike me as a major problem. Others shipped this without <datalist> support too. And not having that support degrades gracefully.

Comment 7

[Mounir Lamouri (:mounir)]

Ok, you win :)

Comment 9

[seemantk]

Happy 2017 all!

Speaking from the perspective of creating data visualizations - a basic slider with tick marks would really enhance the usability of the range <input>.  In the interim years since this bug came up, looks like gecko/webkit as well as ie/edge show tick marks in this use case.

In my view, tick marks don't need to be major/minor -- simply rendering a tick mark for every <option> in the <datalist> would be tremendously helpful and consistent with other browsers.

Comment 10

[Eric Shepherd [:sheppy]]

Currently, HTML range inputs are not very useful because of the lack of any kind of gradiation. I think it's pretty rare that these tick marks aren't a beneficial feature; indeed, having at least some tick marks which are also labeled is a big help. Although type "range" is meant to be used when precision isn't important, realistically, there are times when users are going to be trying to achieve something resembling a specific value. Such as trying to hit 50% volume so that your track is the same volume as the others. Or aiming for about 60% transparency when adjusting an rgba color using a set of sliders in a graphic editor. You might get sort of in the area, but knowing you're on the dot for key or common values is a big help.

Anyway -- the real reason I'm commenting: I've set dev-doc-needed on this bug since if it's ever fixed, docs will need updating to note that Firefox supports it.

Comment 11

[Eric Shepherd [:sheppy]]

I haven't seen this mentioned, but Chrome supports this now. I don't know for how long.

Comment 12

[Mats Palmgren (inactive)]

Edge/Chrome/Safari all support this now.  I'm moving this to Layout since
I suspect it's primarily Layout work to implement this.

Comment 13

[Mats Palmgren (inactive)]

Attachment: Testcase #1

Comment 14

[Mats Palmgren (inactive)]

No UA seems to support labels yet though.  (The URL I mentioned earlier
uses a polyfill so it's misleading for testing support for this feature.)

Comment 15

[James Teh [:Jamie]]

(In reply to Mats Palmgren (:mats) from comment #14)

No UA seems to support labels yet though. (The URL I mentioned earlier
uses a polyfill so it's misleading for testing support for this feature.)

MDN suggests Chrome 66 supports labels if the datalist is styled:
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/range#A_range_control_with_hash_marks_and_labels

Version 66 (66.0.3359.181) of Chrome supports labels but the <datalist> tag has to be styled with CSS as its display property is set to none by default, hiding the labels.

Comment 16

[James Teh [:Jamie]]

This impacts accessibility because it's much easier to ensure good a11y if authors use native widgets. The longer we don't support things like this, the more custom (and probably inaccessible) implementations will proliferate.

Note that if/when the layout part of this is implemented, we'll need code in the a11y module to support this as well.

Related Twitter thread: https://twitter.com/AmeliasBrain/status/1225270470372556801

Comment 17

[Asa Dotzler [:asa]]

Updating the Accessibility Team's impact assessment to conform with the new triage guidelines. See https://wiki.mozilla.org/Accessibility/Triage for descriptions of these whiteboard flags.

Comment 18

[one.snapp8]

Any updates on this? As a developer this is pretty frustrating, since I have to build a long workaround just to get this to work similarly on Firefox

Comment 19

[Łukasz]

It would be useful.

Comment 20

[ashleedawg]

(In reply to one.snapp8 from comment #18)

Any updates on this? As a developer this is pretty frustrating, since I have to build a long workaround just to get this to work similarly on Firefox

Agreed 100%

Comment 21

[Emilio Cobos Álvarez (:emilio)]

Chrome at least seems to tie this to their native appearance code. So this should be reasonably fixable for someone that knows C++ by changing Theme::PaintRange here

Comment 22

[Zach Hoffman [:zrhoffman]]

I'd like to pick up bug 841942. Can I please be assigned it?

Comment 23

[Emilio Cobos Álvarez (:emilio)]

Feel free to send a patch, you should be auto-assigned afterwards.

Comment 24

[Zach Hoffman [:zrhoffman]]

Attachment: 841942-tick-marks.patch

Thanks, patch sent!

What test coverage would be appropriate? Since Chrome and WebKit's tick marks are on different sides of the range input, a WPT reftest might need to be user agent-specific.

Comment 25

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9304960 [details] [diff] [review]
841942-tick-marks.patch

Review of attachment 9304960 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, this looks on the right track, see comments below.

Can you send the patch using Phabricator? See https://firefox-source-docs.mozilla.org/devtools/contributing/making-prs.html

Regarding testing, at least a couple "negative" reftests (so testing that `<input type=range list=mylist>` renders differently to `<input type=range>`, that two different lists render differently) should be doable. We can use `<link rel="mismatch" href="should-render-differently.html">` for that.

Thanks again!

::: layout/forms/nsRangeFrame.h
@@ +126,5 @@
> +   * minimum and its maximum (i.e. returns 0.0 when the value is the same as the
> +   * input's minimum, and returns 1.0 when the value is the same as the input's
> +   * maximum).
> +   */
> +  double GetDoubleAsFractionOfRange(mozilla::Decimal value);

We could consider to add an `mozilla::dom::HTMLInputElement& InputElement();` helper here, to avoid having so many static_cast<>s  spread around. See https://searchfox.org/mozilla-central/rev/05f486c76f84830041e51b7c68e3e697dc768454/layout/forms/nsComboboxControlFrame.h#211 for a reference.

::: widget/Theme.cpp
@@ +840,5 @@
>    }
>  
> +  nsIContent* mContent = rangeFrame->GetContent();
> +  MOZ_ASSERT(mContent->IsHTMLElement(nsGkAtoms::input), "bad cast");
> +  dom::HTMLInputElement* input =

nit: `auto* input =` probably.

@@ +842,5 @@
> +  nsIContent* mContent = rangeFrame->GetContent();
> +  MOZ_ASSERT(mContent->IsHTMLElement(nsGkAtoms::input), "bad cast");
> +  dom::HTMLInputElement* input =
> +      static_cast<dom::HTMLInputElement*>(rangeFrame->GetContent());
> +  auto* list = static_cast<HTMLDataListElement*>(input->GetList());

Let's fix `GetList()` so that it returns the right type if possible. Just changing the return type and changing this code:

https://searchfox.org/mozilla-central/rev/05f486c76f84830041e51b7c68e3e697dc768454/dom/html/HTMLInputElement.cpp#1659-1664

By:

return HTMLDataListElement::FromNodeOrNull(docOrShadow->GetElementById(dataListId));


should do.

@@ +854,5 @@
> +        continue;
> +      }
> +      nsAutoString str;
> +      option->GetValue(str);
> +      nsresult ec;

nit: `rv` is the usual naming for this. Also, can we move all this block of code to its own function that either returns an nsTArray<double>, or takes an `nsTArray<double>&` and fills it up?

@@ +855,5 @@
> +      }
> +      nsAutoString str;
> +      option->GetValue(str);
> +      nsresult ec;
> +      double tickMark = PromiseFlatString(str).ToDouble(&ec);

Why do you need `PromiseFlatString`?

@@ +859,5 @@
> +      double tickMark = PromiseFlatString(str).ToDouble(&ec);
> +      if (NS_FAILED(ec)) {
> +        continue;
> +      }
> +      tickMarks.AppendElement(tickMark);

Hmm, we should invalidate paint when options change... That might be a bit tricky, might be worth doing it as a follow-up.

@@ +928,5 @@
> +                                 Decimal::fromDouble(tickMark));
> +    tickMarkRects.AppendElement(
> +        LayoutDeviceRect(scaledTickMark.X() - tickMarkBox.X() / 2,
> +                         scaledTickMark.Y() - tickMarkBox.Y() / 2,
> +                         tickMarkBox.X(), tickMarkBox.Y()));

This is turning CSS pixels into device pixels without multiplying by the appropriate scale. That seems wrong? Let's use the right units in tickMarkOffsets etc.

::: widget/Theme.h
@@ +117,5 @@
>                                                           OutlineCoversBorder);
>    std::pair<sRGBColor, sRGBColor> ComputeRangeProgressColors(
>        const ElementState&, const Colors&);
> +  std::tuple<sRGBColor, sRGBColor, sRGBColor> ComputeRangeTrackColors(
> +      const ElementState&, const Colors&);

Let's document the new color :)

Comment 26

[Zach Hoffman [:zrhoffman]]

Attachment: Bug 841942 - Display tick marks for <input type=range> when @list/<datalist> is used r=emilio!

Comment 27

[Emilio Cobos Álvarez (:emilio)]

Thanks!

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/79dd2b51b7b6
Display tick marks for <input type=range> when @list/<datalist> is used r=emilio,credential-management-reviewers,sgalich

Comment 29

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/37216 for changes under testing/web-platform/tests

Comment 30

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/79dd2b51b7b6

Comment 31

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 32

[Ruth John]

Docs updates can be tracked here https://github.com/mdn/content/issues/22892

Comment 33

[Dave Letorey]

While documenting feature I came across an keyboard errro which I reported here