Closed
Bug 1297225
Opened 5 years ago
Closed 5 years ago
CSS filter editor relies on the setSelectionRange() for an <input type=number>
Categories
(DevTools :: Inspector: Rules, defect)
DevTools
Inspector: Rules
Tracking
(firefox51 fixed)
RESOLVED
FIXED
Firefox 51
Tracking | Status | |
---|---|---|
firefox51 | --- | fixed |
People
(Reporter: ehsan, Assigned: ehsan)
References
Details
Attachments
(1 file, 1 obsolete file)
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•5 years ago
|
||
Attachment #8783746 -
Flags: review?(pbrosset)
Comment 2•5 years ago
|
||
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•5 years ago
|
||
Good catch, will do!
Assignee | ||
Comment 4•5 years ago
|
||
Attachment #8784102 -
Flags: review?(pbrosset)
Assignee | ||
Updated•5 years ago
|
Attachment #8783746 -
Attachment is obsolete: true
Comment 5•5 years ago
|
||
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+
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•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/502e48c4fb6a
Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
Comment 8•5 years ago
|
||
> Review of attachment 8784102 [details] [diff] [review] [diff] [review]:
> -----------------------------------------------------------------
> + if (lastInput.type == "text") {
Nit: That should have been ===.
Sebastian
Pushed by eakhgari@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/251a9edbb0cc Address the post-landing drive-by nit
Comment 10•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/251a9edbb0cc
Updated•3 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•