diff --git a/src/utils/bindtwostepcarettoattribute.js b/src/utils/bindtwostepcarettoattribute.js index 9c7450297..e8da14c55 100644 --- a/src/utils/bindtwostepcarettoattribute.js +++ b/src/utils/bindtwostepcarettoattribute.js @@ -370,12 +370,29 @@ class TwoStepCaretHandler { // <$text attribute>bar{} // if ( position.isAtEnd && isAtEndBoundary( position, attribute ) ) { - // DON'T ENGAGE 2-SCM if gravity is already overridden. The selection has the attribute already. + // DON'T ENGAGE 2-SCM if the selection has the attribute already. // This is a common selection if set using the mouse. // // <$text attribute>bar{} // if ( this._hasSelectionAttribute ) { + // DON'T ENGAGE 2-SCM if the attribute at the end of the block which has length == 1. + // Make sure the selection will not the attribute after it moves backwards. + // + // foo<$text attribute>b{} + // + if ( isStepAfterTheAttributeBoundary( position, attribute ) ) { + // Skip the automatic gravity restore upon the next selection#change:range event. + // If not skipped, it would automatically restore the gravity, which should remain + // overridden. + this._skipNextAutomaticGravityRestoration(); + this._overrideGravity(); + + // Don't return "true" here because we didn't call _preventCaretMovement. + // Returning here will destabilize the filler logic, which also listens to + // keydown (and the event would be stopped). + } + return; } // ENGAGE 2-SCM if the selection has no attribute. This may happen when the user @@ -413,16 +430,16 @@ class TwoStepCaretHandler { // foo<$text attribute>b{}arbaz // foo<$text attribute>barb{}az // - if ( isAtBoundary( position.getShiftedBy( -1 ), attribute ) ) { + if ( isStepAfterTheAttributeBoundary( position, attribute ) ) { // Skip the automatic gravity restore upon the next selection#change:range event. // If not skipped, it would automatically restore the gravity, which should remain // overridden. this._skipNextAutomaticGravityRestoration(); this._overrideGravity(); - // Don't return "true" here because we didn't call preventDefault. + // Don't return "true" here because we didn't call _preventCaretMovement. // Returning here will destabilize the filler logic, which also listens to - // keydown (and it will be stopped). + // keydown (and the event would be stopped). } } } @@ -565,3 +582,9 @@ function isBetweenDifferentValues( position, attribute ) { return nodeAfter.getAttribute( attribute ) !== nodeBefore.getAttribute( attribute ); } + +// @param {module:engine/model/position~Position} position +// @param {String} attribute +function isStepAfterTheAttributeBoundary( position, attribute ) { + return isAtBoundary( position.getShiftedBy( -1 ), attribute ); +} diff --git a/tests/utils/bindtwostepcarettoattribute.js b/tests/utils/bindtwostepcarettoattribute.js index d1b282ac7..a3c867f42 100644 --- a/tests/utils/bindtwostepcarettoattribute.js +++ b/tests/utils/bindtwostepcarettoattribute.js @@ -324,7 +324,7 @@ describe( 'bindTwoStepCaretToAttribute()', () => { { selectionAttributes: [ 'a' ], isGravityOverridden: false, preventDefault: 0, evtStopCalled: 0 }, '←', // <$text a="1">{}x - { selectionAttributes: [ 'a' ], isGravityOverridden: false, preventDefault: 0, evtStopCalled: 0 }, + { selectionAttributes: [ 'a' ], isGravityOverridden: true, preventDefault: 0, evtStopCalled: 0 }, '←', // {}<$text a="1">x (because it's a first-child) { selectionAttributes: [], isGravityOverridden: false, preventDefault: 1, evtStopCalled: 1 }, @@ -345,7 +345,7 @@ describe( 'bindTwoStepCaretToAttribute()', () => { '←', { selectionAttributes: [ 'a' ], isGravityOverridden: false, preventDefault: 1, evtStopCalled: 1 }, '←', - { selectionAttributes: [ 'a' ], isGravityOverridden: false, preventDefault: 1, evtStopCalled: 1 }, + { selectionAttributes: [ 'a' ], isGravityOverridden: true, preventDefault: 1, evtStopCalled: 1 }, '←', { selectionAttributes: [], isGravityOverridden: false, preventDefault: 2, evtStopCalled: 2 }, '←', @@ -353,6 +353,24 @@ describe( 'bindTwoStepCaretToAttribute()', () => { ] ); } ); + // https://github.com/ckeditor/ckeditor5-engine/issues/1301 + it( 'should handle passing through the only-child with an attribute (single character, text before)', () => { + setData( model, 'abc<$text a="1">x[]' ); + + testTwoStepCaretMovement( [ + { selectionAttributes: [ 'a' ], isGravityOverridden: false, preventDefault: 0, evtStopCalled: 0 }, + '←', + // abc<$text a="1">{}x + { selectionAttributes: [ 'a' ], isGravityOverridden: true, preventDefault: 0, evtStopCalled: 0 }, + '←', + // abc{}<$text a="1">x (because it's a first-child) + { selectionAttributes: [], isGravityOverridden: false, preventDefault: 1, evtStopCalled: 1 }, + '←', + // abc{}<$text a="1">x + { selectionAttributes: [], isGravityOverridden: false, preventDefault: 1, evtStopCalled: 1 } + ] ); + } ); + // https://github.com/ckeditor/ckeditor5-engine/issues/1301 it( 'should handle passing through the only-child with an attribute (multiple characters)', () => { setData( model, '<$text a="1">xyz[]' );