-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make input.select() never throw #2327
Conversation
Fixes #2275 by making it do nothing, instead of throwing, when it does not apply. This matches all browsers. In the process, this inlines the "does not apply" behavior (which was previously in a prelude that readers often missed; see e.g. #2175), and makes all the setters, getters, and methods into algorithms with steps instead of just paragraphs.
lgtm |
<li> | ||
<p>If this element is an <code>input</code> element, and either <code | ||
data-x="dom-textarea/input-select">select()</code> <span data-x="do not apply">does not | ||
apply</span> to this element or the corresponding control has no selectable text, return.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"return."
I'm not a spec-writer, but it seems like HTML usually phrases this instead as "abort this algorithm"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That or "abort these steps".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are slowly moving to https://infra.spec.whatwg.org/ which would allow/encourage just return. We do tend to prefix it with "then", even though JavaScript doesn't.
LGTM with 1 nit |
Tests are ready. I don't feel strongly about "return" vs. "then return" vs. "abort these steps" vs. "abort this algorithm". |
apply</span> to this element or the corresponding control has no selectable text, return.</p> | ||
|
||
<p class="example">For instance, in a user agent where <code | ||
data-x="attr-input-type-color"><input type=color></code> is rendered as a color well with a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should that be "color wheel"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a color well: https://goo.gl/images/JaTWBg
This is a color wheel: https://goo.gl/images/7keUq4
source
Outdated
<code>DOMException</code>.</p></li> | ||
|
||
<li><p><span>Set the selection range</span> with the value of this element's <code | ||
data-x="dom-textarea/input-selectionStart">selectionStart</code>, the given value, and the value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should that be "element's selectionStart attribute"? There may be other cases of missing "attribute" that I'm not seeing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have tests, and browsers pass them, somewhat surprisingly to me: https://github.com/w3c/web-platform-tests/blob/b28ee3b4ac70f3cc670c2d67cee8b5daf5f45646/html/semantics/forms/the-input-element/selection.html#L138
source
Outdated
<span>"<code>InvalidStateError</code>"</span> <code>DOMException</code>.</p></li> | ||
|
||
<li><p><span>Set the selection range</span> with the value of this element's <code | ||
data-x="dom-textarea/input-selectionStart">selectionStart</code>, the value of this element's |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
attribuge
|
||
<ol> | ||
|
||
<li><p>If this element is an <code>input</code> element, and <code | ||
data-x="dom-textarea/input-setRangeText">setRangeText()</code> <span data-x="do not apply">does | ||
not apply</span> to this element, throw an <span>"<code>InvalidStateError</code>"</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have tests for this, or might it also be a no-op everywhere?
I have no strong opinion on "return" vs. "then return", but if you were to sample spec text I've written I think I'd normally not have used "then" after commas. |
Fixes #2275 by making it do nothing, instead of throwing, when it does
not apply. This matches all browsers.
In the process, this inlines the "does not apply" behavior (which was
previously in a prelude that readers often missed; see e.g. #2175), and
makes all the setters, getters, and methods into algorithms with steps
instead of just paragraphs.
Tests: web-platform-tests/wpt#4755
Careful review appreciated as there might be copypasta lurking.
/cc @tkent-google @cvrebert