DOM: Allow copying text from non-text input elements #40192
Conversation
mcsf
added
[Type] Bug
| @@ -26,6 +26,7 @@ export default function isTextField( node ) { | |||
| 'reset', | |||
| 'submit', | |||
| 'number', | |||
| 'email', | |||
There was a problem hiding this comment.
Could any component by negatively affected by this spec correction?
There was a problem hiding this comment.
Also, I really dislike isTextField's exclusion approach. Since, in fact, most input types are not text fields, we might be better off with a stricter inclusion approach, i.e. [ 'text', ... ].includes( type ).
There was a problem hiding this comment.
I don't think anything would be negatively affected by this.. And I also think a reversed approach would be okay, but we would really need to make sure we have all the needed types 😄
|
Size Change: +14 B (0%) Total Size: 1.23 MB
ℹ️ View Unchanged
|
| @@ -18,9 +18,25 @@ import isNumberInput from './is-number-input'; | |||
| * @return {boolean} Whether the input/textareaa element has some "selection". | |||
| */ | |||
| export default function inputFieldHasUncollapsedSelection( element ) { | |||
| if ( ! isTextField( element ) && ! isNumberInput( element ) ) { | |||
| if ( ! isTextField( element ) && ! isHTMLInputElement( element ) ) { | |||
There was a problem hiding this comment.
How about creating a new util here(instead of isHTMLInputElement) that wraps the checks for the specific inputs we want to let browsers handle it. I think these would be number, email and time? In that case we would add an extra same check below, if there is no selection but this is true.
Now if we have any other inputs like button, etc.. would return true and I don't think we need to do this.
There was a problem hiding this comment.
I'm not sure I understood your question; could you show me?
There was a problem hiding this comment.
In several places in the code base we use the isNumberInput for the exact same reason of this PR: trying also include input[type="number"] for some selection related actions/calculations. What I suggest is to make a new util that will include isNumberInput and similar checks as if we had isEmailInput and isTimeInput. Makes sense?
There was a problem hiding this comment.
Thanks, I understand the point now. What's interesting is that, the more I look at this PR and at existing usage of isNumberInput, the more I'm confused. :) I'll comment in more detail in the general thread.
There was a problem hiding this comment.
Thanks for the PR Miguel!
I think this is the right direction, but we need to make sure we update if needed related code to this. For example in documentHasSelection util we perform similar checks and we should probably take into account the added email,time.
Also it's interesting that while we had code for checking number type, since it has no selectionStart/End we would always return false 😄
|
Part of what makes these components tricky is that browsers implement these interfaces differently. My notes in the following paragraph are sort of a recap, but I find it helpful even just for myself to explain it again: The spec states that Here's a quick codesandbox script to try on different browsers. Red indicates input types deemed "opaque" — i.e., that don't support I've been going back and forth on a question: since Safari is arguably being "useful" by supporting selection APIs in a wider range of input types, should we let users benefit from this via browser-dependent behaviour, or should we prioritise behaviour that is consistent across browsers and up to spec? |
My other question, prompted by Nik's comment, is around This leads me to think that there is no reason to continue to treat numbers differently. Ultimately, all these selection-checking functions serve one purpose: to determine what to do when the user uses general keyboard shortcuts to copy, cut, and paste content. Below are two tables describing what I think is the current intended behaviour in Gutenberg. For simplicity, ignore contentEditable elements. When pasting:
Pasting is easier, because it doesn't care whether there is a collapsed or uncollapsed selection in the active element. Also note that, ideally, opaque inputs should behave just like text ones — browser permitting. Copying and cutting is more complicated, due to the desire to distinguish collapsed and uncollapsed selections. In bold I highlight what this PR was initially aiming to fix (i.e. conform with the intended behaviour). In italic I highlight what this PR was intentionally dismissing as a trade-off. When copying or cutting:
Solution oneBased on these tables, I'd like to drop usage of Solution twoAlthough solution 1 works well for me, I'm also tempted to simplify this drastically, removing the collapsed/uncollapsed distinction, which in turn eliminates the need to inspect the input:
Thoughts? Preferences? /cc @ntsekouras, @youknowriad |
mcsf
force-pushed
the
fix/copy-from-non-text-inputs
branch
2 times, most recently
from
72e2a0b to
b19886a
Compare
|
@mcsf I don't feel like I understand all the implications but the reasoning makes sense and I'm always for simplifications. So I'm onboard with solution 2. |
The selection state of non-text input elements (e.g. `number`, but even `email`) is opaque to Gutenberg, per the HTML spec [1]. Unaware of this nuance, `inputFieldHasUncollapsedSelection` would incorrectly report that some non-text input element had no text selected when in fact it did. This caused the block editor to take over `copy` events to copy whole blocks, preventing the user from copying what they had actually selected inside the input element in question. The workaround in this commit is to always assume that there could be an uncollapsed selection in an active element if that element's selection state is opaque. [1]: https://html.spec.whatwg.org/multipage/input.html#do-not-apply
... as oppposed to just text inputs (isTextField) and number inputs (isNumberInput).
mcsf
force-pushed
the
fix/copy-from-non-text-inputs
branch
from
b19886a to
b301855
Compare
That's true 😄 . We should have a follow up on that one. |
For the record, I paired with Nik today and we looked at both solutions, and we decided in favour of the first one. I had overestimated the simplification benefits of Solution 2, and I'd rather keep the special treatment of uncollapsed selections if that means that there's a higher likelihood that the editor will do what the user wanted. Going to merge. :) Thanks, everyone. |
What?
Fixes #32148.
This pull request guarantees that a user is able to copy the contents of any
<input>elements inside blocks using their browser's native actions (Ctrl-C, Command-C).Why?
This pull request fixes a bug in which certain types of input were not copiable — specifically, any "non-text" input, such as
numberor evenemail. Typically, when this bug occurred, the block editor would ignore the browser's native copying behaviour, thus resulting in the copying of the entire block — instead of just the selected content in the input element.How?
The selection state of non-text input elements is opaque to Gutenberg, per the HTML spec. Unaware of this
nuance,
inputFieldHasUncollapsedSelection( element )would incorrectly report that some non-text input element had no text selected when in fact it did. This caused the block editor to take overcopyevents to copy whole blocks, preventing the user from copying what they had actually selected inside the input element in question.The trade-off in this commit is to always assume that there could be an uncollapsed selection in an active element if that element's selection state is opaque.
Caveat
The trade-off means that, if the user places the caret inside a non-text input element but has no text selected (according to the jargon, this means that the selection is collapsed), then pressing the copy shortcut will no longer let the block editor copy the whole block. Text inputs (
input="text"or"url") and rich-text elements should not be affected by this.Testing Instructions
For specifics, see instructions in #32148.
<TextControl type="number"inside a block, as well astype="email",type="time".<TextControl type="text"inside a block, as well astype="url".Attached: The diff I applied to the Paragraph block for my own testing