Merge pull request #17725 from ckeditor/improve-performance-in-placeh…

…older-2

Other (engine): Improve performance of the placeholders.

Other (image): Attribute `loading="lazy"` will be automatically added in editing view to images with `height` and `width` attributes set to improve loading performance.

Tests: Added new `ghs` manual performance test.

MINOR BREAKING CHANGE (image): Starting from this release, images that have `height` and `width` attributes set will automatically receive `loading="lazy"` attribute in the editing area. This happens only for the content loaded into the editor, the data output produced by the editor remains the same. The reason for this change is to improve user experience in documents that may contain hundreds of images.

packages/ckeditor5-ckbox/tests/ckboximageedit/ckboximageeditcommand.js

@@ -951,7 +951,8 @@ describe( 'CKBoxImageEditCommand', () => {
 				'while waiting for the processed image', async () => {
 				expect( getViewData( editor.editing.view, { withoutSelection: true } ) ).to.equal(
 					'<figure class="ck-widget ck-widget_selected image" contenteditable="false" data-ckbox-resource-id="example-id">' +
-						'<img alt="alt text" height="50" src="/assets/sample.png" style="aspect-ratio:50/50" width="50"></img>' +
+						'<img alt="alt text" height="50" loading="lazy" src="/assets/sample.png" style="aspect-ratio:50/50" width="50">' +
+						'</img>' +
 						'<div class="ck ck-reset_all ck-widget__type-around"></div>' +
 					'</figure>'
 				);
@@ -961,7 +962,9 @@ describe( 'CKBoxImageEditCommand', () => {
 				expect( getViewData( editor.editing.view, { withoutSelection: true } ) ).to.equal(
 					'<figure class="ck-widget ck-widget_selected image image-processing" ' +
 						'contenteditable="false" data-ckbox-resource-id="example-id">' +
-						'<img alt="alt text" height="100" src="/assets/sample.png" style="height:100px;width:100px" width="100"></img>' +
+						'<img alt="alt text" height="100" loading="lazy" src="/assets/sample.png" ' +
+							'style="height:100px;width:100px" width="100">' +
+						'</img>' +
 						'<div class="ck ck-reset_all ck-widget__type-around"></div>' +
 					'</figure>'
 				);

packages/ckeditor5-engine/src/view/placeholder.ts

@@ -50,24 +50,22 @@ export function enablePlaceholder( { view, element, text, isDirectHost = true, k
 } ): void {
 	const doc = view.document;
 
-	// Use a single a single post fixer per—document to update all placeholders.
+	// Use a single post fixer per—document to update all placeholders.
 	if ( !documentPlaceholders.has( doc ) ) {
 		documentPlaceholders.set( doc, new Map() );
 
 		// If a post-fixer callback makes a change, it should return `true` so other post–fixers
 		// can re–evaluate the document again.
-		doc.registerPostFixer( writer => updateDocumentPlaceholders( doc, writer ) );
+		doc.registerPostFixer( writer => updateDocumentPlaceholders( documentPlaceholders.get( doc )!, writer ) );
 
 		// Update placeholders on isComposing state change since rendering is disabled while in composition mode.
 		doc.on<ObservableChangeEvent>( 'change:isComposing', () => {
-			view.change( writer => updateDocumentPlaceholders( doc, writer ) );
+			view.change( writer => updateDocumentPlaceholders( documentPlaceholders.get( doc )!, writer ) );
 		}, { priority: 'high' } );
 	}
 
 	if ( element.is( 'editableElement' ) ) {
-		element.on( 'change:placeholder', ( evtInfo, evt, text ) => {
-			setPlaceholder( text );
-		} );
+		element.on( 'change:placeholder', ( evtInfo, evt, text ) => setPlaceholder( text ) );
 	}
 
 	if ( element.placeholder ) {
@@ -81,16 +79,18 @@ export function enablePlaceholder( { view, element, text, isDirectHost = true, k
 	}
 
 	function setPlaceholder( text: string ) {
-		// Store information about the element placeholder under its document.
-		documentPlaceholders.get( doc )!.set( element, {
+		const config = {
 			text,
 			isDirectHost,
 			keepOnFocus,
 			hostElement: isDirectHost ? element : null
-		} );
+		};
+
+		// Store information about the element placeholder under its document.
+		documentPlaceholders.get( doc )!.set( element, config );
 
 		// Update the placeholders right away.
-		view.change( writer => updateDocumentPlaceholders( doc, writer ) );
+		view.change( writer => updateDocumentPlaceholders( [ [ element, config ] ], writer ) );
 	}
 }
 
@@ -182,11 +182,7 @@ export function needsPlaceholder( element: Element, keepOnFocus: boolean ): bool
 		return false;
 	}
 
-	// Anything but uiElement(s) counts as content.
-	const hasContent = Array.from( element.getChildren() )
-		.some( element => !element.is( 'uiElement' ) );
-
-	if ( hasContent ) {
+	if ( hasContent( element ) ) {
 		return false;
 	}
 
@@ -212,13 +208,28 @@ export function needsPlaceholder( element: Element, keepOnFocus: boolean ): bool
 	return !!selectionAnchor && selectionAnchor.parent !== element;
 }
 
+/**
+ * Anything but uiElement(s) counts as content.
+ */
+function hasContent( element: Element ): boolean {
+	for ( const child of element.getChildren() ) {
+		if ( !child.is( 'uiElement' ) ) {
+			return true;
+		}
+	}
+
+	return false;
+}
+
 /**
  * Updates all placeholders associated with a document in a post–fixer callback.
  *
  * @returns True if any changes were made to the view document.
  */
-function updateDocumentPlaceholders( doc: Document, writer: DowncastWriter ): boolean {
-	const placeholders = documentPlaceholders.get( doc )!;
+function updateDocumentPlaceholders(
+	placeholders: Iterable<[ Element, PlaceholderConfig ]>,
+	writer: DowncastWriter
+): boolean {
 	const directHostElements: Array<Element> = [];
 	let wasViewModified = false;
 

packages/ckeditor5-engine/tests/view/placeholder.js

@@ -288,6 +288,8 @@ describe( 'placeholder', () => {
 				isDirectHost: true
 			} );
 
+			view.forceRender();
+
 			expect( viewRoot.getChild( 0 ).getAttribute( 'data-placeholder' ) ).to.equal( 'bar' );
 			expect( viewRoot.getChild( 0 ).isEmpty ).to.be.true;
 			expect( viewRoot.getChild( 0 ).hasClass( 'ck-placeholder' ) ).to.be.true;

packages/ckeditor5-image/src/imagesizeattributes.ts

@@ -121,8 +121,8 @@ export default class ImageSizeAttributes extends Plugin {
 
 		// Dedicated converters to propagate attributes to the <img> element.
 		editor.conversion.for( 'editingDowncast' ).add( dispatcher => {
-			attachDowncastConverter( dispatcher, 'width', 'width', true );
-			attachDowncastConverter( dispatcher, 'height', 'height', true );
+			attachDowncastConverter( dispatcher, 'width', 'width', true, true );
+			attachDowncastConverter( dispatcher, 'height', 'height', true, true );
 		} );
 
 		editor.conversion.for( 'dataDowncast' ).add( dispatcher => {
@@ -134,7 +134,8 @@ export default class ImageSizeAttributes extends Plugin {
 			dispatcher: DowncastDispatcher,
 			modelAttributeName: string,
 			viewAttributeName: string,
-			setRatioForInlineImage: boolean
+			setRatioForInlineImage: boolean,
+			isEditingDowncast: boolean = false
 		) {
 			dispatcher.on<DowncastAttributeEvent>( `attribute:${ modelAttributeName }:${ imageType }`, ( evt, data, conversionApi ) => {
 				if ( !conversionApi.consumable.consume( data.item, evt.name ) ) {
@@ -166,8 +167,14 @@ export default class ImageSizeAttributes extends Plugin {
 				const width = data.item.getAttribute( 'width' );
 				const height = data.item.getAttribute( 'height' );
 
-				if ( width && height ) {
-					viewWriter.setStyle( 'aspect-ratio', `${ width }/${ height }`, img );
+				if ( !width || !height ) {
+					return;
+				}
+
+				viewWriter.setStyle( 'aspect-ratio', `${ width }/${ height }`, img );
+
+				if ( isEditingDowncast ) {
+					viewWriter.setAttribute( 'loading', 'lazy', img );
 				}
 			} );
 		}

packages/ckeditor5-image/tests/imagesizeattributes.js

@@ -290,7 +290,8 @@ describe( 'ImageSizeAttributes', () => {
 
 						expect( getViewData( view, { withoutSelection: true } ) ).to.equal(
 							'<p><span class="ck-widget image-inline image_resized" contenteditable="false" style="width:50px">' +
-								'<img height="200" src="/assets/sample.png" style="aspect-ratio:100/200" width="100"></img>' +
+								'<img height="200" loading="lazy" src="/assets/sample.png" style="aspect-ratio:100/200" width="100">' +
+								'</img>' +
 							'</span></p>'
 						);
 
@@ -307,7 +308,8 @@ describe( 'ImageSizeAttributes', () => {
 
 						expect( getViewData( view, { withoutSelection: true } ) ).to.equal(
 							'<p><span class="ck-widget image-inline" contenteditable="false">' +
-								'<img height="200" src="/assets/sample.png" style="aspect-ratio:100/200" width="100"></img>' +
+								'<img height="200" loading="lazy" src="/assets/sample.png" style="aspect-ratio:100/200" width="100">' +
+								'</img>' +
 							'</span></p>'
 						);
 
@@ -489,7 +491,8 @@ describe( 'ImageSizeAttributes', () => {
 
 						expect( getViewData( view, { withoutSelection: true } ) ).to.equal(
 							'<figure class="ck-widget image image_resized" contenteditable="false" style="width:50px">' +
-								'<img height="200" src="/assets/sample.png" style="aspect-ratio:100/200" width="100"></img>' +
+								'<img height="200" loading="lazy" src="/assets/sample.png" style="aspect-ratio:100/200" width="100">' +
+								'</img>' +
 							'</figure>'
 						);
 
@@ -507,7 +510,8 @@ describe( 'ImageSizeAttributes', () => {
 
 						expect( getViewData( view, { withoutSelection: true } ) ).to.equal(
 							'<figure class="ck-widget image" contenteditable="false">' +
-								'<img height="200" src="/assets/sample.png" style="aspect-ratio:100/200" width="100"></img>' +
+								'<img height="200" loading="lazy" src="/assets/sample.png" style="aspect-ratio:100/200" width="100">' +
+								'</img>' +
 							'</figure>'
 						);
 

tests/_data/data-sets/ghs.js

@@ -0,0 +1,83 @@
+/**
+ * @license Copyright (c) 2003-2024, CKSource Holding sp. z o.o. All rights reserved.
+ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-licensing-options
+ */
+
+// This is main Wikipedia page source copied four times. This is to test content with a lot of messy / unsupported markup.
+// After it is loaded in the editor, it is ~150 A4 pages (however there are a lot of very short paragraphs and list items).
+
+/* eslint-disable */
+
+const initialData =
+	`<p style="color:blue;">Feature paragraph</p>
+
+	<h2 style="color:green">Feature heading1</h2>
+	<h3 style="color:green">Feature heading2</h3>
+	<h4 style="color:green">Feature heading3</h4>
+
+	<p><strong style="color:blue;">Feature bold</strong></p>
+	<p><i style="color:blue;">Feature italic</i></p>
+	<p><s style="color:blue;">Feature strike</s></p>
+	<p><u style="color:blue;">Feature underline</u></p>
+	<p><code style="color:blue;">Feature code</code></p>
+	<p><sub style="color:blue;">Feature subscript</sub></p>
+	<p><sup style="color:blue;">Feature superscript</sup></p>
+
+	<p><a style="color:green;" href="https://example.com">Link feature</a></p>
+	<p><a name="anchor" id="anchor">Anchor (name, ID only)</a></p>
+
+	<p><mark class="marker-yellow" data-mark>Mark feature</mark></p>
+
+	<p><span class="text-big text-italic" style="background-color:hsl(60, 75%, 60%);color:hsl(240, 75%, 60%);font-family:'Courier New', Courier, monospace;border:1px solid black">Font feature</span></p>
+
+	<ul style="background:blue;">
+		<li style="color:yellow;">Bulleted List feature</li>
+		<li style="color:yellow;">Bulleted List feature</li>
+		<li style="color:yellow;">Bulleted List feature</li>
+	</ul>
+
+	<ol style="background:blue;">
+		<li style="color:yellow;">Numbered List feature</li>
+		<li style="color:yellow;">Numbered List feature</li>
+		<li style="color:yellow;">Numbered List feature</li>
+	</ol>
+
+	<ul class="todo-list" style="background:blue;">
+		<li style="color:yellow;">
+			<label class="todo-list__label">
+				<input type="checkbox" disabled="disabled">
+				<span class="todo-list__label__description">Todo List feature</span>
+			</label>
+		</li>
+		<li style="color:yellow;">
+			<label class="todo-list__label">
+				<input type="checkbox" disabled="disabled">
+				<span class="todo-list__label__description">Todo List feature</span>
+			</label>
+		</li>
+		<li style="color:yellow;">
+			<label class="todo-list__label">
+				<input type="checkbox" disabled="disabled">
+				<span class="todo-list__label__description">Todo List feature</span>
+			</label>
+		</li>
+	</ul>
+
+	<blockquote style="color:blue;"><p>Blockquote Feature</p></blockquote>
+
+	<figure style="border: 1px solid blue;" class="image">
+		<img src="/ckeditor5/tests/manual/sample.jpg" width="826" height="388"/>
+		<figcaption style="background:yellow;">Caption</figcaption>
+	</figure>
+
+	<pre style="background:blue;"><code style="color:yellow;" class="language-plaintext">Code Block</code></pre>
+
+	<div style="border: 1px solid blue" class="raw-html-embed">HTML snippet</div>
+	<div data-foo class="page-break" style="page-break-after:always;"><span style="display:none;">&nbsp;</span></div>
+	<p><i class="inline-icon"></i> empty inline at start</p>
+	<p>Text with <i class="inline-icon"></i> empty inline inside</p>
+	<p>Text with empty inline at the end <i class="inline-icon"></i></p>`;
+
+export default function makeData() {
+  return initialData.repeat( 250 );
+}

tests/_data/data-sets/index.js

@@ -3,14 +3,22 @@
  * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-licensing-options
  */
 
-import paragraphs from './paragraphs.js';
-import lists from './lists.js';
-import tableHuge from './table-huge.js';
 import formattingLongP from './formatting-long-paragraphs.js';
+import ghs from './ghs.js';
 import inlineStyles from './inline-styles.js';
+import lists from './lists.js';
 import mixed from './mixed.js';
+import paragraphs from './paragraphs.js';
+import tableHuge from './table-huge.js';
 import wiki from './wiki.js';
 
 export default {
-	paragraphs, lists, tableHuge, formattingLongP, inlineStyles, mixed, wiki
+	formattingLongP,
+	ghs,
+	inlineStyles,
+	lists,
+	mixed,
+	paragraphs,
+	tableHuge,
+	wiki
 };