diff --git a/src/view/renderer.js b/src/view/renderer.js index 28e4108ed..934ed80a0 100644 --- a/src/view/renderer.js +++ b/src/view/renderer.js @@ -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
e.g. +// +//

foo
[]

+// +// 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
. + if ( childAtOffset && childAtOffset.tagName == 'BR' ) { + domSelection.addRange( domSelection.getRangeAt( 0 ) ); + } +} diff --git a/tests/manual/tickets/1439/1.html b/tests/manual/tickets/1439/1.html new file mode 100644 index 000000000..624d2e6b7 --- /dev/null +++ b/tests/manual/tickets/1439/1.html @@ -0,0 +1 @@ +
diff --git a/tests/manual/tickets/1439/1.js b/tests/manual/tickets/1439/1.js new file mode 100644 index 000000000..f1d417c84 --- /dev/null +++ b/tests/manual/tickets/1439/1.js @@ -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 ); + } ); diff --git a/tests/manual/tickets/1439/1.md b/tests/manual/tickets/1439/1.md new file mode 100644 index 000000000..2a2c0d33c --- /dev/null +++ b/tests/manual/tickets/1439/1.md @@ -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 Shift+Enter. + +**Expected** + +1. The soft break should be created, +2. The caret should be **in the new line and blinking**. diff --git a/tests/view/renderer.js b/tests/view/renderer.js index f640d8831..4dd9a2455 100644 --- a/tests/view/renderer.js +++ b/tests/view/renderer.js @@ -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( '' ); } ); + // #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' ); + + //

foo
[]

+ const { view: viewP, selection: newSelection } = parse( + '' + + 'foo[]' + + '[]' + + '' ); + + 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' ); + + //

foo[]bar

+ let { view: viewP, selection: newSelection } = parse( + '' + + 'foo[]' + + 'bar' + + '' ); + + viewRoot._appendChild( viewP ); + selection._setTo( newSelection ); + renderer.markToSync( 'children', viewRoot ); + renderer.render(); + + sinon.assert.notCalled( spy ); + + //

foobar

foo[]

+ ( { view: viewP, selection: newSelection } = parse( + '' + + 'foo[]' + + '' + + '' ) ); + + viewRoot._appendChild( viewP ); + selection._setTo( newSelection ); + renderer.markToSync( 'children', viewRoot ); + renderer.render(); + + sinon.assert.notCalled( spy ); + + //

foobar

foo
[]

+ 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(