Skip to content
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

Prevent a block and console error for input[type=email] #26074

Closed

Conversation

kienstra
Copy link
Contributor

@kienstra kienstra commented Oct 13, 2020

Description

Fixes an issue where if an input[type="email"] is at the top of a block, there's a block and console error. Though this is fixed once you reload the page.

How has this been tested?

Steps to reproduce:

  1. Use Chrome or Firefox, any WordPress version, like 5.5.1, and the master branch of Gutenberg
  2. Create a new post
  3. Copy this into the console:
wp.blocks.registerBlockType(
	'example/email-input-reproduce-issue',
	{
		title: 'Email Input',
		category: 'common',
		icon: 'wordpress-alt',
		keywords: [ 'Foo' ],
		attributes: {
			'email': {
				type: 'string',
			},
		},
		edit: function( props ) {
			return wp.element.createElement( 'input', { type: 'email', value: props.email } );
		},
	}
);
  1. Insert the 'Email Input' block you just registered:insert-block
  2. Expected: The block should display
  3. Actual: There's a block and console error:

block-herb

DOMException: Failed to set the 'selectionStart' property on 'HTMLInputElement': The input element's type ('email') does not support selection.
    at placeCaretAtHorizontalEdge (https://path/to/wp-content/plugins/gutenberg/build/dom/index.js?ver=cbf62a0bae5cfd2eeff038ee115edc0a:390:32)

That error is from:

container.selectionStart = 0;

It looks like the selectionStart property applies 'only to text/number-containing or elements'.

Screenshots

With this PR, there's no error:

no-error

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

When this is at the top of a block,
on first addding the block,
there is a block and JS error:
DOMException: Failed to set the 'selectionStart' property on 'HTMLInputElement': The input element's type ('email') does not support selection.

at placeCaretAtHorizontalEdge (https://path/to/wp-content/plugins/gutenberg/build/dom/index.js?ver=cbf62a0bae5cfd2eeff038ee115edc0a:390:32)

This looks to be because
HTMLInputElement only has a
selectionStart property
for 'text/number-containing or elements'
https://developer.mozilla.org/en-US/docs/Web/API/HTMLInputElement
It looks like document.createElement( 'input' )
doesn't create an object
with .selectionStart.
So manually add one, although
it's not ideal.
If there's no selectionStart,
it should not be moved.
PHPUnit tests failed,
though this PR doesn't touch
a PHP file:
https://github.com/WordPress/gutenberg/runs/1249706078
@kienstra
Copy link
Contributor Author

kienstra commented Oct 13, 2020

Hm, the PHPUnit failure above is from:

/var/www/html/wp-content/plugins/gutenberg/phpunit/class-block-supported-styles-test.php

...which this PR doesn't touch.

That's failing on the master branch also.

@kienstra
Copy link
Contributor Author

This should be ready for review, not planning any more commits unless more builds fail.

Copy link
Contributor

@nickcernis nickcernis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I confirmed the original report and fix.

Before:

Screenshot 2020-10-14 at 18 55 17

After:

Screenshot 2020-10-14 at 18 57 22

@kienstra
Copy link
Contributor Author

Hi @ellatrix,
Would you be able to review this when you have a chance? Thanks!

@talldan talldan added [a11y] Keyboard & Focus [Type] Bug An existing feature does not function as intended labels Nov 10, 2020
Copy link
Contributor

@talldan talldan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a worthwhile fix given an email input currently crashes the block 😓 .

The issue that I noticed when testing is that up/down arrow key navigation doesn't work in the same way for email inputs as it does for other inputs (see the search block for an example), the input is essentially a trap for navigating between blocks unless the user switches to navigation mode using escape.

I'm not sure if that's related to the change here or if some other code needs to be fixed. @ellatrix any thoughts?

@ellatrix
Copy link
Member

It sounds like we need to adjust isEdge too?

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Jan 28, 2021
@diegohaz
Copy link
Member

Thanks a lot for contributing @kienstra! I just created #28559 based on this PR. I believe that solves this issue without the side effects @talldan mentioned.

@t-hamano
Copy link
Contributor

Having checked this PR again, I assume that a similar problem may have been solved by #40192.

The issue that I noticed when testing is that up/down arrow key navigation doesn't work in the same way for email inputs as it does for other inputs

This issue also appears to have already been resolved.

@talldan @ellatrix
I think we can close the following issues and PRs, how do you think?

Screencast

a5ae0e684f5beec00021d34de13f172b.mp4

@kienstra kienstra closed this Dec 21, 2022
@kienstra kienstra deleted the fix/input-selection-start-error branch December 21, 2022 20:02
@priethor priethor added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Writing Flow Block selection, navigation, splitting, merging, deletion... [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DOMException: Failed to set the 'selectionStart' property on 'HTMLInputElement'
7 participants