Sfoglia il codice sorgente

Fix: The two-step caret movement could handle the single-character-attribute-preceded-by-some-text-at-the-end-of-the-block-case.

Aleksander Nowodzinski 7 anni fa
parent
commit
58fe0f8a2b

+ 27 - 4
packages/ckeditor5-engine/src/utils/bindtwostepcarettoattribute.js

@@ -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
@@ -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).
 			}
 		}
 	}
@@ -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 );
+}

+ 20 - 2
packages/ckeditor5-engine/tests/utils/bindtwostepcarettoattribute.js

@@ -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 },
@@ -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</$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>[]' );