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

Commit

Permalink
Fix: The two-step caret movement could handle the single-character-at…
Browse files Browse the repository at this point in the history
…tribute-preceded-by-some-text-at-the-end-of-the-block-case.
  • Loading branch information
oleq committed Apr 18, 2018
1 parent 8f964bd commit 446edb9
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 6 deletions.
31 changes: 27 additions & 4 deletions src/utils/bindtwostepcarettoattribute.js
Original file line number Diff line number Diff line change
Expand Up @@ -370,12 +370,29 @@ class TwoStepCaretHandler {
// <paragraph><$text attribute>bar</$text>{}</paragraph>
//
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.
//
// <paragraph><$text attribute>bar{}</$text></paragraph>
//
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.
//
// <paragraph>foo<$text attribute>b{}</$text></paragraph>
//
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
Expand Down Expand Up @@ -413,16 +430,16 @@ class TwoStepCaretHandler {
// <paragraph>foo<$text attribute>b{}ar</$text>baz</paragraph>
// <paragraph>foo<$text attribute>bar</$text>b{}az</paragraph>
//
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).
}
}
}
Expand Down Expand Up @@ -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 );
}
22 changes: 20 additions & 2 deletions tests/utils/bindtwostepcarettoattribute.js
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ describe( 'bindTwoStepCaretToAttribute()', () => {
{ selectionAttributes: [ 'a' ], isGravityOverridden: false, preventDefault: 0, evtStopCalled: 0 },
'←',
// <$text a="1">{}x</$text>
{ selectionAttributes: [ 'a' ], isGravityOverridden: false, preventDefault: 0, evtStopCalled: 0 },
{ selectionAttributes: [ 'a' ], isGravityOverridden: true, preventDefault: 0, evtStopCalled: 0 },
'←',
// {}<$text a="1">x</$text> (because it's a first-child)
{ selectionAttributes: [], isGravityOverridden: false, preventDefault: 1, evtStopCalled: 1 },
Expand All @@ -345,14 +345,32 @@ 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 },
'←',
{ selectionAttributes: [], isGravityOverridden: false, preventDefault: 2, evtStopCalled: 2 }
] );
} );

// 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</$text>[]' );

testTwoStepCaretMovement( [
{ selectionAttributes: [ 'a' ], isGravityOverridden: false, preventDefault: 0, evtStopCalled: 0 },
'←',
// abc<$text a="1">{}x</$text>
{ selectionAttributes: [ 'a' ], isGravityOverridden: true, preventDefault: 0, evtStopCalled: 0 },
'←',
// abc{}<$text a="1">x</$text> (because it's a first-child)
{ selectionAttributes: [], isGravityOverridden: false, preventDefault: 1, evtStopCalled: 1 },
'←',
// abc{}<$text a="1">x</$text>
{ 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</$text>[]' );
Expand Down

0 comments on commit 446edb9

Please sign in to comment.