Skip to content

Commit

Permalink
DOM: Abort selection range set on unset range target (#8691)
Browse files Browse the repository at this point in the history
  • Loading branch information
aduth authored and youknowriad committed Aug 8, 2018
1 parent e95026c commit bec6a3c
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 3 deletions.
11 changes: 8 additions & 3 deletions packages/dom/src/dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -266,8 +266,9 @@ export function placeCaretAtHorizontalEdge( container, isReverse ) {
return;
}

container.focus();

if ( ! container.isContentEditable ) {
container.focus();
return;
}

Expand All @@ -276,6 +277,12 @@ export function placeCaretAtHorizontalEdge( container, isReverse ) {
// where `startContainer`, `endContainer` would always be container itself.
const rangeTarget = container[ isReverse ? 'lastChild' : 'firstChild' ];

// If no range target, it implies that the container is empty. Focusing is
// sufficient for caret to be placed correctly.
if ( ! rangeTarget ) {
return;
}

const selection = window.getSelection();
const range = document.createRange();

Expand All @@ -284,8 +291,6 @@ export function placeCaretAtHorizontalEdge( container, isReverse ) {

selection.removeAllRanges();
selection.addRange( range );

container.focus();
}

/**
Expand Down
10 changes: 10 additions & 0 deletions test/e2e/specs/__snapshots__/splitting-merging.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,16 @@ exports[`splitting and merging blocks should delete an empty first line 1`] = `
<!-- /wp:paragraph -->"
`;

exports[`splitting and merging blocks should gracefully handle if placing caret in empty container 1`] = `
"<!-- wp:paragraph -->
<p><strong>Foo</strong></p>
<!-- /wp:paragraph -->
<!-- wp:paragraph -->
<p></p>
<!-- /wp:paragraph -->"
`;

exports[`splitting and merging blocks should merge into inline boundary position 1`] = `
"<!-- wp:paragraph -->
<p>Bar</p>
Expand Down
23 changes: 23 additions & 0 deletions test/e2e/specs/splitting-merging.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,27 @@ describe( 'splitting and merging blocks', () => {

expect( await getEditedPostContent() ).toMatchSnapshot();
} );

it( 'should gracefully handle if placing caret in empty container', async () => {
// Regression Test: placeCaretAtHorizontalEdge previously did not
// account for contentEditables which have no children.
//
// See: https://github.com/WordPress/gutenberg/issues/8676
await insertBlock( 'Paragraph' );
await page.keyboard.type( 'Foo' );

// The regression appeared to only affect paragraphs created while
// within an inline boundary.
await page.keyboard.down( 'Shift' );
await pressTimes( 'ArrowLeft', 3 );
await page.keyboard.up( 'Shift' );
await pressWithModifier( 'mod', 'b' );
await page.keyboard.press( 'ArrowRight' );
await page.keyboard.press( 'Enter' );
await page.keyboard.press( 'Enter' );

await page.keyboard.press( 'Backspace' );

expect( await getEditedPostContent() ).toMatchSnapshot();
} );
} );

0 comments on commit bec6a3c

Please sign in to comment.