Merge pull request #15373 from ckeditor/ck/15051

Fix (html-support): The `DocumentSelection` should not store the GHS `linkA` attribute if the `linkHref` attribute was removed by the two-step caret movement feature. Closes #15051. Other (link, typing): The logic behind the two-step caret movement extracted to the common code in the two-step caret movement feature. Other (typing): Unified behavior of the `insertText` command for cases using the `DocumentSelection` and `Selection` as applied attributes behaved differently in those cases.
ckeditor · Nov 22, 2023 · 73c2985 · 73c2985
2 parents ad94655 + 2e858ed
commit 73c2985
Show file tree

Hide file tree

Showing 12 changed files with 1,456 additions and 582 deletions.
diff --git a/packages/ckeditor5-engine/src/index.ts b/packages/ckeditor5-engine/src/index.ts
@@ -78,7 +78,8 @@ export { transformSets } from './model/operation/transform';
 export {
 	default as DocumentSelection,
 	type DocumentSelectionChangeRangeEvent,
-	type DocumentSelectionChangeMarkerEvent
+	type DocumentSelectionChangeMarkerEvent,
+	type DocumentSelectionChangeAttributeEvent
 } from './model/documentselection';
 export { default as Range } from './model/range';
 export { default as LiveRange, type LiveRangeChangeRangeEvent } from './model/liverange';

diff --git a/packages/ckeditor5-html-support/src/datafilter.ts b/packages/ckeditor5-html-support/src/datafilter.ts
@@ -16,7 +16,8 @@ import {
 	type ViewElement,
 	type MatchResult,
 	type ViewConsumable,
-	type MatcherObjectPattern
+	type MatcherObjectPattern,
+	type DocumentSelectionChangeAttributeEvent
 } from 'ckeditor5/src/engine';
 
 import {
@@ -438,6 +439,7 @@ export default class DataFilter extends Plugin {
 	 */
 	private _registerCoupledAttributesPostFixer() {
 		const model = this.editor.model;
+		const selection = model.document.selection;
 
 		model.document.registerPostFixer( writer => {
 			const changes = model.document.differ.getChanges();
@@ -471,6 +473,41 @@ export default class DataFilter extends Plugin {
 
 			return changed;
 		} );
+
+		this.listenTo<DocumentSelectionChangeAttributeEvent>( selection, 'change:attribute', ( evt, { attributeKeys } ) => {
+			const removeAttributes = new Set<string>();
+			const coupledAttributes = this._getCoupledAttributesMap();
+
+			for ( const attributeKey of attributeKeys ) {
+				// Handle only attribute removals.
+				if ( selection.hasAttribute( attributeKey ) ) {
+					continue;
+				}
+
+				// Find a list of coupled GHS attributes.
+				const coupledAttributeKeys = coupledAttributes.get( attributeKey );
+
+				if ( !coupledAttributeKeys ) {
+					continue;
+				}
+
+				for ( const coupledAttributeKey of coupledAttributeKeys ) {
+					if ( selection.hasAttribute( coupledAttributeKey ) ) {
+						removeAttributes.add( coupledAttributeKey );
+					}
+				}
+			}
+
+			if ( removeAttributes.size == 0 ) {
+				return;
+			}
+
+			model.change( writer => {
+				for ( const attributeKey of removeAttributes ) {
+					writer.removeSelectionAttribute( attributeKey );
+				}
+			} );
+		} );
 	}
 
 	/**

diff --git a/packages/ckeditor5-html-support/tests/datafilter.js b/packages/ckeditor5-html-support/tests/datafilter.js
@@ -4045,6 +4045,110 @@ describe( 'DataFilter', () => {
 				'</p>'
 			);
 		} );
+
+		it( 'should remove GHS selection attribute for the same range as a coupled feature attribute was removed', () => {
+			dataFilter.loadAllowedConfig( [ {
+				name: /^.*$/,
+				styles: true,
+				attributes: true,
+				classes: true
+			} ] );
+
+			editor.setData( '<p><a href="foo" class="bar">foobar</a></p>' );
+
+			expect( getModelDataWithAttributes( model ) ).to.deep.equal( {
+				data: '<paragraph><$text htmlA="(1)" linkHref="foo">[]foobar</$text></paragraph>',
+				attributes: {
+					1: {
+						classes: [ 'bar' ]
+					}
+				}
+			} );
+
+			expect( model.document.selection.getAttribute( 'linkHref' ) ).to.deep.equal( 'foo' );
+			expect( model.document.selection.getAttribute( 'htmlA' ) ).to.deep.equal( { classes: [ 'bar' ] } );
+
+			expect( editor.getData() ).to.equal( '<p><a class="bar" href="foo">foobar</a></p>' );
+
+			model.change( writer => {
+				writer.removeSelectionAttribute( 'linkHref' );
+			} );
+
+			expect( getModelDataWithAttributes( model ) ).to.deep.equal( {
+				data: '<paragraph>[]<$text htmlA="(1)" linkHref="foo">foobar</$text></paragraph>',
+				attributes: {
+					1: {
+						classes: [ 'bar' ]
+					}
+				}
+			} );
+
+			expect( model.document.selection.getAttribute( 'linkHref' ) ).to.be.undefined;
+			expect( model.document.selection.getAttribute( 'htmlA' ) ).to.be.undefined;
+
+			expect( editor.getData() ).to.equal( '<p><a class="bar" href="foo">foobar</a></p>' );
+		} );
+
+		it( 'should not remove other GHS selection attribute when other coupled one is removed', () => {
+			dataFilter.loadAllowedConfig( [ {
+				name: /^.*$/,
+				styles: true,
+				attributes: true,
+				classes: true
+			} ] );
+
+			editor.setData( '<p><span style="color:red;text-transform:uppercase;"><strong>foobar</strong></span></p>' );
+
+			expect( getModelDataWithAttributes( model ) ).to.deep.equal( {
+				data: '<paragraph><$text fontColor="red" htmlSpan="(1)" htmlStrong="(2)">[]foobar</$text></paragraph>',
+				attributes: {
+					1: {
+						styles: {
+							'text-transform': 'uppercase'
+						}
+					},
+					2: {}
+				}
+			} );
+
+			expect( model.document.selection.getAttribute( 'fontColor' ) ).to.deep.equal( 'red' );
+			expect( model.document.selection.getAttribute( 'htmlSpan' ) ).to.deep.equal( { styles: { 'text-transform': 'uppercase' } } );
+			expect( model.document.selection.getAttribute( 'htmlStrong' ) ).to.deep.equal( {} );
+
+			expect( editor.getData() ).to.equal(
+				'<p><span style="color:red;"><span style="text-transform:uppercase;"><strong>foobar</strong></span></span></p>'
+			);
+
+			model.change( writer => {
+				writer.removeSelectionAttribute( 'fontColor' );
+			} );
+
+			expect( getModelDataWithAttributes( model ) ).to.deep.equal( {
+				data:
+					'<paragraph>' +
+						'<$text htmlSpan="(1)" htmlStrong="(2)">[]</$text>' +
+						'<$text fontColor="red" htmlSpan="(3)" htmlStrong="(4)">foobar</$text>' +
+					'</paragraph>',
+				attributes: {
+					1: {
+						styles: {
+							'text-transform': 'uppercase'
+						}
+					},
+					2: {}
+				}
+			} );
+
+			expect( model.document.selection.getAttribute( 'fontColor' ) ).to.be.undefined;
+			expect( model.document.selection.getAttribute( 'htmlSpan' ) ).to.deep.equal( { styles: { 'text-transform': 'uppercase' } } );
+			expect( model.document.selection.getAttribute( 'htmlStrong' ) ).to.deep.equal( {} );
+
+			expect( editor.getData() ).to.equal(
+				'<p>' +
+					'<span style="color:red;"><span style="text-transform:uppercase;"><strong>foobar</strong></span></span>' +
+				'</p>'
+			);
+		} );
 	} );
 
 	describe( 'loadAllowedEmptyElementsConfig', () => {

diff --git a/packages/ckeditor5-html-support/tests/manual/ghs-link.html b/packages/ckeditor5-html-support/tests/manual/ghs-link.html
@@ -0,0 +1,18 @@
+<head>
+	<meta http-equiv="Content-Security-Policy" content="script-src 'self' https://ckeditor.com 'unsafe-inline' 'unsafe-eval'">
+	<style>
+		.link {
+			display: inline-block;
+			padding: 1em;
+			outline: 1px solid rgba(255, 0, 0, 0.3);
+		}
+	</style>
+</head>
+
+<div id="editor">
+	<p>foo <a href="#bar" class="link">bar</a></p>
+	<p>foo <a href="#bar" class="link">bar</a> baz</p>
+	<p><a href="#bar" class="link">bar</a> baz</p>
+	<p><a href="#bar" class="link">bar</a></p>
+	<p><a href="#foo" class="link">foo</a><a href="#bar" class="link">bar</a></p>
+</div>
diff --git a/packages/ckeditor5-html-support/tests/manual/ghs-link.js b/packages/ckeditor5-html-support/tests/manual/ghs-link.js
@@ -0,0 +1,44 @@
+/**
+ * @license Copyright (c) 2003-2023, CKSource Holding sp. z o.o. All rights reserved.
+ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
+ */
+
+/* globals console:false, window, document */
+
+import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor';
+import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset';
+import SourceEditing from '@ckeditor/ckeditor5-source-editing/src/sourceediting';
+
+import GeneralHtmlSupport from '../../src/generalhtmlsupport';
+
+ClassicEditor
+	.create( document.querySelector( '#editor' ), {
+		plugins: [
+			ArticlePluginSet, SourceEditing, GeneralHtmlSupport
+		],
+		toolbar: [
+			'sourceEditing',
+			'|',
+			'heading',
+			'|',
+			'bold', 'italic', 'link',
+			'|',
+			'undo', 'redo'
+		],
+		htmlSupport: {
+			allow: [
+				{
+					name: /^.*$/,
+					styles: true,
+					attributes: true,
+					classes: true
+				}
+			]
+		}
+	} )
+	.then( editor => {
+		window.editor = editor;
+	} )
+	.catch( err => {
+		console.error( err.stack );
+	} );
diff --git a/packages/ckeditor5-html-support/tests/manual/ghs-link.md b/packages/ckeditor5-html-support/tests/manual/ghs-link.md
@@ -0,0 +1 @@
+## GHS Link handling