CSS filter editor relies on the setSelectionRange() for an <input type=number>

RESOLVED FIXED in Firefox 51

Status

RESOLVED FIXED
2 years ago
4 months ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

unspecified
Firefox 51
Dependency tree / graph

Firefox Tracking Flags

(firefox51 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

2 years ago
The HTML spec says that setSelectionRange() and the rest of the selection APIs do not apply to <input type=number>.  I'm fixing our implementation in bug 1293570 and have discovered this dependency through that bug.

The code here doesn't actually depend on being able to do this for <input type=number>s.  This is only used for text input boxes in this widget.  Note that the code in _keyDown() also accesses these APIs, but for filters which don't have a unit, which are the ones that we create <input type=text>s for in render(), so those call sites are fine.
(Assignee)

Comment 1

2 years ago
Created attachment 8783746 [details] [diff] [review]
Avoid calling HTMLInputElement.setSelectionRange() on <input type=number>s in the CSS filter widget code
Attachment #8783746 - Flags: review?(pbrosset)
Comment on attachment 8783746 [details] [diff] [review]
Avoid calling HTMLInputElement.setSelectionRange() on <input type=number>s in the CSS filter widget code

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

::: devtools/client/shared/widgets/FilterWidget.js
@@ +675,5 @@
>      }
>  
>      let lastInput =
>          this.filtersList.querySelector(".filter:last-of-type input");
> +    if (lastInput && lastInput.type == "text") {

Adding the condition here means that we're no longer focusing the field, which we want to do so users can start typing in the field directly.

Can you move the if (lastInput.type == "text") below, so it's just around the
const end = lastInput.value.length;
lastInput.setSelectionRange(end, end);
Attachment #8783746 - Flags: review?(pbrosset)
(Assignee)

Comment 3

2 years ago
Good catch, will do!
(Assignee)

Comment 4

2 years ago
Created attachment 8784102 [details] [diff] [review]
Avoid calling HTMLInputElement.setSelectionRange() on <input type=number>s in the CSS filter widget code
Attachment #8784102 - Flags: review?(pbrosset)
(Assignee)

Updated

2 years ago
Attachment #8783746 - Attachment is obsolete: true
Comment on attachment 8784102 [details] [diff] [review]
Avoid calling HTMLInputElement.setSelectionRange() on <input type=number>s in the CSS filter widget code

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

Thanks!
Attachment #8784102 - Flags: review?(pbrosset) → review+

Comment 6

2 years ago
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/502e48c4fb6a
Avoid calling HTMLInputElement.setSelectionRange() on <input type=number>s in the CSS filter widget code; r=pbro

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/502e48c4fb6a
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
> Review of attachment 8784102 [details] [diff] [review] [diff] [review]:
> -----------------------------------------------------------------
> +      if (lastInput.type == "text") {

Nit: That should have been ===.

Sebastian

Comment 9

2 years ago
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/251a9edbb0cc
Address the post-landing drive-by nit

Updated

4 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.