From 002e356a1152b66821836b828d00aacf2f804135 Mon Sep 17 00:00:00 2001
From: Arkadiusz Filipczak <ark.flp@gmail.com>
Date: Fri, 13 Aug 2021 15:32:12 +0200
Subject: [PATCH 1/4] Highlight at the end - failing tests

---
 .../tests/highlightcommand.js                 | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/packages/ckeditor5-highlight/tests/highlightcommand.js b/packages/ckeditor5-highlight/tests/highlightcommand.js
index 1f4a1036e72..350c56293c1 100644
--- a/packages/ckeditor5-highlight/tests/highlightcommand.js
+++ b/packages/ckeditor5-highlight/tests/highlightcommand.js
@@ -187,6 +187,34 @@ describe( 'HighlightCommand', () => {
 					expect( command.value ).to.equal( 'pinkMarker' );
 					expect( doc.selection.getAttribute( 'highlight' ) ).to.equal( 'pinkMarker' );
 				} );
+
+				// https://github.com/ckeditor/ckeditor5/issues/2616
+				it( 'should not change entire highlight when at the end of highlighted text', () => {
+					setData( model, '<p>abc<$text highlight="yellowMarker">foobar</$text>[]xyz</p>' );
+
+					expect( command.value ).to.equal( 'yellowMarker' );
+
+					command.execute( { value: 'greenMarker' } );
+
+					expect( getData( model ) ).to.equal(
+						'<p>abc<$text highlight="yellowMarker">foobar</$text><$text highlight="greenMarker">[]</$text>xyz</p>'
+					);
+
+					expect( command.value ).to.equal( 'greenMarker' );
+				} );
+
+				// https://github.com/ckeditor/ckeditor5/issues/2616
+				it( 'should not remove entire highlight when at the end of highlighted text of the same value', () => {
+					setData( model, '<p>abc<$text highlight="yellowMarker">foobar</$text>[]xyz</p>' );
+
+					expect( command.value ).to.equal( 'yellowMarker' );
+
+					command.execute( { value: 'yellowMarker' } );
+
+					expect( getData( model ) ).to.equal( '<p>abc<$text highlight="yellowMarker">foobar</$text>[]xyz</p>' );
+
+					expect( command.value ).to.be.undefined;
+				} );
 			} );
 
 			describe( 'on not collapsed range', () => {
@@ -250,6 +278,19 @@ describe( 'HighlightCommand', () => {
 
 					expect( command.value ).to.be.undefined;
 				} );
+
+				// https://github.com/ckeditor/ckeditor5/issues/2616
+				it( 'should not remove entire highlight when at the end of highlighted text', () => {
+					setData( model, '<p>abc<$text highlight="yellowMarker">foobar</$text>[]xyz</p>' );
+
+					expect( command.value ).to.equal( 'yellowMarker' );
+
+					command.execute();
+
+					expect( getData( model ) ).to.equal( '<p>abc<$text highlight="yellowMarker">foobar</$text>[]xyz</p>' );
+
+					expect( command.value ).to.be.undefined;
+				} );
 			} );
 
 			describe( 'on not collapsed range', () => {

From 3bad75033f9572e313243ce3df147ff1ab13163f Mon Sep 17 00:00:00 2001
From: Arkadiusz Filipczak <ark.flp@gmail.com>
Date: Fri, 13 Aug 2021 15:43:06 +0200
Subject: [PATCH 2/4] Highlight at the end - implementation

---
 packages/ckeditor5-highlight/src/highlightcommand.js | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/packages/ckeditor5-highlight/src/highlightcommand.js b/packages/ckeditor5-highlight/src/highlightcommand.js
index 418fdcf1870..7a8b42307c3 100644
--- a/packages/ckeditor5-highlight/src/highlightcommand.js
+++ b/packages/ckeditor5-highlight/src/highlightcommand.js
@@ -72,16 +72,20 @@ export default class HighlightCommand extends Command {
 					const highlightStart = position.getLastMatchingPosition( isSameHighlight, { direction: 'backward' } );
 					const highlightEnd = position.getLastMatchingPosition( isSameHighlight );
 
-					const highlightRange = writer.createRange( highlightStart, highlightEnd );
+					const highlightRange = position.isEqual( highlightEnd ) ? null : writer.createRange( highlightStart, highlightEnd );
 
 					// Then depending on current value...
 					if ( !highlighter || this.value === highlighter ) {
 						// ...remove attribute when passing highlighter different then current or executing "eraser".
-						writer.removeAttribute( 'highlight', highlightRange );
+						if ( highlightRange ) {
+							writer.removeAttribute( 'highlight', highlightRange );
+						}
 						writer.removeSelectionAttribute( 'highlight' );
 					} else {
 						// ...update `highlight` value.
-						writer.setAttribute( 'highlight', highlighter, highlightRange );
+						if ( highlightRange ) {
+							writer.setAttribute( 'highlight', highlighter, highlightRange );
+						}
 						writer.setSelectionAttribute( 'highlight', highlighter );
 					}
 				} else if ( highlighter ) {

From a71eed72cbcc527870026aef5ab6ad1f5cb99490 Mon Sep 17 00:00:00 2001
From: Arkadiusz Filipczak <ark.flp@gmail.com>
Date: Mon, 16 Aug 2021 12:43:27 +0200
Subject: [PATCH 3/4] Highlight at the end - fixes after review

---
 .../ckeditor5-highlight/src/highlightcommand.js   | 12 +++++++++---
 .../ckeditor5-highlight/tests/highlightcommand.js | 15 ++++++++++-----
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/packages/ckeditor5-highlight/src/highlightcommand.js b/packages/ckeditor5-highlight/src/highlightcommand.js
index 7a8b42307c3..2ad7f654f39 100644
--- a/packages/ckeditor5-highlight/src/highlightcommand.js
+++ b/packages/ckeditor5-highlight/src/highlightcommand.js
@@ -72,20 +72,26 @@ export default class HighlightCommand extends Command {
 					const highlightStart = position.getLastMatchingPosition( isSameHighlight, { direction: 'backward' } );
 					const highlightEnd = position.getLastMatchingPosition( isSameHighlight );
 
-					const highlightRange = position.isEqual( highlightEnd ) ? null : writer.createRange( highlightStart, highlightEnd );
+					const highlightRange = writer.createRange( highlightStart, highlightEnd );
 
 					// Then depending on current value...
 					if ( !highlighter || this.value === highlighter ) {
 						// ...remove attribute when passing highlighter different then current or executing "eraser".
-						if ( highlightRange ) {
+
+						// If we're at the end of the highlighted range, we don't want to remove highlight of the range.
+						if ( !position.isEqual( highlightEnd ) ) {
 							writer.removeAttribute( 'highlight', highlightRange );
 						}
+
 						writer.removeSelectionAttribute( 'highlight' );
 					} else {
 						// ...update `highlight` value.
-						if ( highlightRange ) {
+
+						// If we're at the end of the highlighted range, we don't want to change the highlight of the range.
+						if ( !position.isEqual( highlightEnd ) ) {
 							writer.setAttribute( 'highlight', highlighter, highlightRange );
 						}
+
 						writer.setSelectionAttribute( 'highlight', highlighter );
 					}
 				} else if ( highlighter ) {
diff --git a/packages/ckeditor5-highlight/tests/highlightcommand.js b/packages/ckeditor5-highlight/tests/highlightcommand.js
index 350c56293c1..81590644c69 100644
--- a/packages/ckeditor5-highlight/tests/highlightcommand.js
+++ b/packages/ckeditor5-highlight/tests/highlightcommand.js
@@ -188,7 +188,6 @@ describe( 'HighlightCommand', () => {
 					expect( doc.selection.getAttribute( 'highlight' ) ).to.equal( 'pinkMarker' );
 				} );
 
-				// https://github.com/ckeditor/ckeditor5/issues/2616
 				it( 'should not change entire highlight when at the end of highlighted text', () => {
 					setData( model, '<p>abc<$text highlight="yellowMarker">foobar</$text>[]xyz</p>' );
 
@@ -196,14 +195,13 @@ describe( 'HighlightCommand', () => {
 
 					command.execute( { value: 'greenMarker' } );
 
-					expect( getData( model ) ).to.equal(
-						'<p>abc<$text highlight="yellowMarker">foobar</$text><$text highlight="greenMarker">[]</$text>xyz</p>'
+					expect( getData( model, { withoutSelection: true } ) ).to.equal(
+						'<p>abc<$text highlight="yellowMarker">foobar</$text>xyz</p>'
 					);
 
 					expect( command.value ).to.equal( 'greenMarker' );
 				} );
 
-				// https://github.com/ckeditor/ckeditor5/issues/2616
 				it( 'should not remove entire highlight when at the end of highlighted text of the same value', () => {
 					setData( model, '<p>abc<$text highlight="yellowMarker">foobar</$text>[]xyz</p>' );
 
@@ -215,6 +213,14 @@ describe( 'HighlightCommand', () => {
 
 					expect( command.value ).to.be.undefined;
 				} );
+
+				it( 'should change selection attribute when at the end of highlighted text', () => {
+					setData( model, '<p>abc<$text highlight="yellowMarker">foobar</$text>[]xyz</p>' );
+
+					command.execute( { value: 'greenMarker' } );
+
+					expect( doc.selection.getAttribute( 'highlight' ) ).to.equal( 'greenMarker' );
+				} );
 			} );
 
 			describe( 'on not collapsed range', () => {
@@ -279,7 +285,6 @@ describe( 'HighlightCommand', () => {
 					expect( command.value ).to.be.undefined;
 				} );
 
-				// https://github.com/ckeditor/ckeditor5/issues/2616
 				it( 'should not remove entire highlight when at the end of highlighted text', () => {
 					setData( model, '<p>abc<$text highlight="yellowMarker">foobar</$text>[]xyz</p>' );
 

From b0ff7b9ba91db24e6046baaa2d94ddcf56ecb1d1 Mon Sep 17 00:00:00 2001
From: Arkadiusz Filipczak <ark.flp@gmail.com>
Date: Tue, 17 Aug 2021 12:55:25 +0200
Subject: [PATCH 4/4] Highlight at the end - fixes after review

---
 packages/ckeditor5-highlight/src/highlightcommand.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/ckeditor5-highlight/src/highlightcommand.js b/packages/ckeditor5-highlight/src/highlightcommand.js
index 2ad7f654f39..507b9c97cee 100644
--- a/packages/ckeditor5-highlight/src/highlightcommand.js
+++ b/packages/ckeditor5-highlight/src/highlightcommand.js
@@ -57,8 +57,6 @@ export default class HighlightCommand extends Command {
 		const highlighter = options.value;
 
 		model.change( writer => {
-			const ranges = model.schema.getValidRanges( selection.getRanges(), 'highlight' );
-
 			if ( selection.isCollapsed ) {
 				const position = selection.getFirstPosition();
 
@@ -98,6 +96,8 @@ export default class HighlightCommand extends Command {
 					writer.setSelectionAttribute( 'highlight', highlighter );
 				}
 			} else {
+				const ranges = model.schema.getValidRanges( selection.getRanges(), 'highlight' );
+
 				for ( const range of ranges ) {
 					if ( highlighter ) {
 						writer.setAttribute( 'highlight', highlighter, range );