Skip to content

Commit

Permalink
Merge branch 't/12630d'
Browse files Browse the repository at this point in the history
  • Loading branch information
Reinmar committed Nov 18, 2014
2 parents 63d5130 + 3103e96 commit 9fc1a76
Show file tree
Hide file tree
Showing 24 changed files with 598 additions and 260 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Fixed Issues:
* [#12141](http://dev.ckeditor.com/ticket/12141): Fixed: List items are lost when indenting a list item with a content wrapped with a block element. * [#12141](http://dev.ckeditor.com/ticket/12141): Fixed: List items are lost when indenting a list item with a content wrapped with a block element.
* [#12515](http://dev.ckeditor.com/ticket/12515): Fixed: Cursor is in the wrong position when executing undo command after adding image and typing some text. * [#12515](http://dev.ckeditor.com/ticket/12515): Fixed: Cursor is in the wrong position when executing undo command after adding image and typing some text.
* [#12621](http://dev.ckeditor.com/ticket/12621): Fixed: Can't remove styles in empty lines. * [#12621](http://dev.ckeditor.com/ticket/12621): Fixed: Can't remove styles in empty lines.
* [#12630](http://dev.ckeditor.com/ticket/12630): [Chrome] Fixed: Selection is placed outside paragraph when clicked the New Page button. This patch significantly simplified the way how the initial selection (a selection after contents of the editable is overwritten) is being fixed. That might have fixed many related scenarios on all browsers.


Other Changes: Other Changes:
* [#12550](http://dev.ckeditor.com/ticket/12550): Added CKEDITOR.dtd.main. * [#12550](http://dev.ckeditor.com/ticket/12550): Added CKEDITOR.dtd.main.
Expand Down
15 changes: 6 additions & 9 deletions bender.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -56,10 +56,7 @@ var config = {
'tests/core/editable/keystrokes/delbackspacequirks/collapsed#test backspace #9': 'env.safari', 'tests/core/editable/keystrokes/delbackspacequirks/collapsed#test backspace #9': 'env.safari',
'tests/core/editable/keystrokes/delbackspacequirks/collapsed#test backspace, merge #2': 'env.safari', 'tests/core/editable/keystrokes/delbackspacequirks/collapsed#test backspace, merge #2': 'env.safari',
'tests/core/editable/keystrokes/delbackspacequirks/collapsed#test backspace, merge #3': 'env.safari', 'tests/core/editable/keystrokes/delbackspacequirks/collapsed#test backspace, merge #3': 'env.safari',
'tests/core/editable/keystrokes/delbackspacequirks/collapsed#test backspace, merge #8': 'env.safari', 'tests/core/editable/keystrokes/delbackspacequirks/collapsed#test backspace, merge #8': 'env.safari'

// Firefox (#12104)
'tests/core/editor/focus#test blur after setData': 'env.gecko'
} }
}, },


Expand All @@ -84,11 +81,11 @@ var config = {
// Firefox (#11399) // Firefox (#11399)
'tests/plugins/widget/nestededitables#test selection in nested editable is preserved after opening and closing dialog - inline editor': 'env.gecko', 'tests/plugins/widget/nestededitables#test selection in nested editable is preserved after opening and closing dialog - inline editor': 'env.gecko',


// Firefox (#12104) // https://bugzilla.mozilla.org/show_bug.cgi?id=911201
'tests/plugins/widget/widgetselection#test focusing widget': 'env.gecko', 'tests/plugins/magicline/widgets#test commands[previous], first block in nested': 'env.gecko',
'tests/plugins/widget/widgetselection#test focusing by click': 'env.gecko', 'tests/plugins/magicline/widgets#test commands[next], block after block in nested': 'env.gecko',
'tests/plugins/widget/widgetselection#test focus editor when focusing widget by click': 'env.gecko', 'tests/plugins/magicline/widgets#test commands[previous], block before block in nested': 'env.gecko',
'tests/plugins/widget/widgetselection#test focus editor when focusing widget by method': 'env.gecko' 'tests/plugins/magicline/widgets#test commands[next], last block in nested': 'env.gecko'
} }
}, },


Expand Down
103 changes: 102 additions & 1 deletion core/editable.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -416,6 +416,7 @@
data = this.editor.dataProcessor.toHtml( data ); data = this.editor.dataProcessor.toHtml( data );


this.setHtml( data ); this.setHtml( data );
this.fixInitialSelection();


// Editable is ready after first setData. // Editable is ready after first setData.
if ( this.status == 'unloaded' ) if ( this.status == 'unloaded' )
Expand Down Expand Up @@ -465,7 +466,7 @@
}, },


/** /**
* Check if the editable is one of the host page element, indicates the * Checks if the editable is one of the host page element, indicates the
* an inline editing environment. * an inline editing environment.
* *
* @returns {Boolean} * @returns {Boolean}
Expand All @@ -474,6 +475,106 @@
return this.getDocument().equals( CKEDITOR.document ); return this.getDocument().equals( CKEDITOR.document );
}, },


/**
* Fixes the selection and focus which may be in incorrect state after
* editable's inner HTML has been overwritten.
*
* If the editable did not have focus, then the selection will be fixed when the editable
* is focused for the first time. If the editable already had focus, then the selection will
* be fixed immediately.
*
* To understand the problem see:
*
* * http://tests.ckeditor.dev:1030/tests/core/selection/manual/focusaftersettingdata
* * http://tests.ckeditor.dev:1030/tests/core/selection/manual/focusafterundoing
* * http://tests.ckeditor.dev:1030/tests/core/selection/manual/selectionafterfocusing
* * http://tests.ckeditor.dev:1030/tests/plugins/newpage/manual/selectionafternewpage
*
* @since 4.4.6
* @private
*/
fixInitialSelection: function() {
var that = this;

// Deal with IE8- (the old MS selection) first.
if ( CKEDITOR.env.ie && CKEDITOR.env.version < 9 ) {
if ( this.hasFocus ) {
this.focus();
fixMSSelection();
}

return;
}

// If editable did not have focus, fix the selection when it is first focused.
if ( !this.hasFocus ) {
this.once( 'focus', function() {
fixSelection();
}, null, null, -999 );
// If editable had focus, fix the selection immediately.
} else {
this.focus();
fixSelection();
}

function fixSelection() {
var $doc = that.getDocument().$,
$sel = $doc.getSelection();

if ( requiresFix( $sel ) ) {
var range = new CKEDITOR.dom.range( that );
range.moveToElementEditStart( that );

var $range = $doc.createRange();
$range.setStart( range.startContainer.$, range.startOffset );
$range.collapse( true );

$sel.removeAllRanges();
$sel.addRange( $range );
}
}

function requiresFix( $sel ) {
// This condition covers most broken cases after setting data.
if ( $sel.anchorNode && $sel.anchorNode == that.$ ) {
return true;
}

// Fix for:
// http://tests.ckeditor.dev:1030/tests/core/selection/manual/focusaftersettingdata
// (the inline editor TC)
if ( CKEDITOR.env.webkit ) {
var active = that.getDocument().getActive();
if ( active && active.equals( that ) && !$sel.anchorNode ) {
return true;
}
}
}

function fixMSSelection() {
var $doc = that.getDocument().$,
$sel = $doc.selection,
active = that.getDocument().getActive();

if ( $sel.type == 'None' && active.equals( that ) ) {
var range = new CKEDITOR.dom.range( that ),
parentElement,
$range = $doc.body.createTextRange();

range.moveToElementEditStart( that );

parentElement = range.startContainer;
if ( parentElement.type != CKEDITOR.NODE_ELEMENT ) {
parentElement = parentElement.getParent();
}

$range.moveToElementText( parentElement.$ );
$range.collapse( true );
$range.select();
}
}
},

/** /**
* Editable element bootstrapping. * Editable element bootstrapping.
* *
Expand Down
28 changes: 25 additions & 3 deletions core/htmldataprocessor.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@


editor.on( 'toHtml', function( evt ) { editor.on( 'toHtml', function( evt ) {
var evtData = evt.data, var evtData = evt.data,
data = evtData.dataValue; data = evtData.dataValue,
fixBodyTag;


// The source data is already HTML, but we need to clean // The source data is already HTML, but we need to clean
// it up and apply the filter. // it up and apply the filter.
Expand Down Expand Up @@ -116,10 +117,23 @@
// can be properly filtered. // can be properly filtered.
data = unprotectRealComments( data ); data = unprotectRealComments( data );


if ( evtData.fixForBody === false ) {
fixBodyTag = false;
} else {
fixBodyTag = getFixBodyTag( evtData.enterMode, editor.config.autoParagraph );
}

// Now use our parser to make further fixes to the structure, as // Now use our parser to make further fixes to the structure, as
// well as apply the filter. // well as apply the filter.
evtData.dataValue = CKEDITOR.htmlParser.fragment.fromHtml( data = CKEDITOR.htmlParser.fragment.fromHtml( data, evtData.context, fixBodyTag );
data, evtData.context, evtData.fixForBody === false ? false : getFixBodyTag( evtData.enterMode, editor.config.autoParagraph ) );
// The empty root element needs to be fixed by adding 'p' or 'div' into it.
// This avoids the need to create that element on the first focus (#12630).
if ( fixBodyTag ) {
fixEmptyRoot( data, fixBodyTag );
}

evtData.dataValue = data;
}, null, null, 5 ); }, null, null, 5 );


// Filter incoming "data". // Filter incoming "data".
Expand Down Expand Up @@ -907,6 +921,14 @@


return data; return data;
} }

// Creates a block if the root element is empty.
function fixEmptyRoot( root, fixBodyTag ) {
if ( !root.children.length && CKEDITOR.dtd[ root.name ][ fixBodyTag ] ) {
var fixBodyElement = new CKEDITOR.htmlParser.element( fixBodyTag );
root.add( fixBodyElement );
}
}
} )(); } )();


/** /**
Expand Down
114 changes: 26 additions & 88 deletions core/selection.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -224,44 +224,6 @@
sel.extend( bm[ 1 ].node, bm[ 1 ].offset ); sel.extend( bm[ 1 ].node, bm[ 1 ].offset );
} }


// Read the comments in selection constructor.
function fixInitialSelection( root, nativeSel, doFocus ) {
// It may happen that setting proper selection will
// cause focus to be fired (even without actually focusing root).
// Cancel it because focus shouldn't be fired when retriving selection. (#10115)
var listener = root.on( 'focus', function( evt ) {
evt.cancel();
}, null, null, -100 );

// FF && Webkit.
if ( !CKEDITOR.env.ie ) {
var range = new CKEDITOR.dom.range( root );
range.moveToElementEditStart( root );

var nativeRange = root.getDocument().$.createRange();
nativeRange.setStart( range.startContainer.$, range.startOffset );
nativeRange.collapse( 1 );

nativeSel.removeAllRanges();
nativeSel.addRange( nativeRange );
}
else {
// IE in specific case may also fire selectionchange.
// We cannot block bubbling selectionchange, so at least we
// can prevent from falling into inf recursion caused by fix for #9699
// (see wysiwygarea plugin).
// http://dev.ckeditor.com/ticket/10438#comment:13
var listener2 = root.getDocument().on( 'selectionchange', function( evt ) {
evt.cancel();
}, null, null, -100 );
}

doFocus && root.focus();

listener.removeListener();
listener2 && listener2.removeListener();
}

// Creates cke_hidden_sel container and puts real selection there. // Creates cke_hidden_sel container and puts real selection there.
function hideSelection( editor ) { function hideSelection( editor ) {
var style = CKEDITOR.env.ie ? 'display:none' : 'position:fixed;top:0;left:-1000px', var style = CKEDITOR.env.ie ? 'display:none' : 'position:fixed;top:0;left:-1000px',
Expand Down Expand Up @@ -830,14 +792,32 @@


// When loaded data are ready check whether hidden selection container was not loaded. // When loaded data are ready check whether hidden selection container was not loaded.
editor.on( 'loadSnapshot', function() { editor.on( 'loadSnapshot', function() {
// TODO replace with el.find() which will be introduced in #9764, var isElement = CKEDITOR.dom.walker.nodeType( CKEDITOR.NODE_ELEMENT ),
// because it may happen that hidden sel container won't be the last element. // TODO replace with el.find() which will be introduced in #9764,
var el = editor.editable().getLast( function( node ) { // because it may happen that hidden sel container won't be the last element.
return node.type == CKEDITOR.NODE_ELEMENT; last = editor.editable().getLast( isElement );
} );

if ( last && last.hasAttribute( 'data-cke-hidden-sel' ) ) {
if ( el && el.hasAttribute( 'data-cke-hidden-sel' ) ) last.remove();
el.remove();
// Firefox does a very unfortunate thing. When a non-editable element is the only
// element in the editable, when we remove the hidden selection container, Firefox
// will insert a bogus <br> at the beginning of the editable...
// See: https://bugzilla.mozilla.org/show_bug.cgi?id=911201
//
// This behavior is never desired because this <br> pushes the content lower, but in
// this case it is especially dangerous, because it happens when a bookmark is being restored.
// Since this <br> is inserted at the beginning it changes indexes and thus breaks the bookmark2
// what results in errors.
//
// So... let's revert what Firefox broke.
if ( CKEDITOR.env.gecko ) {
var first = editor.editable().getFirst( isElement );
if ( first && first.is( 'br' ) && first.getAttribute( '_moz_editor_bogus_node' ) ) {
first.remove();
}
}
}
}, null, null, 100 ); }, null, null, 100 );


editor.on( 'key', function( evt ) { editor.on( 'key', function( evt ) {
Expand Down Expand Up @@ -1134,48 +1114,6 @@
return this; return this;
} }


// On WebKit, it may happen that we've already have focus
// on the editable element while still having no selection
// available. We normalize it here by replicating the
// behavior of other browsers.
//
// Webkit's condition covers also the case when editable hasn't been focused
// at all. Thanks to this hack Webkit always has selection in the right place.
//
// On FF and IE we only fix the first case, when editable was activated
// but the selection is broken - usually this happens after setData if editor was focused.

var sel = isMSSelection ? this.document.$.selection : this.document.getWindow().$.getSelection();

if ( CKEDITOR.env.webkit ) {
if ( sel.type == 'None' && this.document.getActive().equals( root ) || sel.type == 'Caret' && sel.anchorNode.nodeType == CKEDITOR.NODE_DOCUMENT )
fixInitialSelection( root, sel );
}
else if ( CKEDITOR.env.gecko ) {
if ( sel && this.document.getActive().equals( root ) &&
sel.anchorNode && sel.anchorNode.nodeType == CKEDITOR.NODE_DOCUMENT )
fixInitialSelection( root, sel, true );
}
else if ( CKEDITOR.env.ie ) {
var active = this.document.getActive();

// IEs 9+.
if ( !isMSSelection ) {
var anchorNode = sel && sel.anchorNode;

if ( anchorNode )
anchorNode = new CKEDITOR.dom.node( anchorNode );

if ( active && active.equals( this.document.getDocumentElement() ) &&
anchorNode && ( root.equals( anchorNode ) || root.contains( anchorNode ) ) )
fixInitialSelection( root, null, true );
}
// IEs 7&8.
else if ( sel.type == 'None' && active && active.equals( this.document.getDocumentElement() ) ) {
fixInitialSelection( root, null, true );
}
}

// Check whether browser focus is really inside of the editable element. // Check whether browser focus is really inside of the editable element.


var nativeSel = this.getNative(), var nativeSel = this.getNative(),
Expand Down
27 changes: 3 additions & 24 deletions plugins/wysiwygarea/plugin.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -142,30 +142,6 @@
script = doc.getElementById( 'cke_basetagscrpt' ); script = doc.getElementById( 'cke_basetagscrpt' );
script && script.parentNode.removeChild( script ); script && script.parentNode.removeChild( script );


if ( CKEDITOR.env.gecko ) {
// Force Gecko to change contentEditable from false to true on domReady
// (because it's previously set to true on iframe's body creation).
// Otherwise del/backspace and some other editable features will be broken in Fx <4
// See: #107 and https://bugzilla.mozilla.org/show_bug.cgi?id=440916
body.contentEditable = false;

// Remove any leading <br> which is between the <body> and the comment.
// This one fixes Firefox 3.6 bug: the browser inserts a leading <br>
// on document.write if the body has contenteditable="true".
if ( CKEDITOR.env.version < 20000 ) {
body.innerHTML = body.innerHTML.replace( /^.*<!-- cke-content-start -->/, '' );

// The above hack messes up the selection in FF36.
// To clean this up, manually select collapsed range that
// starts within the body.
setTimeout( function() {
var range = new CKEDITOR.dom.range( new CKEDITOR.dom.document( doc ) );
range.setStart( new CKEDITOR.dom.node( body ), 0 );
editor.getSelection().selectRanges( [ range ] );
}, 0 );
}
}

body.contentEditable = true; body.contentEditable = true;


if ( CKEDITOR.env.ie ) { if ( CKEDITOR.env.ie ) {
Expand All @@ -186,6 +162,7 @@
doc = new CKEDITOR.dom.document( doc ); doc = new CKEDITOR.dom.document( doc );


this.setup(); this.setup();
this.fixInitialSelection();


if ( CKEDITOR.env.ie ) { if ( CKEDITOR.env.ie ) {
doc.getDocumentElement().addClass( doc.$.compatMode ); doc.getDocumentElement().addClass( doc.$.compatMode );
Expand Down Expand Up @@ -350,6 +327,8 @@


if ( isSnapshot ) { if ( isSnapshot ) {
this.setHtml( data ); this.setHtml( data );
this.fixInitialSelection();

// Fire dataReady for the consistency with inline editors // Fire dataReady for the consistency with inline editors
// and because it makes sense. (#10370) // and because it makes sense. (#10370)
editor.fire( 'dataReady' ); editor.fire( 'dataReady' );
Expand Down
Loading

0 comments on commit 9fc1a76

Please sign in to comment.