Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

The link rel attribute should be handled as a token list to allow mixing manual link decorators for the same attribute #17461

Draft
wants to merge 17 commits into
base: master
Choose a base branch
from
Draft

The link rel attribute should be handled as a token list to allow mixing manual link decorators for the same attribute #17461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
cdf9b8d
Refactoring of view styles and classes to more generic solution.
niegowski Nov 4, 2024
5e6a334
Fixed unwrapping.
niegowski Nov 5, 2024
5310c3f
Updated tests.
niegowski Nov 5, 2024
8cd520d
Added fast-access private properties.
niegowski Nov 5, 2024
60c7c58
Fixed tests.
niegowski Nov 5, 2024
3daf3b3
Merge branch 'refs/heads/master' into ck/13985
niegowski Nov 6, 2024
1b142c1
Using getter for easier usage and marginal performance difference.
niegowski Nov 6, 2024
6437f0e
Bring back original method signature.
niegowski Nov 6, 2024
9685665
Merge branch 'refs/heads/master' into ck/13985
niegowski Nov 6, 2024
a7ac306
Unified setAttribute for both tokenized, styles map and plain string …
niegowski Nov 8, 2024
cc84584
Reduced usage of addClass, removeClass, addStyle, removeStyle in Down…
niegowski Nov 8, 2024
f2b5543
Wrap/unwrap AttributeElement delegated to the view Elements.
niegowski Nov 13, 2024
4665f8f
Added tests.
niegowski Nov 13, 2024
7d04816
Added hasAttribute with expected value handling.
niegowski Nov 14, 2024
ece3d94
Fixed mention upcast conversion (all attributes must be consumed).
niegowski Nov 14, 2024
72c6449
Removed not used property from type declaration.
niegowski Nov 14, 2024
6ffcbb7
Part of the Matcher internal logic split into the view Element.
niegowski Nov 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 19 additions & 44 deletions packages/ckeditor5-engine/src/conversion/downcasthelpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1664,60 +1664,35 @@ function changeAttribute( attributeCreator: AttributeCreatorFunction ) {

// First remove the old attribute if there was one.
if ( data.attributeOldValue !== null && oldAttribute ) {
if ( oldAttribute.key == 'class' ) {
const classes = typeof oldAttribute.value == 'string' ? oldAttribute.value.split( /\s+/ ) : oldAttribute.value;
let value = oldAttribute.value;

for ( const className of classes ) {
viewWriter.removeClass( className, viewElement );
}
} else if ( oldAttribute.key == 'style' ) {
if ( oldAttribute.key == 'style' ) {
if ( typeof oldAttribute.value == 'string' ) {
const styles = new StylesMap( viewWriter.document.stylesProcessor );

styles.setTo( oldAttribute.value );

for ( const [ key ] of styles.getStylesEntries() ) {
viewWriter.removeStyle( key, viewElement );
}
value = new StylesMap( viewWriter.document.stylesProcessor )
.setTo( oldAttribute.value )
.getStylesEntries()
.map( ( [ key ] ) => key );
} else {
const keys = Object.keys( oldAttribute.value );

for ( const key of keys ) {
viewWriter.removeStyle( key, viewElement );
}
value = Object.keys( oldAttribute.value );
}
} else {
viewWriter.removeAttribute( oldAttribute.key, viewElement );
}

viewWriter.removeAttribute( oldAttribute.key, value as any, viewElement ); // TODO any
}

// Then set the new attribute.
if ( data.attributeNewValue !== null && newAttribute ) {
if ( newAttribute.key == 'class' ) {
const classes = typeof newAttribute.value == 'string' ? newAttribute.value.split( /\s+/ ) : newAttribute.value;

for ( const className of classes ) {
viewWriter.addClass( className, viewElement );
}
} else if ( newAttribute.key == 'style' ) {
if ( typeof newAttribute.value == 'string' ) {
const styles = new StylesMap( viewWriter.document.stylesProcessor );

styles.setTo( newAttribute.value );

for ( const [ key, value ] of styles.getStylesEntries() ) {
viewWriter.setStyle( key, value, viewElement );
}
} else {
const keys = Object.keys( newAttribute.value );

for ( const key of keys ) {
viewWriter.setStyle( key, ( newAttribute.value as Record<string, string> )[ key ], viewElement );
}
}
} else {
viewWriter.setAttribute( newAttribute.key, newAttribute.value as string, viewElement );
let value = newAttribute.value;

if ( newAttribute.key == 'style' && typeof newAttribute.value == 'string' ) {
value = Object.fromEntries(
new StylesMap( viewWriter.document.stylesProcessor )
.setTo( newAttribute.value )
.getStylesEntries()
);
}

viewWriter.setAttribute( newAttribute.key, value, false, viewElement );
}
};
}
Expand Down
24 changes: 24 additions & 0 deletions packages/ckeditor5-engine/src/view/attributeelement.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,30 @@ export default class AttributeElement extends Element {
return super.isSimilar( otherElement ) && this.priority == ( otherElement as any ).priority;
}

/**
* TODO
*/
protected override _canMergeAttributesFrom( otherElement: AttributeElement ): boolean {
// Can't merge if any of elements have an id or a difference of priority.
if ( this.id !== null || otherElement.id !== null || this.priority !== otherElement.priority ) {
return false;
}

return super._canMergeAttributesFrom( otherElement );
}

/**
* TODO
*/
protected override _hasAttributesMatching( otherElement: AttributeElement ): boolean {
// TODO Can't merge if any of elements have an id or a difference of priority.
if ( this.id !== null || otherElement.id !== null || this.priority !== otherElement.priority ) {
return false;
}

return super._hasAttributesMatching( otherElement );
}

/**
* Clones provided element with priority.
*
Expand Down
196 changes: 49 additions & 147 deletions packages/ckeditor5-engine/src/view/downcastwriter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import AttributeElement from './attributeelement.js';
import EmptyElement from './emptyelement.js';
import UIElement from './uielement.js';
import RawElement from './rawelement.js';
import { CKEditorError, isIterable } from '@ckeditor/ckeditor5-utils';
import { CKEditorError, isIterable, type ArrayOrItem } from '@ckeditor/ckeditor5-utils';
import DocumentFragment from './documentfragment.js';
import Text from './text.js';
import EditableElement from './editableelement.js';
Expand All @@ -31,6 +31,7 @@ import type { default as Element, ElementAttributes } from './element.js';
import type DomConverter from './domconverter.js';
import type Item from './item.js';
import type { SlotFilter } from '../conversion/downcasthelpers.js';
import type { Styles, StyleValue } from './stylesmap.js';

type DomDocument = globalThis.Document;
type DomElement = globalThis.HTMLElement;
Expand Down Expand Up @@ -523,21 +524,61 @@ export default class DowncastWriter {
* @param key The attribute key.
* @param value The attribute value.
*/
public setAttribute( key: string, value: unknown, element: Element ): void {
element._setAttribute( key, value );
public setAttribute( key: string, value: unknown, element: Element ): void;

/**
* Adds or overwrites the element's attribute with a specified key and value. TODO
*
* ```ts
* writer.setAttribute( 'href', 'http://ckeditor.com', linkElement );
* ```
*
* @param key The attribute key.
* @param value The attribute value.
*/
public setAttribute( key: string, value: unknown | Styles | [ string, StyleValue ], reset: boolean, element: Element ): void;

public setAttribute(
key: string,
value: unknown | Styles | [ string, StyleValue ],
elementOrReset: Element | boolean,
element?: Element
): void {
if ( element !== undefined ) {
element._setAttribute( key, value, elementOrReset as boolean );
} else {
( elementOrReset as Element )._setAttribute( key, value );
}
}

/**
* Removes attribute from the element.
* Removes attribute from the element. TODO
*
* ```ts
* writer.removeAttribute( 'href', linkElement );
* ```
*
* @param key Attribute key.
*/
public removeAttribute( key: string, element: Element ): void {
element._removeAttribute( key );
public removeAttribute( key: string, element: Element ): void;

/**
* Removes attribute from the element. TODO
*
* ```ts
* writer.removeAttribute( 'class', 'foo', linkElement );
* ```
*
* @param key Attribute key.
*/
public removeAttribute( key: string, tokens: ArrayOrItem<string>, element: Element ): void;

public removeAttribute( key: string, elementOrTokens: Element | ArrayOrItem<string>, element?: Element ): void {
if ( element !== undefined ) {
element._removeAttribute( key, elementOrTokens as ArrayOrItem<string> );
} else {
( elementOrTokens as Element )._removeAttribute( key );
}
}

/**
Expand Down Expand Up @@ -1524,7 +1565,7 @@ export default class DowncastWriter {
//
// <p><span class="bar">abc</span></p> --> <p><span class="foo bar">abc</span></p>
//
if ( isAttribute && this._wrapAttributeElement( wrapElement, child ) ) {
if ( isAttribute && ( child as AttributeElement )._mergeAttributesFrom( wrapElement ) ) {
wrapPositions.push( new Position( parent, i ) );
}
//
Expand Down Expand Up @@ -1639,7 +1680,7 @@ export default class DowncastWriter {
// <p><span class="foo bar">abc</span>xyz</p> --> <p><span class="bar">abc</span>xyz</p>
// <p><i class="foo">abc</i>xyz</p> --> <p><i class="foo">abc</i>xyz</p>
//
if ( this._unwrapAttributeElement( unwrapElement, child ) ) {
if ( child._subtractAttributesOf( unwrapElement ) ) {
unwrapPositions.push(
new Position( parent, i ),
new Position( parent, i + 1 )
Expand Down Expand Up @@ -1762,137 +1803,6 @@ export default class DowncastWriter {
return movePositionToTextNode( newPosition );
}

/**
* Wraps one {@link module:engine/view/attributeelement~AttributeElement AttributeElement} into another by
* merging them if possible. When merging is possible - all attributes, styles and classes are moved from wrapper
* element to element being wrapped.
*
* @param wrapper Wrapper AttributeElement.
* @param toWrap AttributeElement to wrap using wrapper element.
* @returns Returns `true` if elements are merged.
*/
private _wrapAttributeElement( wrapper: AttributeElement, toWrap: AttributeElement ): boolean {
if ( !canBeJoined( wrapper, toWrap ) ) {
return false;
}

// Can't merge if name or priority differs.
if ( wrapper.name !== toWrap.name || wrapper.priority !== toWrap.priority ) {
return false;
}

// Check if attributes can be merged.
for ( const key of wrapper.getAttributeKeys() ) {
// Classes and styles should be checked separately.
if ( key === 'class' || key === 'style' ) {
continue;
}

// If some attributes are different we cannot wrap.
if ( toWrap.hasAttribute( key ) && toWrap.getAttribute( key ) !== wrapper.getAttribute( key ) ) {
return false;
}
}

// Check if styles can be merged.
for ( const key of wrapper.getStyleNames() ) {
if ( toWrap.hasStyle( key ) && toWrap.getStyle( key ) !== wrapper.getStyle( key ) ) {
return false;
}
}

// Move all attributes/classes/styles from wrapper to wrapped AttributeElement.
for ( const key of wrapper.getAttributeKeys() ) {
// Classes and styles should be checked separately.
if ( key === 'class' || key === 'style' ) {
continue;
}

// Move only these attributes that are not present - other are similar.
if ( !toWrap.hasAttribute( key ) ) {
this.setAttribute( key, wrapper.getAttribute( key )!, toWrap );
}
}

for ( const key of wrapper.getStyleNames() ) {
if ( !toWrap.hasStyle( key ) ) {
this.setStyle( key, wrapper.getStyle( key )!, toWrap );
}
}

for ( const key of wrapper.getClassNames() ) {
if ( !toWrap.hasClass( key ) ) {
this.addClass( key, toWrap );
}
}

return true;
}

/**
* Unwraps {@link module:engine/view/attributeelement~AttributeElement AttributeElement} from another by removing
* corresponding attributes, classes and styles. All attributes, classes and styles from wrapper should be present
* inside element being unwrapped.
*
* @param wrapper Wrapper AttributeElement.
* @param toUnwrap AttributeElement to unwrap using wrapper element.
* @returns Returns `true` if elements are unwrapped.
**/
private _unwrapAttributeElement( wrapper: AttributeElement, toUnwrap: AttributeElement ): boolean {
if ( !canBeJoined( wrapper, toUnwrap ) ) {
return false;
}

// Can't unwrap if name or priority differs.
if ( wrapper.name !== toUnwrap.name || wrapper.priority !== toUnwrap.priority ) {
return false;
}

// Check if AttributeElement has all wrapper attributes.
for ( const key of wrapper.getAttributeKeys() ) {
// Classes and styles should be checked separately.
if ( key === 'class' || key === 'style' ) {
continue;
}

// If some attributes are missing or different we cannot unwrap.
if ( !toUnwrap.hasAttribute( key ) || toUnwrap.getAttribute( key ) !== wrapper.getAttribute( key ) ) {
return false;
}
}

// Check if AttributeElement has all wrapper classes.
if ( !toUnwrap.hasClass( ...wrapper.getClassNames() ) ) {
return false;
}

// Check if AttributeElement has all wrapper styles.
for ( const key of wrapper.getStyleNames() ) {
// If some styles are missing or different we cannot unwrap.
if ( !toUnwrap.hasStyle( key ) || toUnwrap.getStyle( key ) !== wrapper.getStyle( key ) ) {
return false;
}
}

// Remove all wrapper's attributes from unwrapped element.
for ( const key of wrapper.getAttributeKeys() ) {
// Classes and styles should be checked separately.
if ( key === 'class' || key === 'style' ) {
continue;
}

this.removeAttribute( key, toUnwrap );
}

// Remove all wrapper's classes from unwrapped element.
this.removeClass( Array.from( wrapper.getClassNames() ), toUnwrap );

// Remove all wrapper's styles from unwrapped element.
this.removeStyle( Array.from( wrapper.getStyleNames() ), toUnwrap );

return true;
}

/**
* Helper function used by other `DowncastWriter` methods. Breaks attribute elements at the boundaries of given range.
*
Expand Down Expand Up @@ -2331,11 +2241,3 @@ function validateRangeContainer( range: Range, errorContext: Document ) {
throw new CKEditorError( 'view-writer-invalid-range-container', errorContext );
}
}

/**
* Checks if two attribute elements can be joined together. Elements can be joined together if, and only if
* they do not have ids specified.
*/
function canBeJoined( a: AttributeElement, b: AttributeElement ) {
return a.id === null && b.id === null;
}
Loading