Add fix to make inputs of type email return true from isTextField #21162
Conversation
|
Size Change: -74 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
glendaviesnz
changed the title
|
What are people's thoughts on this as at least a temporary fix for the likes of Automattic/jetpack#15121? It is not a particularly nice approach having to list alternative ways to identify |
packages/dom/src/dom.js
Outdated
| @@ -453,6 +453,7 @@ export function isTextField( element ) { | |||
|
|
|||
| return ( | |||
| ( nodeName === 'INPUT' && selectionStart !== null ) || | |||
| element.getAttribute( 'type' ) === 'email' || | |||
There was a problem hiding this comment.
Probably should make this ( nodeName === 'INPUT' && element.getAttribute( 'type' ) === 'email' ) will update it if there is some agreement to go with this approach.
There was a problem hiding this comment.
I don't get it... is this input type not an input element?
There was a problem hiding this comment.
The problem is that Chrome at least does not set selectStart for anything other than <input type="text">. For <input type="email"> selectStart is null, so this method returns false, but in all instances as far as I can tell we are going to want the block editor to treat an input of type email as a text input - sorry, I outlined this in the original ticket, but should have copied it over to this PR.
There was a problem hiding this comment.
https://stackoverflow.com/a/21959157 seems to be related and links to this overview. type=number should be affected as well.
There was a problem hiding this comment.
@ellatrix How would you like to see this fixed?
There was a problem hiding this comment.
Sorry, the commit was part of #5513.
There was a problem hiding this comment.
Another idea might be restore the previous isInputField() and change isTextField() to use isInputField() + selectionStart. Then use isInputField() in documentHasSelection() which is the method used to check for the native copy handler.
There was a problem hiding this comment.
@aduth Do have any thoughts on removing selectionStart from isTextField()?
There was a problem hiding this comment.
I could have been clearer with code comments. If I recall correctly, I think the main idea was to exclude certain <input> elements like "checkbox" and "button". If selectionStart is a problem for indicating the positive case of "is a text input", then maybe the logic can be inverted to the negative case of "is not an excluded input".
For example:
nodeName === 'INPUT' && type !== 'checkbox' && type !== 'button' /* && ... */The existing unit tests should give some coverage here.
There was a problem hiding this comment.
At a quick glance it seems that of the possible 17 input types 7 could be classed as text in terms of user using standard keyboard/paste for text input, so if we want to remove selectionStart it probably makes more sense to identify the positive cases explicitly, so have updated the PR to take this approach - if there is some agreement on this will also take a look at tests to check we have this covered effectively.
glendaviesnz
force-pushed
the
fix/email-field-past-bug
branch
from
01b6862 to
eb3c480
Compare
packages/dom/src/dom.js
Outdated
| 'password', | ||
| 'search', | ||
| 'url', | ||
| 'tel', |
There was a problem hiding this comment.
We probably have to add the date types as well since not all browsers (e.g. Safari) support them and fall back to a text field. Since that's actually the default fall back vor any unknown type we should probably go with a list which are definitely no text inputs. button, checkbox, hidden, upload, …
There was a problem hiding this comment.
ah, good point, have switched to the checking for non text inputs - I think the list is correct, let me know if I have missed anything.
glendaviesnz
force-pushed
the
fix/email-field-past-bug
branch
from
8e0dfd6 to
b172b1b
Compare
…all native paste to handle pasting in email inputs
Co-authored-by: Ella van Durpe <[email protected]>
glendaviesnz
force-pushed
the
fix/email-field-past-bug
branch
from
b172b1b to
93495f3
Compare
|
Looks like the e2e tests may have picked up some issues with this change, will try and take a look at that in next day or so. |
…1162) * Add fix to make inputs of type email return true from isTextField to allow native paste to handle pasting in email inputs Co-authored-by: Glen Davies <[email protected]> Co-authored-by: Ella van Durpe <[email protected]>
Description
Currently
inputfields of typeemailreturn false fromisTextFieldwhich causes the Gutenberg past handler to take over when pasting into an email input.Fixes Automattic/jetpack#15121
Fixes #12780
How has this been tested?
Only tested manually at this staging using the email field in the Jetpack Simple Payments block.
Screenshots
before:
after:
Types of changes
Adds an additional check in order flag an input of type email as a text input to prevent Gutenberg's paste handler from hijacking the native input field copy/cut/paste events.
Checklist: