Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

t/1439: Firefox should visually move the caret to the new line after a soft break #1582

Merged
merged 7 commits into from
Oct 26, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions src/view/renderer.js
Original file line number Diff line number Diff line change
@@ -3,6 +3,8 @@
* For licensing, see LICENSE.md.
*/

/* globals Node */

/**
* @module engine/view/renderer
*/
@@ -20,6 +22,7 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
import isText from '@ckeditor/ckeditor5-utils/src/dom/istext';
import isNode from '@ckeditor/ckeditor5-utils/src/dom/isnode';
import fastDiff from '@ckeditor/ckeditor5-utils/src/fastdiff';
import env from '@ckeditor/ckeditor5-utils/src/env';

/**
* Renderer is responsible for updating the DOM structure and the DOM selection based on
@@ -752,6 +755,11 @@ export default class Renderer {

domSelection.collapse( anchor.parent, anchor.offset );
domSelection.extend( focus.parent, focus.offset );

// Firefox–specific hack (https://github.com/ckeditor/ckeditor5-engine/issues/1439).
if ( env.isGecko ) {
fixGeckoSelectionAfterBr( focus, domSelection );
}
}

/**
@@ -923,3 +931,28 @@ function sameNodes( blockFiller, actualDomChild, expectedDomChild ) {
// Not matching types.
return false;
}

// The following is a Firefox–specific hack (https://github.com/ckeditor/ckeditor5-engine/issues/1439).
// When the native DOM selection is at the end of the block and preceded by <br /> e.g.
//
// <p>foo<br/>[]</p>
//
// which happens a lot when using the soft line break, the browser fails to (visually) move the
// caret to the new line. A quick fix is as simple as force–refreshing the selection with the same range.
function fixGeckoSelectionAfterBr( focus, domSelection ) {
const parent = focus.parent;

// This fix works only when the focus point is at the very end of an element.
// There is no point in running it in cases unrelated to the browser bug.
if ( parent.nodeType != Node.ELEMENT_NODE || focus.offset != parent.childNodes.length - 1 ) {
return;
}

const childAtOffset = parent.childNodes[ focus.offset ];

// To stay on the safe side, the fix being as specific as possible, it targets only the
// selection which is at the very end of the element and preceded by <br />.
if ( childAtOffset && childAtOffset.tagName == 'BR' ) {
domSelection.addRange( domSelection.getRangeAt( 0 ) );
}
}
1 change: 1 addition & 0 deletions tests/manual/tickets/1439/1.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<div id="editor"></div>
33 changes: 33 additions & 0 deletions tests/manual/tickets/1439/1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/**
* @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

/* globals console, window, document */

import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor';
import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset';

ClassicEditor
.create( document.querySelector( '#editor' ), {
plugins: [ ArticlePluginSet ],
toolbar: {
items: [
'heading',
'bold',
'italic',
'link',
'bulletedList',
'numberedList',
'blockQuote',
'undo',
'redo'
]
}
} )
.then( editor => {
window.editor = editor;
} )
.catch( err => {
console.error( err.stack );
} );
11 changes: 11 additions & 0 deletions tests/manual/tickets/1439/1.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
## Firefox should visually move the caret to the new line after a soft break [#1439](https://github.com/ckeditor/ckeditor5-engine/issues/1439)


1. Open Firefox,
2. In an empty editor type "foo",
3. Press <kbd>Shift</kbd>+<kbd>Enter</kbd>.

**Expected**

1. The soft break should be created,
2. The caret should be **in the new line and blinking**.
69 changes: 69 additions & 0 deletions tests/view/renderer.js
Original file line number Diff line number Diff line change
@@ -25,6 +25,7 @@ import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
import createViewRoot from './_utils/createroot';
import createElement from '@ckeditor/ckeditor5-utils/src/dom/createelement';
import normalizeHtml from '@ckeditor/ckeditor5-utils/tests/_utils/normalizehtml';
import env from '@ckeditor/ckeditor5-utils/src/env';

describe( 'Renderer', () => {
let selection, domConverter, renderer;
@@ -1714,6 +1715,74 @@ describe( 'Renderer', () => {
expect( domRoot.innerHTML ).to.equal( '<ul><li>Foo<ul><li><b>Bar</b><i>Baz</i></li></ul></li></ul>' );
} );

// #1439
it( 'should not force–refresh the selection in non–Gecko browsers after a soft break', () => {
const domSelection = domRoot.ownerDocument.defaultView.getSelection();

testUtils.sinon.stub( env, 'isGecko' ).get( () => false );
const spy = testUtils.sinon.stub( domSelection, 'addRange' );

// <p>foo<br/>[]</p>
const { view: viewP, selection: newSelection } = parse(
'<container:p>' +
'foo[]' +
'<empty:br></empty:br>[]' +
'</container:p>' );

viewRoot._appendChild( viewP );
selection._setTo( newSelection );
renderer.markToSync( 'children', viewRoot );
renderer.render();

sinon.assert.notCalled( spy );
} );

// #1439
it( 'should force–refresh the selection in Gecko browsers after a soft break to nudge the caret', () => {
const domSelection = domRoot.ownerDocument.defaultView.getSelection();

testUtils.sinon.stub( env, 'isGecko' ).get( () => true );
const spy = testUtils.sinon.stub( domSelection, 'addRange' );

// <p>foo[]<b>bar</b></p>
let { view: viewP, selection: newSelection } = parse(
'<container:p>' +
'foo[]' +
'<attribute:b>bar</attribute:b>' +
'</container:p>' );

viewRoot._appendChild( viewP );
selection._setTo( newSelection );
renderer.markToSync( 'children', viewRoot );
renderer.render();

sinon.assert.notCalled( spy );

// <p>foo<b>bar</b></p><p>foo[]<br/></p>
( { view: viewP, selection: newSelection } = parse(
'<container:p>' +
'foo[]' +
'<empty:br></empty:br>' +
'</container:p>' ) );

viewRoot._appendChild( viewP );
selection._setTo( newSelection );
renderer.markToSync( 'children', viewRoot );
renderer.render();

sinon.assert.notCalled( spy );

// <p>foo<b>bar</b></p><p>foo<br/>[]</p>
selection._setTo( [
new ViewRange( new ViewPosition( viewP, 2 ), new ViewPosition( viewP, 2 ) )
] );

renderer.markToSync( 'children', viewRoot );
renderer.render();

sinon.assert.calledOnce( spy );
} );

describe( 'fake selection', () => {
beforeEach( () => {
const { view: viewP, selection: newSelection } = parse(