Skip to content

Commit 958d7c9

Browse files
ReinmarReinmar
committed
Merge branch 't/12491'
2 parents d6b338a + 5bb8c38 commit 958d7c9

File tree

3 files changed

+266
-35
lines changed

3 files changed

+266
-35
lines changed

‎CHANGES.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ CKEditor 4 Changelog
33

44
## CKEditor 4.4.6
55

6+
Fixed Issues:
7+
8+
* [#12489](http://dev.ckeditor.com/ticket/12423) and [#12491](http://dev.ckeditor.com/ticket/12423): Fixed: Various issues related to restoring selection after making operations on filler char. See the [fixed cases](http://dev.ckeditor.com/ticket/12491#comment:4).
9+
610
## CKEditor 4.4.5
711

812
New Features:

‎core/selection.js

Lines changed: 36 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -149,25 +149,20 @@
149149

150150
// Text selection position might get mangled by
151151
// subsequent dom modification, save it now for restoring. (#8617)
152-
if ( keepSelection !== false )
153-
{
152+
if ( keepSelection !== false ) {
154153
var bm,
155-
doc = element.getDocument(),
156-
sel = doc.getSelection().getNative(),
154+
sel = element.getDocument().getSelection().getNative(),
157155
// Be error proof.
158156
range = sel && sel.type != 'None' && sel.getRangeAt( 0 );
159157

160158
if ( fillingChar.getLength() > 1 && range && range.intersectsNode( fillingChar.$ ) ) {
161-
bm = [ sel.anchorOffset, sel.focusOffset ];
159+
bm = createNativeSelectionBookmark( sel );
162160

163161
// Anticipate the offset change brought by the removed char.
164162
var startAffected = sel.anchorNode == fillingChar.$ && sel.anchorOffset > 0,
165163
endAffected = sel.focusNode == fillingChar.$ && sel.focusOffset > 0;
166-
startAffected && bm[ 0 ]--;
167-
endAffected && bm[ 1 ]--;
168-
169-
// Revert the bookmark order on reverse selection.
170-
isReversedSelection( sel ) && bm.unshift( bm.pop() );
164+
startAffected && bm[ 0 ].offset--;
165+
endAffected && bm[ 1 ].offset--;
171166
}
172167
}
173168

@@ -176,13 +171,9 @@
176171
// invisible char from it.
177172
fillingChar.setText( replaceFillingChar( fillingChar.getText() ) );
178173

179-
// Restore the bookmark.
174+
// Restore the bookmark preserving selection's direction.
180175
if ( bm ) {
181-
var rng = sel.getRangeAt( 0 );
182-
rng.setStart( rng.startContainer, bm[ 0 ] );
183-
rng.setEnd( rng.startContainer, bm[ 1 ] );
184-
sel.removeAllRanges();
185-
sel.addRange( rng );
176+
moveNativeSelectionToBookmark( element.getDocument().$, bm );
186177
}
187178
}
188179
}
@@ -194,14 +185,22 @@
194185
} );
195186
}
196187

197-
function isReversedSelection( sel ) {
198-
if ( !sel.isCollapsed ) {
199-
var range = sel.getRangeAt( 0 );
200-
// Potentially alter an reversed selection range.
201-
range.setStart( sel.anchorNode, sel.anchorOffset );
202-
range.setEnd( sel.focusNode, sel.focusOffset );
203-
return range.collapsed;
204-
}
188+
function createNativeSelectionBookmark( sel ) {
189+
return [
190+
{ node: sel.anchorNode, offset: sel.anchorOffset },
191+
{ node: sel.focusNode, offset: sel.focusOffset }
192+
];
193+
}
194+
195+
function moveNativeSelectionToBookmark( document, bm ) {
196+
var sel = document.getSelection(),
197+
range = document.createRange();
198+
199+
range.setStart( bm[ 0 ].node, bm[ 0 ].offset );
200+
range.collapse( true );
201+
sel.removeAllRanges();
202+
sel.addRange( range );
203+
sel.extend( bm[ 1 ].node, bm[ 1 ].offset );
205204
}
206205

207206
// Read the comments in selection constructor.
@@ -840,7 +839,9 @@
840839
} );
841840

842841
CKEDITOR.on( 'instanceReady', function( evt ) {
843-
var editor = evt.editor;
842+
var editor = evt.editor,
843+
fillingCharBefore,
844+
selectionBookmark;
844845

845846
// On WebKit only, we need a special "filling" char on some situations
846847
// (#1272). Here we set the events that should invalidate that char.
@@ -852,8 +853,6 @@
852853
removeFillingChar( editor.editable() );
853854
}, null, null, -1 );
854855

855-
var fillingCharBefore, resetSelection;
856-
857856
editor.on( 'beforeUndoImage', beforeData );
858857
editor.on( 'afterUndoImage', afterData );
859858
editor.on( 'beforeGetData', beforeData, null, null, 0 );
@@ -868,11 +867,13 @@
868867
var fillingChar = getFillingChar( editable );
869868

870869
if ( fillingChar ) {
871-
// If cursor is right blinking by side of the filler node, save it for restoring,
872-
// as the following text substitution will blind it. (#7437)
873-
var sel = editor.document.$.defaultView.getSelection();
874-
if ( sel.type == 'Caret' && sel.anchorNode == fillingChar.$ )
875-
resetSelection = 1;
870+
// If the selection's focus or anchor is located in the filling char's text node,
871+
// we need to restore the selection in afterData, because it will be lost
872+
// when setting text. Selection's direction must be preserved.
873+
// (#7437, #12489, #12491 comment:3)
874+
var sel = editor.document.$.getSelection();
875+
if ( sel.type != 'None' && ( sel.anchorNode == fillingChar.$ || sel.focusNode == fillingChar.$ ) )
876+
selectionBookmark = createNativeSelectionBookmark( sel );
876877

877878
fillingCharBefore = fillingChar.getText();
878879
fillingChar.setText( replaceFillingChar( fillingCharBefore ) );
@@ -889,9 +890,9 @@
889890
if ( fillingChar ) {
890891
fillingChar.setText( fillingCharBefore );
891892

892-
if ( resetSelection ) {
893-
editor.document.$.defaultView.getSelection().setPosition( fillingChar.$, fillingChar.getLength() );
894-
resetSelection = 0;
893+
if ( selectionBookmark ) {
894+
moveNativeSelectionToBookmark( editor.document.$, selectionBookmark );
895+
selectionBookmark = null;
895896
}
896897
}
897898
}

‎tests/core/selection/editor.js

Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,27 @@ bender.test( {
3737
assert.areSame( expected || source, bender.tools.getHtmlWithSelection( ed ) );
3838
},
3939

40+
setSelectionInEmptyInlineElement: function( editor ) {
41+
var editable = editor.editable(),
42+
range = editor.createRange();
43+
44+
editable.setHtml( '<p>x<u></u>x</p>' );
45+
46+
var uEl = editable.findOne( 'u' );
47+
48+
range.moveToPosition( uEl, CKEDITOR.POSITION_AFTER_START );
49+
editor.getSelection().selectRanges( [ range ] );
50+
},
51+
52+
assertFillingChar: function( editable, parent, contents, msg ) {
53+
var fillingChar = editable.getCustomData( 'cke-fillingChar' );
54+
assert.isTrue( !!fillingChar, 'Filling char exists - ' + msg );
55+
assert.areSame( parent, fillingChar.getParent(), 'Filling char parent - ' + msg );
56+
assert.areSame( contents, fillingChar.getText(), 'Filling char contents - ' + msg );
57+
58+
return fillingChar;
59+
},
60+
4061
'test editor selection with no focus' : function() {
4162
var ed = this.editor;
4263

@@ -444,6 +465,211 @@ bender.test( {
444465
} );
445466
},
446467

468+
'test filling char is removed and restored when taking snapshot': function() {
469+
if ( !CKEDITOR.env.webkit )
470+
assert.ignore();
471+
472+
var editor = this.editor,
473+
bot = this.editorBot,
474+
editable = editor.editable(),
475+
range = editor.createRange();
476+
477+
this.setSelectionInEmptyInlineElement( editor );
478+
479+
var uEl = editable.findOne( 'u' ),
480+
fillingChar = this.assertFillingChar( editable, uEl, '\u200b', 'after set selection' );
481+
482+
editor.fire( 'beforeUndoImage' );
483+
this.assertFillingChar( editable, uEl, '', 'after beforeUndoImage' );
484+
485+
editor.fire( 'afterUndoImage' );
486+
fillingChar = this.assertFillingChar( editable, uEl, '\u200b', 'after afterUndoImage' );
487+
488+
range = editor.getSelection().getRanges()[ 0 ];
489+
assert.areSame( fillingChar, range.startContainer, 'Selection was restored - container' );
490+
assert.areSame( 1, range.startOffset, 'Selection was restored - offset after ZWS' );
491+
},
492+
493+
// #12489
494+
'test filling char is removed and restored when taking snapshot if selection is not right after the filling char': function() {
495+
if ( !CKEDITOR.env.webkit )
496+
assert.ignore();
497+
498+
var editor = this.editor,
499+
bot = this.editorBot,
500+
editable = editor.editable(),
501+
range = editor.createRange();
502+
503+
this.setSelectionInEmptyInlineElement( editor );
504+
505+
var uEl = editable.findOne( 'u' ),
506+
fillingChar = this.assertFillingChar( editable, uEl, '\u200b', 'after set selection' );
507+
508+
// Happens when typing and navigating...
509+
// Setting selection using native API to avoid losing the filling char on selection.setRanges().
510+
fillingChar.setText( fillingChar.getText() + 'abcd' );
511+
editor.document.$.getSelection().setPosition( fillingChar.$, 3 ); // ZWSab^cd
512+
513+
this.assertFillingChar( editable, uEl, '\u200babcd', 'after type' );
514+
515+
editor.fire( 'beforeUndoImage' );
516+
this.assertFillingChar( editable, uEl, 'abcd', 'after beforeUndoImage' );
517+
518+
editor.fire( 'afterUndoImage' );
519+
fillingChar = this.assertFillingChar( editable, uEl, '\u200babcd', 'after afterUndoImage' );
520+
521+
range = editor.getSelection().getRanges()[ 0 ];
522+
assert.areSame( fillingChar, range.startContainer, 'Selection was restored - container' );
523+
assert.areSame( 3, range.startOffset, 'Selection was restored - offset in ZWSab^cd' );
524+
},
525+
526+
// #8617
527+
'test selection is preserved when removing filling char on left-arrow': function() {
528+
if ( !CKEDITOR.env.webkit )
529+
assert.ignore();
530+
531+
var editor = this.editor,
532+
bot = this.editorBot,
533+
editable = editor.editable(),
534+
range = editor.createRange();
535+
536+
this.setSelectionInEmptyInlineElement( editor );
537+
538+
var uEl = editable.findOne( 'u' ),
539+
fillingChar = this.assertFillingChar( editable, uEl, '\u200b', 'after setting selection' );
540+
541+
// Happens when typing and navigating...
542+
// Setting selection using native API to avoid losing the filling char on selection.setRanges().
543+
fillingChar.setText( fillingChar.getText() + 'abc' );
544+
editor.document.$.getSelection().setPosition( fillingChar.$, 4 ); // ZWSabc^
545+
546+
this.assertFillingChar( editable, uEl, '\u200babc', 'after typing' );
547+
548+
// Mock LEFT arrow.
549+
editor.document.fire( 'keydown', new CKEDITOR.dom.event( { keyCode: 37 } ) );
550+
551+
assert.areSame( 'abc', uEl.getHtml(), 'Filling char is removed on left-arrow press' );
552+
553+
range = editor.getSelection().getRanges()[ 0 ];
554+
assert.areSame( uEl.getFirst(), range.startContainer, 'Selection was restored - container' );
555+
assert.areSame( 3, range.startOffset, 'Selection was restored - offset in abc^' );
556+
},
557+
558+
// #12419
559+
'test selection is preserved when removing filling char on select all': function() {
560+
if ( !CKEDITOR.env.webkit )
561+
assert.ignore();
562+
563+
var editor = this.editor,
564+
bot = this.editorBot,
565+
editable = editor.editable(),
566+
range = editor.createRange(),
567+
htmlMatchingOpts = {
568+
compareSelection: true,
569+
normalizeSelection: true
570+
};
571+
572+
this.setSelectionInEmptyInlineElement( editor );
573+
574+
var uEl = editable.findOne( 'u' ),
575+
fillingChar = this.assertFillingChar( editable, uEl, '\u200b', 'after setting selection' );
576+
577+
// Happens when typing and navigating...
578+
// Setting selection using native API to avoid losing the filling char on selection.setRanges().
579+
fillingChar.setText( fillingChar.getText() + 'abc' );
580+
editor.document.$.getSelection().setPosition( fillingChar.$, 4 ); // ZWSabc^
581+
582+
this.assertFillingChar( editable, uEl, '\u200babc', 'after typing' );
583+
584+
// Select all contents.
585+
range.selectNodeContents( editable.findOne( 'p' ) );
586+
editor.getSelection().selectRanges( [ range ] );
587+
588+
assert.areSame( 'abc', uEl.getHtml(), 'Filling char is removed on selection change' );
589+
assert.isInnerHtmlMatching( '<p>[x<u>abc</u>x]</p>', bender.tools.selection.getWithHtml( editor ),
590+
htmlMatchingOpts, 'Selection is correctly set' );
591+
},
592+
593+
'test direction of selection is preserved when removing filling char': function() {
594+
if ( !CKEDITOR.env.webkit )
595+
assert.ignore();
596+
597+
var editor = this.editor,
598+
bot = this.editorBot,
599+
editable = editor.editable(),
600+
range = editor.createRange();
601+
602+
this.setSelectionInEmptyInlineElement( editor );
603+
604+
var uEl = editable.findOne( 'u' ),
605+
fillingChar = this.assertFillingChar( editable, uEl, '\u200b', 'after setting selection' );
606+
607+
// Happens when typing and making selection from right to left...
608+
// Setting selection using native API to avoid losing the filling char on selection.setRanges().
609+
fillingChar.setText( fillingChar.getText() + 'abc' );
610+
range = editor.document.$.createRange();
611+
// ZWSabc]
612+
range.setStart( fillingChar.$, 4 );
613+
var nativeSel = editor.document.$.getSelection();
614+
nativeSel.removeAllRanges();
615+
nativeSel.addRange( range );
616+
// ZWSa[bc
617+
nativeSel.extend( fillingChar.$, 2 );
618+
619+
this.assertFillingChar( editable, uEl, '\u200babc', 'after typing' );
620+
621+
// Mock LEFT arrow.
622+
editor.document.fire( 'keydown', new CKEDITOR.dom.event( { keyCode: 37 } ) );
623+
624+
assert.areSame( 'abc', uEl.getHtml(), 'Filling char is removed on left-arrow press' );
625+
626+
nativeSel = editor.document.$.getSelection();
627+
assert.areSame( 3, nativeSel.anchorOffset, 'sel.anchorOffset' );
628+
assert.areSame( 1, nativeSel.focusOffset, 'sel.focusOffset' );
629+
},
630+
631+
// This particular scenario is reproducible when after typing in an empty inline element
632+
// user tries to select text by mouse from right to left in that element - selection is lost.
633+
// #12491 comment:3
634+
'test direction of selection is preserved when taking snapshot': function() {
635+
if ( !CKEDITOR.env.webkit )
636+
assert.ignore();
637+
638+
var editor = this.editor,
639+
bot = this.editorBot,
640+
editable = editor.editable(),
641+
range = editor.createRange();
642+
643+
this.setSelectionInEmptyInlineElement( editor );
644+
645+
var uEl = editable.findOne( 'u' ),
646+
fillingChar = this.assertFillingChar( editable, uEl, '\u200b', 'after set selection' );
647+
648+
// Happens when typing and making selection from right to left...
649+
// Setting selection using native API to avoid losing the filling char on selection.setRanges().
650+
fillingChar.setText( fillingChar.getText() + 'abc' );
651+
range = editor.document.$.createRange();
652+
// ZWSabc]
653+
range.setStart( fillingChar.$, 4 );
654+
var nativeSel = editor.document.$.getSelection();
655+
nativeSel.removeAllRanges();
656+
nativeSel.addRange( range );
657+
// ZWSa[bc
658+
nativeSel.extend( fillingChar.$, 2 );
659+
660+
this.assertFillingChar( editable, uEl, '\u200babc', 'after type' );
661+
662+
editor.fire( 'beforeUndoImage' );
663+
this.assertFillingChar( editable, uEl, 'abc', 'after beforeUndoImage' );
664+
665+
editor.fire( 'afterUndoImage' );
666+
this.assertFillingChar( editable, uEl, '\u200babc', 'after afterUndoImage' );
667+
668+
nativeSel = editor.document.$.getSelection();
669+
assert.areSame( 4, nativeSel.anchorOffset, 'sel.anchorOffset' );
670+
assert.areSame( 2, nativeSel.focusOffset, 'sel.focusOffset' );
671+
},
672+
447673
'test selection in source mode': function() {
448674
bender.editorBot.create( {
449675
name: 'test_editor_source',

0 commit comments

Comments
 (0)