From 3c28b677691a8d84345086f2c7060a6b44dfcb21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Thu, 13 May 2021 15:21:51 +0200 Subject: [PATCH 01/28] Improve engine view matcher with new pattern syntax. --- packages/ckeditor5-engine/src/view/matcher.js | 216 ++++++++++++------ .../ckeditor5-engine/tests/view/matcher.js | 99 ++++++++ 2 files changed, 248 insertions(+), 67 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/matcher.js b/packages/ckeditor5-engine/src/view/matcher.js index 64fd31aed8c..3aaee746b4a 100644 --- a/packages/ckeditor5-engine/src/view/matcher.js +++ b/packages/ckeditor5-engine/src/view/matcher.js @@ -7,6 +7,8 @@ * @module engine/view/matcher */ +import { isPlainObject } from 'lodash-es'; + /** * View matcher class. * Instance of this class can be used to find {@link module:engine/view/element~Element elements} that match given pattern. @@ -242,35 +244,105 @@ function matchName( pattern, name ) { return pattern === name; } -// Checks if attributes of provided element can be matched against provided patterns. +// Checks if an array of key/value pairs can be matched against provided patterns. +// +// Patterns can be provided in a following ways: +// - a boolean value matches any attribute with any value (or no value), +// - a RegExp expression or object matches any attribute name, +// - an object matches any attribute that has the same name as the object item's key, where object item's value is: +// - equal to `true`, which matches any attribute value, +// - a string that is equal to attribute value, +// - a regular expression that matches attribute value, +// - an array with items, where the item is: +// - a string that is equal to attribute value, +// - an object with `key` and `value` property, where `key` is a regular expression matching attribute name and +// `value` is either regular expression matching attribute value or a string equal to attribute value. +// +// pattern: true, +// +// // or +// +// pattern: /h[1-6]/ +// +// // or +// +// pattern: { +// required: true, +// src: /https.*/, +// rel: 'nofollow' +// } +// +// // or +// +// pattern: [ 'data-property-1', 'data-property-2' ], +// +// // or +// +// pattern: [ 'data-test', 'data-foo' ], +// +// // or +// +// pattern: [ +// { key: /data-property-.*/, value: 'foobar' }, +// // or +// { key: /data-property-.*/, value: /foo.*/ } +// ] +// } +// +// @param {Object} patterns Object with information about attributes to match. +// @param {Array} attributes An array of key/value pairs, e.g.: +// +// [ +// [ 'src', 'https://example.com' ], +// [ 'rel', 'nofollow' ] +// ] // -// @param {Object} patterns Object with information about attributes to match. Each key of the object will be -// used as attribute name. Value of each key can be a string or regular expression to match against attribute value. -// @param {module:engine/view/element~Element} element Element which attributes will be tested. // @returns {Array|null} Returns array with matched attribute names or `null` if no attributes were matched. -function matchAttributes( patterns, element ) { +function matchPatterns( patterns, attributes ) { + const attributeKeys = attributes.map( ( [ key ] ) => key ); const match = []; - for ( const name in patterns ) { - const pattern = patterns[ name ]; + if ( patterns === true ) { + if ( attributeKeys.length ) { + return attributeKeys; + } else { + return null; + } + } else if ( patterns instanceof RegExp ) { + attributeKeys.forEach( attribute => { + if ( patterns.test( attribute ) ) { + match.push( attribute ); + } + } ); - if ( element.hasAttribute( name ) ) { - const attribute = element.getAttribute( name ); + if ( match.length ) { + return match; + } else { + return null; + } + } - if ( pattern === true ) { - match.push( name ); - } else if ( pattern instanceof RegExp ) { - if ( pattern.test( attribute ) ) { - match.push( name ); - } else { - return null; + patterns = normalizePatterns( patterns ); + + for ( const pattern in patterns ) { + const { key: patternKey, value: patternVal } = patterns[ pattern ]; + + attributes.forEach( ( [ attributeKey, attributeValue ] ) => { + if ( + ( patternKey instanceof RegExp && patternKey.test( attributeKey ) ) || + patternKey === attributeKey + ) { + if ( patternVal === true ) { + match.push( attributeKey ); + } else if ( patternVal === attributeValue ) { + match.push( attributeKey ); + } else if ( patternVal instanceof RegExp && patternVal.test( attributeValue ) ) { + match.push( attributeKey ); } - } else if ( attribute === pattern ) { - match.push( name ); - } else { - return null; } - } else { + } ); + + if ( !match.length ) { return null; } } @@ -278,35 +350,66 @@ function matchAttributes( patterns, element ) { return match; } +// Bring all the possible pattern forms to an array of objects with `key` and `value` property. +// For example: +// +// { +// src: /https:.*/ +// } +// +// to: +// +// { +// key: 'src', +// value: /https:.*/ +// } +// @param {Object|Array} patterns +// @returns {Array|null} Returns an array of objects or null if patterns were not in a form of expected input. +function normalizePatterns( patterns ) { + if ( Array.isArray( patterns ) ) { + return patterns.map( + pattern => { + // eslint-disable-next-line dot-notation + if ( isPlainObject( pattern ) && pattern[ 'key' ] && pattern[ 'value' ] ) { + return pattern; + } + + // Assume the pattern is either String or RegExp. + return { key: pattern, value: true }; + } + ); + } else if ( isPlainObject( patterns ) ) { + return Object.entries( patterns ).map( + ( [ patternKey, patternValue ] ) => ( { key: patternKey, value: patternValue } ) + ); + } else { + return null; + } +} + +// Checks if attributes of provided element can be matched against provided patterns. +// +// @param {Object} patterns Object with information about attributes to match. Each key of the object will be +// used as attribute name. Value of each key can be a string or regular expression to match against attribute value. +// @param {module:engine/view/element~Element} element Element which attributes will be tested. +// @returns {Array|null} Returns array with matched attribute names or `null` if no attributes were matched. +function matchAttributes( patterns, element ) { + // TODO: The `getAttributes()` returns `class` and `style` attribute if present. Should we exclude them when querying attributes? + const attributes = Array.from( element.getAttributes() ); + + return matchPatterns( patterns, attributes ); +} + // Checks if classes of provided element can be matched against provided patterns. // // @param {Array.} patterns Array of strings or regular expressions to match against element's classes. // @param {module:engine/view/element~Element} element Element which classes will be tested. // @returns {Array|null} Returns array with matched class names or `null` if no classes were matched. function matchClasses( patterns, element ) { - const match = []; - - for ( const pattern of patterns ) { - if ( pattern instanceof RegExp ) { - const classes = element.getClassNames(); - - for ( const name of classes ) { - if ( pattern.test( name ) ) { - match.push( name ); - } - } + const classNames = Array.from( element.getClassNames() ); + const attributes = classNames.map( className => [ className ] ); - if ( match.length === 0 ) { - return null; - } - } else if ( element.hasClass( pattern ) ) { - match.push( pattern ); - } else { - return null; - } - } - - return match; + return matchPatterns( patterns, attributes ); } // Checks if styles of provided element can be matched against provided patterns. @@ -316,31 +419,10 @@ function matchClasses( patterns, element ) { // @param {module:engine/view/element~Element} element Element which styles will be tested. // @returns {Array|null} Returns array with matched style names or `null` if no styles were matched. function matchStyles( patterns, element ) { - const match = []; - - for ( const name in patterns ) { - const pattern = patterns[ name ]; + const styleNames = element.getStyleNames(); + const attributes = styleNames.map( style => [ style, element.getStyle( style ) ] ); - if ( element.hasStyle( name ) ) { - const style = element.getStyle( name ); - - if ( pattern instanceof RegExp ) { - if ( pattern.test( style ) ) { - match.push( name ); - } else { - return null; - } - } else if ( style === pattern ) { - match.push( name ); - } else { - return null; - } - } else { - return null; - } - } - - return match; + return matchPatterns( patterns, attributes ); } /** diff --git a/packages/ckeditor5-engine/tests/view/matcher.js b/packages/ckeditor5-engine/tests/view/matcher.js index 2995a7a888c..139b42a009d 100644 --- a/packages/ckeditor5-engine/tests/view/matcher.js +++ b/packages/ckeditor5-engine/tests/view/matcher.js @@ -83,6 +83,105 @@ describe( 'Matcher', () => { expect( matcher.match( el2 ) ).to.be.null; } ); + it( 'should match all element attributes', () => { + const pattern = { + attributes: true + }; + const matcher = new Matcher( pattern ); + const el1 = new Element( document, 'p', { title: 'foobar' } ); + const el2 = new Element( document, 'p', { title: '' } ); + const el3 = new Element( document, 'p' ); + + let result = matcher.match( el1 ); + + expect( result ).to.be.an( 'object' ); + expect( result ).to.have.property( 'element' ).and.equal( el1 ); + expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); + expect( result ).to.have.property( 'match' ).that.has.property( 'attributes' ).that.is.an( 'array' ); + expect( result.match.attributes[ 0 ] ).equal( 'title' ); + + result = matcher.match( el2 ); + + expect( result ).to.be.an( 'object' ); + expect( result ).to.have.property( 'element' ).and.equal( el2 ); + expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); + expect( result ).to.have.property( 'match' ).that.has.property( 'attributes' ).that.is.an( 'array' ); + expect( result.match.attributes[ 0 ] ).equal( 'title' ); + + expect( matcher.match( el3 ) ).to.be.null; + } ); + + it( 'should match all element attributes using RegExp', () => { + const pattern = { + attributes: /data-.*/ + }; + const matcher = new Matcher( pattern ); + const el1 = new Element( document, 'p', { 'data-foo': 'foo', 'data-bar': 'bar', title: 'other' } ); + const el2 = new Element( document, 'p', { title: 'foobar' } ); + const el3 = new Element( document, 'p' ); + + const result = matcher.match( el1 ); + + expect( result ).to.be.an( 'object' ); + expect( result ).to.have.property( 'element' ).and.equal( el1 ); + expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); + expect( result ).to.have.property( 'match' ).that.has.property( 'attributes' ).that.is.an( 'array' ); + expect( result.match.attributes.length ).equal( 2 ); + expect( result.match.attributes[ 0 ] ).equal( 'data-foo' ); + expect( result.match.attributes[ 1 ] ).equal( 'data-bar' ); + + expect( matcher.match( el2 ) ).to.be.null; + expect( matcher.match( el3 ) ).to.be.null; + } ); + + it( 'should match all element attributes using key->value, where both are RegExp objects', () => { + const pattern = { + attributes: [ + { key: /data-.*/, value: /b.*?r/ } + ] + }; + const matcher = new Matcher( pattern ); + const el1 = new Element( document, 'p', { 'data-foo': 'foo', 'data-bar': 'bar', title: 'other' } ); + const el2 = new Element( document, 'p', { title: 'foobar' } ); + const el3 = new Element( document, 'p' ); + + const result = matcher.match( el1 ); + + expect( result ).to.be.an( 'object' ); + expect( result ).to.have.property( 'element' ).and.equal( el1 ); + expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); + expect( result ).to.have.property( 'match' ).that.has.property( 'attributes' ).that.is.an( 'array' ); + expect( result.match.attributes.length ).equal( 1 ); + expect( result.match.attributes[ 0 ] ).equal( 'data-bar' ); + + expect( matcher.match( el2 ) ).to.be.null; + expect( matcher.match( el3 ) ).to.be.null; + } ); + + it( 'should match all element attributes using key->value, where key is a RegExp object', () => { + const pattern = { + attributes: [ + { key: /data-.*/, value: 'bar' } + ] + }; + const matcher = new Matcher( pattern ); + const el1 = new Element( document, 'p', { 'data-foo': 'foo', 'data-bar': 'bar', title: 'other' } ); + const el2 = new Element( document, 'p', { title: 'foobar' } ); + const el3 = new Element( document, 'p' ); + + const result = matcher.match( el1 ); + + expect( result ).to.be.an( 'object' ); + expect( result ).to.have.property( 'element' ).and.equal( el1 ); + expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); + expect( result ).to.have.property( 'match' ).that.has.property( 'attributes' ).that.is.an( 'array' ); + expect( result.match.attributes.length ).equal( 1 ); + expect( result.match.attributes[ 0 ] ).equal( 'data-bar' ); + + expect( matcher.match( el2 ) ).to.be.null; + expect( matcher.match( el3 ) ).to.be.null; + } ); + it( 'should match element attributes', () => { const pattern = { attributes: { From 5cf44cf307725d99973461a4eed2ae14ec5bb80a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Thu, 13 May 2021 16:36:33 +0200 Subject: [PATCH 02/28] Update the code. --- packages/ckeditor5-engine/src/view/matcher.js | 44 +++++++++---------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/matcher.js b/packages/ckeditor5-engine/src/view/matcher.js index 3aaee746b4a..39ef3e190c1 100644 --- a/packages/ckeditor5-engine/src/view/matcher.js +++ b/packages/ckeditor5-engine/src/view/matcher.js @@ -322,29 +322,27 @@ function matchPatterns( patterns, attributes ) { } } - patterns = normalizePatterns( patterns ); - - for ( const pattern in patterns ) { - const { key: patternKey, value: patternVal } = patterns[ pattern ]; + patterns = normalizePatterns( patterns ) || []; + patterns.forEach( ( { key: patternKey, value: patternVal } ) => { attributes.forEach( ( [ attributeKey, attributeValue ] ) => { if ( - ( patternKey instanceof RegExp && patternKey.test( attributeKey ) ) || - patternKey === attributeKey + patternKey === attributeKey || + ( patternKey instanceof RegExp && patternKey.test( attributeKey ) ) ) { - if ( patternVal === true ) { - match.push( attributeKey ); - } else if ( patternVal === attributeValue ) { - match.push( attributeKey ); - } else if ( patternVal instanceof RegExp && patternVal.test( attributeValue ) ) { + if ( + patternVal === true || + patternVal === attributeValue || + ( patternVal instanceof RegExp && patternVal.test( attributeValue ) ) + ) { match.push( attributeKey ); } } } ); + } ); - if ( !match.length ) { - return null; - } + if ( !match.length ) { + return null; } return match; @@ -364,7 +362,7 @@ function matchPatterns( patterns, attributes ) { // value: /https:.*/ // } // @param {Object|Array} patterns -// @returns {Array|null} Returns an array of objects or null if patterns were not in a form of expected input. +// @returns {Array|null} Returns an array of objects or null if provided patterns were not in an expected form. function normalizePatterns( patterns ) { if ( Array.isArray( patterns ) ) { return patterns.map( @@ -382,9 +380,9 @@ function normalizePatterns( patterns ) { return Object.entries( patterns ).map( ( [ patternKey, patternValue ] ) => ( { key: patternKey, value: patternValue } ) ); - } else { - return null; } + + return null; } // Checks if attributes of provided element can be matched against provided patterns. @@ -406,10 +404,10 @@ function matchAttributes( patterns, element ) { // @param {module:engine/view/element~Element} element Element which classes will be tested. // @returns {Array|null} Returns array with matched class names or `null` if no classes were matched. function matchClasses( patterns, element ) { - const classNames = Array.from( element.getClassNames() ); - const attributes = classNames.map( className => [ className ] ); + const classes = Array.from( element.getClassNames() ) + .map( className => [ className ] ); - return matchPatterns( patterns, attributes ); + return matchPatterns( patterns, classes ); } // Checks if styles of provided element can be matched against provided patterns. @@ -419,10 +417,10 @@ function matchClasses( patterns, element ) { // @param {module:engine/view/element~Element} element Element which styles will be tested. // @returns {Array|null} Returns array with matched style names or `null` if no styles were matched. function matchStyles( patterns, element ) { - const styleNames = element.getStyleNames(); - const attributes = styleNames.map( style => [ style, element.getStyle( style ) ] ); + const styles = element.getStyleNames() + .map( style => [ style, element.getStyle( style ) ] ); - return matchPatterns( patterns, attributes ); + return matchPatterns( patterns, styles ); } /** From 39b4ad52262179d05ea7de76639d3694836baa49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Thu, 13 May 2021 22:59:26 +0200 Subject: [PATCH 03/28] Separate the conditions. --- packages/ckeditor5-engine/src/view/matcher.js | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/matcher.js b/packages/ckeditor5-engine/src/view/matcher.js index 39ef3e190c1..8b587c86ada 100644 --- a/packages/ckeditor5-engine/src/view/matcher.js +++ b/packages/ckeditor5-engine/src/view/matcher.js @@ -324,19 +324,13 @@ function matchPatterns( patterns, attributes ) { patterns = normalizePatterns( patterns ) || []; - patterns.forEach( ( { key: patternKey, value: patternVal } ) => { + patterns.forEach( ( { key: patternKey, value: patternValue } ) => { attributes.forEach( ( [ attributeKey, attributeValue ] ) => { if ( - patternKey === attributeKey || - ( patternKey instanceof RegExp && patternKey.test( attributeKey ) ) + isAttributeKeyMatched( patternKey, attributeKey ) && + isAttributeValueMatched( patternValue, attributeValue ) ) { - if ( - patternVal === true || - patternVal === attributeValue || - ( patternVal instanceof RegExp && patternVal.test( attributeValue ) ) - ) { - match.push( attributeKey ); - } + match.push( attributeKey ); } } ); } ); @@ -348,6 +342,17 @@ function matchPatterns( patterns, attributes ) { return match; } +function isAttributeKeyMatched( patternKey, attributeKey ) { + return patternKey === attributeKey || + ( patternKey instanceof RegExp && patternKey.test( attributeKey ) ); +} + +function isAttributeValueMatched( patternValue, attributeValue ) { + return patternValue === true || + patternValue === attributeValue || + ( patternValue instanceof RegExp && patternValue.test( attributeValue ) ); +} + // Bring all the possible pattern forms to an array of objects with `key` and `value` property. // For example: // From 089ca6ff09f06d8df698285cb11844a81f827301 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Fri, 14 May 2021 14:37:57 +0200 Subject: [PATCH 04/28] Update comments and tests. --- packages/ckeditor5-engine/src/view/matcher.js | 36 +++++++++++----- .../ckeditor5-engine/tests/view/matcher.js | 43 +++++++++++++++++-- 2 files changed, 64 insertions(+), 15 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/matcher.js b/packages/ckeditor5-engine/src/view/matcher.js index 8b587c86ada..c15bfab3a0a 100644 --- a/packages/ckeditor5-engine/src/view/matcher.js +++ b/packages/ckeditor5-engine/src/view/matcher.js @@ -342,17 +342,6 @@ function matchPatterns( patterns, attributes ) { return match; } -function isAttributeKeyMatched( patternKey, attributeKey ) { - return patternKey === attributeKey || - ( patternKey instanceof RegExp && patternKey.test( attributeKey ) ); -} - -function isAttributeValueMatched( patternValue, attributeValue ) { - return patternValue === true || - patternValue === attributeValue || - ( patternValue instanceof RegExp && patternValue.test( attributeValue ) ); -} - // Bring all the possible pattern forms to an array of objects with `key` and `value` property. // For example: // @@ -390,6 +379,23 @@ function normalizePatterns( patterns ) { return null; } +// @param {String|RegExp} patternKey A pattern representing attribute key we want to match. +// @param {String} attributeKey An actual attribute key (e.g. `'src'`, `'background-color'`, `'ck-widget'`) we're testing against pattern. +// @returns {Boolean} +function isAttributeKeyMatched( patternKey, attributeKey ) { + return patternKey === attributeKey || + ( patternKey instanceof RegExp && patternKey.test( attributeKey ) ); +} + +// @param {String|RegExp} patternValue A pattern representing attribute value we want to match. +// @param {String} attributeValue An actual attribute value (e.g. `'http://example.com/'`, `'red'`) we're testing against pattern. +// @returns {Boolean} +function isAttributeValueMatched( patternValue, attributeValue ) { + return patternValue === true || + patternValue === attributeValue || + ( patternValue instanceof RegExp && patternValue.test( attributeValue ) ); +} + // Checks if attributes of provided element can be matched against provided patterns. // // @param {Object} patterns Object with information about attributes to match. Each key of the object will be @@ -409,6 +415,7 @@ function matchAttributes( patterns, element ) { // @param {module:engine/view/element~Element} element Element which classes will be tested. // @returns {Array|null} Returns array with matched class names or `null` if no classes were matched. function matchClasses( patterns, element ) { + // TODO: No special case for classes. We assume the config to be reasonable (i.e. just an array of classes, not an object). const classes = Array.from( element.getClassNames() ) .map( className => [ className ] ); @@ -479,6 +486,13 @@ function matchStyles( patterns, element ) { * } * }; * + * // Match view element which has many custom data attributes. + * const pattern = { + * attributes: [ + * { key: /data-.+/, value: true } + * ] + * } + * * // Pattern with multiple properties. * const pattern = { * name: 'span', diff --git a/packages/ckeditor5-engine/tests/view/matcher.js b/packages/ckeditor5-engine/tests/view/matcher.js index 139b42a009d..9e8fdc013ad 100644 --- a/packages/ckeditor5-engine/tests/view/matcher.js +++ b/packages/ckeditor5-engine/tests/view/matcher.js @@ -134,10 +134,10 @@ describe( 'Matcher', () => { expect( matcher.match( el3 ) ).to.be.null; } ); - it( 'should match all element attributes using key->value, where both are RegExp objects', () => { + it( 'should match all element attributes using key->value, where key is a RegExp object and value is a boolean', () => { const pattern = { attributes: [ - { key: /data-.*/, value: /b.*?r/ } + { key: /data-b.*/, value: true } ] }; const matcher = new Matcher( pattern ); @@ -158,7 +158,7 @@ describe( 'Matcher', () => { expect( matcher.match( el3 ) ).to.be.null; } ); - it( 'should match all element attributes using key->value, where key is a RegExp object', () => { + it( 'should match all element attributes using key->value, where key is a RegExp object and value is a string', () => { const pattern = { attributes: [ { key: /data-.*/, value: 'bar' } @@ -182,7 +182,42 @@ describe( 'Matcher', () => { expect( matcher.match( el3 ) ).to.be.null; } ); - it( 'should match element attributes', () => { + it( 'should match all element attributes using key->value, where both are RegExp objects', () => { + const pattern = { + attributes: [ + { key: /data-.*/, value: /b.*?r/ } + ] + }; + const matcher = new Matcher( pattern ); + const el1 = new Element( document, 'p', { 'data-foo': 'foo', 'data-bar': 'bar', title: 'other' } ); + const el2 = new Element( document, 'p', { title: 'foobar' } ); + const el3 = new Element( document, 'p' ); + + const result = matcher.match( el1 ); + + expect( result ).to.be.an( 'object' ); + expect( result ).to.have.property( 'element' ).and.equal( el1 ); + expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); + expect( result ).to.have.property( 'match' ).that.has.property( 'attributes' ).that.is.an( 'array' ); + expect( result.match.attributes.length ).equal( 1 ); + expect( result.match.attributes[ 0 ] ).equal( 'data-bar' ); + + expect( matcher.match( el2 ) ).to.be.null; + expect( matcher.match( el3 ) ).to.be.null; + } ); + + it( 'should match nothing if pattern is incorrect', () => { + const pattern = { + // A stub for a non-plain object. + attributes: 1 + }; + const matcher = new Matcher( pattern ); + const el1 = new Element( document, 'p', { 'data-foo': 'foo', 'data-bar': 'bar', title: 'other' } ); + + expect( matcher.match( el1 ) ).to.be.null; + } ); + + it( 'should match element attributes using String', () => { const pattern = { attributes: { title: 'foobar' From d18de339d30da37f0a3ed6d0187fdbe56654f109 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Mon, 17 May 2021 13:16:56 +0200 Subject: [PATCH 05/28] Add missing unit tests for Matcher. --- .../ckeditor5-engine/tests/view/matcher.js | 100 ++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/packages/ckeditor5-engine/tests/view/matcher.js b/packages/ckeditor5-engine/tests/view/matcher.js index 9e8fdc013ad..de1476b479d 100644 --- a/packages/ckeditor5-engine/tests/view/matcher.js +++ b/packages/ckeditor5-engine/tests/view/matcher.js @@ -310,6 +310,30 @@ describe( 'Matcher', () => { expect( new Matcher( { classes: 'baz' } ).match( el1 ) ).to.be.null; } ); + it( 'should match element class names using an array', () => { + const pattern = { classes: [ 'foobar', 'foobaz' ] }; + const matcher = new Matcher( pattern ); + const el1 = new Element( document, 'p', { class: 'foobar' } ); + const el2 = new Element( document, 'p', { class: 'foobaz' } ); + const el3 = new Element( document, 'p', { class: 'qux' } ); + + let result = matcher.match( el1 ); + expect( result ).to.be.an( 'object' ); + expect( result ).to.have.property( 'element' ).that.equal( el1 ); + expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); + expect( result ).to.have.property( 'match' ).that.has.property( 'classes' ).that.is.an( 'array' ); + expect( result.match.classes[ 0 ] ).equal( 'foobar' ); + + result = matcher.match( el2 ); + expect( result ).to.be.an( 'object' ); + expect( result ).to.have.property( 'element' ).that.equal( el2 ); + expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); + expect( result ).to.have.property( 'match' ).that.has.property( 'classes' ).that.is.an( 'array' ); + expect( result.match.classes[ 0 ] ).equal( 'foobaz' ); + + expect( matcher.match( el3 ) ).to.be.null; + } ); + it( 'should match element class names using RegExp', () => { const pattern = { classes: /fooba./ }; const matcher = new Matcher( pattern ); @@ -353,6 +377,60 @@ describe( 'Matcher', () => { expect( new Matcher( { styles: { color: 'blue' } } ).match( el1 ) ).to.be.null; } ); + it( 'should match element styles using boolean', () => { + const pattern = { + styles: { + color: true + } + }; + const matcher = new Matcher( pattern ); + const el1 = new Element( document, 'p', { style: 'color: blue' } ); + const el2 = new Element( document, 'p', { style: 'color: darkblue' } ); + const el3 = new Element( document, 'p', { style: 'border: 1px solid' } ); + + let result = matcher.match( el1 ); + expect( result ).to.be.an( 'object' ); + expect( result ).to.have.property( 'element' ).that.equal( el1 ); + expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); + expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); + expect( result.match.styles[ 0 ] ).to.equal( 'color' ); + + result = matcher.match( el2 ); + expect( result ).to.be.an( 'object' ); + expect( result ).to.have.property( 'element' ).that.equal( el2 ); + expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); + expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); + expect( result.match.styles[ 0 ] ).to.equal( 'color' ); + + expect( matcher.match( el3 ) ).to.be.null; + } ); + + it( 'should match element styles using an array', () => { + const pattern = { + styles: [ 'color' ] + }; + const matcher = new Matcher( pattern ); + const el1 = new Element( document, 'p', { style: 'color: blue' } ); + const el2 = new Element( document, 'p', { style: 'color: darkblue' } ); + const el3 = new Element( document, 'p', { style: 'border: 1px solid' } ); + + let result = matcher.match( el1 ); + expect( result ).to.be.an( 'object' ); + expect( result ).to.have.property( 'element' ).that.equal( el1 ); + expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); + expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); + expect( result.match.styles[ 0 ] ).to.equal( 'color' ); + + result = matcher.match( el2 ); + expect( result ).to.be.an( 'object' ); + expect( result ).to.have.property( 'element' ).that.equal( el2 ); + expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); + expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); + expect( result.match.styles[ 0 ] ).to.equal( 'color' ); + + expect( matcher.match( el3 ) ).to.be.null; + } ); + it( 'should match element styles using RegExp', () => { const pattern = { styles: { @@ -380,6 +458,28 @@ describe( 'Matcher', () => { expect( matcher.match( el3 ) ).to.be.null; } ); + it( 'should match element styles using key->value, where both are RegExp objects', () => { + const pattern = { + styles: [ + { key: /border-.*/, value: /.*/ } + ] + }; + const matcher = new Matcher( pattern ); + const el1 = new Element( document, 'p', { style: 'border-top: 1px solid blue' } ); + const el2 = new Element( document, 'p', { style: 'border: 1px solid red' } ); + const el3 = new Element( document, 'p', { style: 'color: red' } ); + + const result = matcher.match( el1 ); + expect( result ).to.be.an( 'object' ); + expect( result ).to.have.property( 'element' ).that.equal( el1 ); + expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); + expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); + expect( result.match.styles[ 0 ] ).to.equal( 'border-top' ); + + expect( matcher.match( el2 ) ).to.be.null; + expect( matcher.match( el3 ) ).to.be.null; + } ); + it( 'should allow to use function as a pattern', () => { const match = { name: true }; const pattern = element => { From c3830f3e33affba7e247c5f7d808e38fc72fa055 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Mon, 17 May 2021 14:51:32 +0200 Subject: [PATCH 06/28] Denormalize/extend styles to let them be matched by patterns. --- packages/ckeditor5-engine/src/view/matcher.js | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/ckeditor5-engine/src/view/matcher.js b/packages/ckeditor5-engine/src/view/matcher.js index c15bfab3a0a..48632b8ec33 100644 --- a/packages/ckeditor5-engine/src/view/matcher.js +++ b/packages/ckeditor5-engine/src/view/matcher.js @@ -429,10 +429,29 @@ function matchClasses( patterns, element ) { // @param {module:engine/view/element~Element} element Element which styles will be tested. // @returns {Array|null} Returns array with matched style names or `null` if no styles were matched. function matchStyles( patterns, element ) { + const mapToDenormalized = ( style => { + const styles = element.getNormalizedStyle( style ); + + if ( typeof styles == 'object' ) { + return Object.entries( styles ).map( + ( [ st, val ] ) => { + return [ `${ style }-${ st }`, val ]; + } + ); + } + + return []; + } ); const styles = element.getStyleNames() .map( style => [ style, element.getStyle( style ) ] ); - return matchPatterns( patterns, styles ); + const denorm = []; + styles.forEach( ( [ style ] ) => { + const d = mapToDenormalized( style ); + denorm.push( ...d ); + } ); + + return matchPatterns( patterns, styles.concat( denorm ) ); } /** From 86d7084b5b55b430c8ef40a36d833a3e618751b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Mon, 17 May 2021 15:49:15 +0200 Subject: [PATCH 07/28] Make sure all patterns has to be matched to return a match. --- packages/ckeditor5-engine/src/view/matcher.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/ckeditor5-engine/src/view/matcher.js b/packages/ckeditor5-engine/src/view/matcher.js index 48632b8ec33..15fa4fe681a 100644 --- a/packages/ckeditor5-engine/src/view/matcher.js +++ b/packages/ckeditor5-engine/src/view/matcher.js @@ -335,7 +335,8 @@ function matchPatterns( patterns, attributes ) { } ); } ); - if ( !match.length ) { + // Return null when there was no matches or we didn't match all patterns. + if ( !match.length || match.length != patterns.length ) { return null; } From cafa6cfedf1dd7c25829355cf0de861fbeb5be41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Mon, 17 May 2021 16:24:49 +0200 Subject: [PATCH 08/28] Disable temporarily some tests. --- .../ckeditor5-engine/tests/view/matcher.js | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/packages/ckeditor5-engine/tests/view/matcher.js b/packages/ckeditor5-engine/tests/view/matcher.js index de1476b479d..b126ce68e0f 100644 --- a/packages/ckeditor5-engine/tests/view/matcher.js +++ b/packages/ckeditor5-engine/tests/view/matcher.js @@ -83,6 +83,8 @@ describe( 'Matcher', () => { expect( matcher.match( el2 ) ).to.be.null; } ); + // TODO: Add test for making sure all attributes has to be matched (AND operator instead of OR). + it( 'should match all element attributes', () => { const pattern = { attributes: true @@ -313,25 +315,28 @@ describe( 'Matcher', () => { it( 'should match element class names using an array', () => { const pattern = { classes: [ 'foobar', 'foobaz' ] }; const matcher = new Matcher( pattern ); - const el1 = new Element( document, 'p', { class: 'foobar' } ); - const el2 = new Element( document, 'p', { class: 'foobaz' } ); - const el3 = new Element( document, 'p', { class: 'qux' } ); + const el1 = new Element( document, 'p', { class: 'foobar foobaz' } ); - let result = matcher.match( el1 ); + // TODO: Uncomment after deciding whether we want AND or OR operand for array of patterns. + // const el2 = new Element( document, 'p', { class: 'foobaz' } ); + // const el3 = new Element( document, 'p', { class: 'qux' } ); + + const result = matcher.match( el1 ); expect( result ).to.be.an( 'object' ); expect( result ).to.have.property( 'element' ).that.equal( el1 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'classes' ).that.is.an( 'array' ); expect( result.match.classes[ 0 ] ).equal( 'foobar' ); - result = matcher.match( el2 ); - expect( result ).to.be.an( 'object' ); - expect( result ).to.have.property( 'element' ).that.equal( el2 ); - expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); - expect( result ).to.have.property( 'match' ).that.has.property( 'classes' ).that.is.an( 'array' ); - expect( result.match.classes[ 0 ] ).equal( 'foobaz' ); + // TODO: Uncomment after deciding whether we want AND or OR operand for array of patterns. + // result = matcher.match( el2 ); + // expect( result ).to.be.an( 'object' ); + // expect( result ).to.have.property( 'element' ).that.equal( el2 ); + // expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); + // expect( result ).to.have.property( 'match' ).that.has.property( 'classes' ).that.is.an( 'array' ); + // expect( result.match.classes[ 0 ] ).equal( 'foobaz' ); - expect( matcher.match( el3 ) ).to.be.null; + // expect( matcher.match( el3 ) ).to.be.null; } ); it( 'should match element class names using RegExp', () => { From d2d95d70aa2996a45fae7b7547e02aaa2582cbfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Mon, 17 May 2021 22:40:25 +0200 Subject: [PATCH 09/28] Clean up the code for styles expanding. --- packages/ckeditor5-engine/src/view/matcher.js | 46 +++++++++++-------- .../ckeditor5-engine/tests/view/matcher.js | 33 +++++++++++++ 2 files changed, 60 insertions(+), 19 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/matcher.js b/packages/ckeditor5-engine/src/view/matcher.js index 15fa4fe681a..cbcf495c255 100644 --- a/packages/ckeditor5-engine/src/view/matcher.js +++ b/packages/ckeditor5-engine/src/view/matcher.js @@ -430,29 +430,37 @@ function matchClasses( patterns, element ) { // @param {module:engine/view/element~Element} element Element which styles will be tested. // @returns {Array|null} Returns array with matched style names or `null` if no styles were matched. function matchStyles( patterns, element ) { - const mapToDenormalized = ( style => { - const styles = element.getNormalizedStyle( style ); + const styles = []; - if ( typeof styles == 'object' ) { - return Object.entries( styles ).map( - ( [ st, val ] ) => { - return [ `${ style }-${ st }`, val ]; - } - ); - } + element.getStyleNames() + .forEach( style => { + // Add the direct style (might be a shorthand). + styles.push( [ style, element.getStyle( style ) ] ); - return []; - } ); - const styles = element.getStyleNames() - .map( style => [ style, element.getStyle( style ) ] ); + // Try to expand the shorthand if possible. + styles.push( ...expandStyle( element, style ) ); + } ); - const denorm = []; - styles.forEach( ( [ style ] ) => { - const d = mapToDenormalized( style ); - denorm.push( ...d ); - } ); + return matchPatterns( patterns, styles ); +} + +// Expand shorthand CSS properties and return them as an array: `[ property, value ]`. +// +// @param {module:engine/view/element~Element} element Element to read styles from. +// @param {String} style CSS property name. +// @returns {Array} Pairs of expanded properties and their respective values. +function expandStyle( element, style ) { + const styles = element.getNormalizedStyle( style ); + + if ( typeof styles == 'object' ) { + return Object.entries( styles ).map( + ( [ normalizedStyle, value ] ) => { + return [ `${ style }-${ normalizedStyle }`, value ]; + } + ); + } - return matchPatterns( patterns, styles.concat( denorm ) ); + return []; } /** diff --git a/packages/ckeditor5-engine/tests/view/matcher.js b/packages/ckeditor5-engine/tests/view/matcher.js index b126ce68e0f..7459fdf3cfe 100644 --- a/packages/ckeditor5-engine/tests/view/matcher.js +++ b/packages/ckeditor5-engine/tests/view/matcher.js @@ -7,6 +7,7 @@ import Matcher from '../../src/view/matcher'; import Element from '../../src/view/element'; import Document from '../../src/view/document'; import { StylesProcessor } from '../../src/view/stylesmap'; +import { addMarginRules } from '../../src/view/styles/margin'; describe( 'Matcher', () => { let document; @@ -410,6 +411,38 @@ describe( 'Matcher', () => { expect( matcher.match( el3 ) ).to.be.null; } ); + // TODO: Should I `addStyleProcessorRules()` by myself in Matcher? + it( 'should match element styles when CSS shorthand is used', () => { + const pattern = { + styles: { + 'margin-left': /.*/ + } + }; + const matcher = new Matcher( pattern ); + + addMarginRules( document.stylesProcessor ); + + const el1 = new Element( document, 'p', { style: 'margin: 1px' } ); + const el2 = new Element( document, 'p', { style: 'margin-left: darkblue' } ); + const el3 = new Element( document, 'p', { style: 'border: 1px solid' } ); + + let result = matcher.match( el1 ); + expect( result ).to.be.an( 'object' ); + expect( result ).to.have.property( 'element' ).that.equal( el1 ); + expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); + expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); + expect( result.match.styles[ 0 ] ).to.equal( 'margin-left' ); + + result = matcher.match( el2 ); + expect( result ).to.be.an( 'object' ); + expect( result ).to.have.property( 'element' ).that.equal( el2 ); + expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); + expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); + expect( result.match.styles[ 0 ] ).to.equal( 'margin-left' ); + + expect( matcher.match( el3 ) ).to.be.null; + } ); + it( 'should match element styles using an array', () => { const pattern = { styles: [ 'color' ] From 67497d70d21c87e47f94050c18f770df549601be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Wed, 19 May 2021 12:36:56 +0200 Subject: [PATCH 10/28] Move more logic to normalization function. --- packages/ckeditor5-engine/src/view/matcher.js | 66 ++++++++----------- 1 file changed, 27 insertions(+), 39 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/matcher.js b/packages/ckeditor5-engine/src/view/matcher.js index cbcf495c255..ed85ae1ac79 100644 --- a/packages/ckeditor5-engine/src/view/matcher.js +++ b/packages/ckeditor5-engine/src/view/matcher.js @@ -74,11 +74,6 @@ export default class Matcher { item = { name: item }; } - // Single class name/RegExp can be provided. - if ( item.classes && ( typeof item.classes == 'string' || item.classes instanceof RegExp ) ) { - item.classes = [ item.classes ]; - } - this._patterns.push( item ); } } @@ -290,7 +285,7 @@ function matchName( pattern, name ) { // } // // @param {Object} patterns Object with information about attributes to match. -// @param {Array} attributes An array of key/value pairs, e.g.: +// @param {Array} items An array of key/value pairs, e.g.: // // [ // [ 'src', 'https://example.com' ], @@ -298,25 +293,13 @@ function matchName( pattern, name ) { // ] // // @returns {Array|null} Returns array with matched attribute names or `null` if no attributes were matched. -function matchPatterns( patterns, attributes ) { - const attributeKeys = attributes.map( ( [ key ] ) => key ); +function matchPatterns( patterns, items ) { + const itemKeys = items.map( ( [ key ] ) => key ); const match = []; if ( patterns === true ) { - if ( attributeKeys.length ) { - return attributeKeys; - } else { - return null; - } - } else if ( patterns instanceof RegExp ) { - attributeKeys.forEach( attribute => { - if ( patterns.test( attribute ) ) { - match.push( attribute ); - } - } ); - - if ( match.length ) { - return match; + if ( itemKeys.length ) { + return itemKeys; } else { return null; } @@ -325,18 +308,19 @@ function matchPatterns( patterns, attributes ) { patterns = normalizePatterns( patterns ) || []; patterns.forEach( ( { key: patternKey, value: patternValue } ) => { - attributes.forEach( ( [ attributeKey, attributeValue ] ) => { + items.forEach( ( [ itemKey, itemValue ] ) => { if ( - isAttributeKeyMatched( patternKey, attributeKey ) && - isAttributeValueMatched( patternValue, attributeValue ) + isKeyMatched( patternKey, itemKey ) && + isValueMatched( patternValue, itemValue ) ) { - match.push( attributeKey ); + match.push( itemKey ); } } ); } ); - // Return null when there was no matches or we didn't match all patterns. - if ( !match.length || match.length != patterns.length ) { + // Return matches only if there are at least as many of them as there are patterns. + // The RegExp pattern can match more than one item. + if ( !match.length || match.length < patterns.length ) { return null; } @@ -359,7 +343,11 @@ function matchPatterns( patterns, attributes ) { // @param {Object|Array} patterns // @returns {Array|null} Returns an array of objects or null if provided patterns were not in an expected form. function normalizePatterns( patterns ) { - if ( Array.isArray( patterns ) ) { + if ( typeof patterns == 'string' ) { + return [ { key: patterns, value: true } ]; + } else if ( patterns instanceof RegExp ) { + return [ { key: patterns, value: true } ]; + } else if ( Array.isArray( patterns ) ) { return patterns.map( pattern => { // eslint-disable-next-line dot-notation @@ -380,21 +368,21 @@ function normalizePatterns( patterns ) { return null; } -// @param {String|RegExp} patternKey A pattern representing attribute key we want to match. -// @param {String} attributeKey An actual attribute key (e.g. `'src'`, `'background-color'`, `'ck-widget'`) we're testing against pattern. +// @param {String|RegExp} patternKey A pattern representing a key we want to match. +// @param {String} itemKey An actual item key (e.g. `'src'`, `'background-color'`, `'ck-widget'`) we're testing against pattern. // @returns {Boolean} -function isAttributeKeyMatched( patternKey, attributeKey ) { - return patternKey === attributeKey || - ( patternKey instanceof RegExp && patternKey.test( attributeKey ) ); +function isKeyMatched( patternKey, itemKey ) { + return patternKey === itemKey || + ( patternKey instanceof RegExp && patternKey.test( itemKey ) ); } -// @param {String|RegExp} patternValue A pattern representing attribute value we want to match. -// @param {String} attributeValue An actual attribute value (e.g. `'http://example.com/'`, `'red'`) we're testing against pattern. +// @param {String|RegExp} patternValue A pattern representing a value we want to match. +// @param {String} itemValue An actual item value (e.g. `'http://example.com/'`, `'red'`) we're testing against pattern. // @returns {Boolean} -function isAttributeValueMatched( patternValue, attributeValue ) { +function isValueMatched( patternValue, itemValue ) { return patternValue === true || - patternValue === attributeValue || - ( patternValue instanceof RegExp && patternValue.test( attributeValue ) ); + patternValue === itemValue || + ( patternValue instanceof RegExp && patternValue.test( itemValue ) ); } // Checks if attributes of provided element can be matched against provided patterns. From c7525a2590c74c8da177e17c15ab68ded37d05b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Wed, 19 May 2021 13:46:32 +0200 Subject: [PATCH 11/28] Update tests. --- packages/ckeditor5-engine/tests/view/matcher.js | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/packages/ckeditor5-engine/tests/view/matcher.js b/packages/ckeditor5-engine/tests/view/matcher.js index 7459fdf3cfe..039b144f6ab 100644 --- a/packages/ckeditor5-engine/tests/view/matcher.js +++ b/packages/ckeditor5-engine/tests/view/matcher.js @@ -318,9 +318,8 @@ describe( 'Matcher', () => { const matcher = new Matcher( pattern ); const el1 = new Element( document, 'p', { class: 'foobar foobaz' } ); - // TODO: Uncomment after deciding whether we want AND or OR operand for array of patterns. - // const el2 = new Element( document, 'p', { class: 'foobaz' } ); - // const el3 = new Element( document, 'p', { class: 'qux' } ); + const el2 = new Element( document, 'p', { class: 'foobaz' } ); + const el3 = new Element( document, 'p', { class: 'qux' } ); const result = matcher.match( el1 ); expect( result ).to.be.an( 'object' ); @@ -329,15 +328,8 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'match' ).that.has.property( 'classes' ).that.is.an( 'array' ); expect( result.match.classes[ 0 ] ).equal( 'foobar' ); - // TODO: Uncomment after deciding whether we want AND or OR operand for array of patterns. - // result = matcher.match( el2 ); - // expect( result ).to.be.an( 'object' ); - // expect( result ).to.have.property( 'element' ).that.equal( el2 ); - // expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); - // expect( result ).to.have.property( 'match' ).that.has.property( 'classes' ).that.is.an( 'array' ); - // expect( result.match.classes[ 0 ] ).equal( 'foobaz' ); - - // expect( matcher.match( el3 ) ).to.be.null; + expect( matcher.match( el2 ) ).to.be.null; + expect( matcher.match( el3 ) ).to.be.null; } ); it( 'should match element class names using RegExp', () => { From e1944c65977aa1bea49b9b5e15eb76c14dbb5ca1 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Thu, 20 May 2021 21:51:22 +0200 Subject: [PATCH 12/28] Styles matcher refactor. --- packages/ckeditor5-engine/src/view/element.js | 5 +- packages/ckeditor5-engine/src/view/matcher.js | 115 ++++++------------ .../src/view/styles/background.js | 2 + .../ckeditor5-engine/src/view/stylesmap.js | 35 +++++- .../ckeditor5-engine/tests/view/element.js | 1 + .../ckeditor5-engine/tests/view/matcher.js | 94 ++++++++++++-- .../ckeditor5-engine/tests/view/stylesmap.js | 1 + 7 files changed, 162 insertions(+), 91 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/element.js b/packages/ckeditor5-engine/src/view/element.js index cae6426270e..37fae168dc7 100644 --- a/packages/ckeditor5-engine/src/view/element.js +++ b/packages/ckeditor5-engine/src/view/element.js @@ -468,10 +468,11 @@ export default class Element extends Node { /** * Returns iterator that contains all style names. * + * TODO param * @returns {Iterable.} */ - getStyleNames() { - return this._styles.getStyleNames(); + getStyleNames( deep = false ) { + return this._styles.getStyleNames( deep ); } /** diff --git a/packages/ckeditor5-engine/src/view/matcher.js b/packages/ckeditor5-engine/src/view/matcher.js index ed85ae1ac79..321b3ef8dba 100644 --- a/packages/ckeditor5-engine/src/view/matcher.js +++ b/packages/ckeditor5-engine/src/view/matcher.js @@ -293,25 +293,16 @@ function matchName( pattern, name ) { // ] // // @returns {Array|null} Returns array with matched attribute names or `null` if no attributes were matched. -function matchPatterns( patterns, items ) { - const itemKeys = items.map( ( [ key ] ) => key ); +function matchPatterns( patterns, items, getter ) { + const normalizedPatterns = normalizePatterns( patterns ); + const normalizedItems = Array.from( items ); const match = []; - if ( patterns === true ) { - if ( itemKeys.length ) { - return itemKeys; - } else { - return null; - } - } - - patterns = normalizePatterns( patterns ) || []; - - patterns.forEach( ( { key: patternKey, value: patternValue } ) => { - items.forEach( ( [ itemKey, itemValue ] ) => { + normalizedPatterns.forEach( ( [ patternKey, patternValue ] ) => { + normalizedItems.forEach( itemKey => { if ( isKeyMatched( patternKey, itemKey ) && - isValueMatched( patternValue, itemValue ) + isValueMatched( patternValue, itemKey, getter ) ) { match.push( itemKey ); } @@ -320,7 +311,7 @@ function matchPatterns( patterns, items ) { // Return matches only if there are at least as many of them as there are patterns. // The RegExp pattern can match more than one item. - if ( !match.length || match.length < patterns.length ) { + if ( !normalizedPatterns.length || match.length < normalizedPatterns.length ) { return null; } @@ -343,46 +334,44 @@ function matchPatterns( patterns, items ) { // @param {Object|Array} patterns // @returns {Array|null} Returns an array of objects or null if provided patterns were not in an expected form. function normalizePatterns( patterns ) { - if ( typeof patterns == 'string' ) { - return [ { key: patterns, value: true } ]; - } else if ( patterns instanceof RegExp ) { - return [ { key: patterns, value: true } ]; - } else if ( Array.isArray( patterns ) ) { - return patterns.map( - pattern => { - // eslint-disable-next-line dot-notation - if ( isPlainObject( pattern ) && pattern[ 'key' ] && pattern[ 'value' ] ) { - return pattern; - } - - // Assume the pattern is either String or RegExp. - return { key: pattern, value: true }; + if ( Array.isArray( patterns ) ) { + return patterns.map( pattern => { + if ( isPlainObject( pattern ) && pattern.key && pattern.value ) { + return [ pattern.key, pattern.value ]; } - ); + + // Assume the pattern is either String or RegExp. + return [ pattern, true ]; + } ); } else if ( isPlainObject( patterns ) ) { - return Object.entries( patterns ).map( - ( [ patternKey, patternValue ] ) => ( { key: patternKey, value: patternValue } ) - ); + return Object.entries( patterns ); } - return null; + // Other cases (true, string or regexp). + return [ [ patterns, true ] ]; } // @param {String|RegExp} patternKey A pattern representing a key we want to match. // @param {String} itemKey An actual item key (e.g. `'src'`, `'background-color'`, `'ck-widget'`) we're testing against pattern. // @returns {Boolean} function isKeyMatched( patternKey, itemKey ) { - return patternKey === itemKey || - ( patternKey instanceof RegExp && patternKey.test( itemKey ) ); + return patternKey === true || + patternKey === itemKey || + patternKey instanceof RegExp && patternKey.test( itemKey ); } // @param {String|RegExp} patternValue A pattern representing a value we want to match. +// TODO param // @param {String} itemValue An actual item value (e.g. `'http://example.com/'`, `'red'`) we're testing against pattern. // @returns {Boolean} -function isValueMatched( patternValue, itemValue ) { - return patternValue === true || - patternValue === itemValue || - ( patternValue instanceof RegExp && patternValue.test( itemValue ) ); +function isValueMatched( patternValue, itemKey, getter ) { + if ( patternValue === true ) { + return true; + } + + const itemValue = getter( itemKey ); + + return patternValue === itemValue || patternValue instanceof RegExp && patternValue.test( itemValue ); } // Checks if attributes of provided element can be matched against provided patterns. @@ -392,10 +381,7 @@ function isValueMatched( patternValue, itemValue ) { // @param {module:engine/view/element~Element} element Element which attributes will be tested. // @returns {Array|null} Returns array with matched attribute names or `null` if no attributes were matched. function matchAttributes( patterns, element ) { - // TODO: The `getAttributes()` returns `class` and `style` attribute if present. Should we exclude them when querying attributes? - const attributes = Array.from( element.getAttributes() ); - - return matchPatterns( patterns, attributes ); + return matchPatterns( patterns, element.getAttributeKeys(), key => element.getAttribute( key ) ); } // Checks if classes of provided element can be matched against provided patterns. @@ -404,11 +390,8 @@ function matchAttributes( patterns, element ) { // @param {module:engine/view/element~Element} element Element which classes will be tested. // @returns {Array|null} Returns array with matched class names or `null` if no classes were matched. function matchClasses( patterns, element ) { - // TODO: No special case for classes. We assume the config to be reasonable (i.e. just an array of classes, not an object). - const classes = Array.from( element.getClassNames() ) - .map( className => [ className ] ); - - return matchPatterns( patterns, classes ); + // We don't need `getter` here because patterns for classes are always normalized to `[ className, true ]`. + return matchPatterns( patterns, element.getClassNames() ); } // Checks if styles of provided element can be matched against provided patterns. @@ -418,37 +401,7 @@ function matchClasses( patterns, element ) { // @param {module:engine/view/element~Element} element Element which styles will be tested. // @returns {Array|null} Returns array with matched style names or `null` if no styles were matched. function matchStyles( patterns, element ) { - const styles = []; - - element.getStyleNames() - .forEach( style => { - // Add the direct style (might be a shorthand). - styles.push( [ style, element.getStyle( style ) ] ); - - // Try to expand the shorthand if possible. - styles.push( ...expandStyle( element, style ) ); - } ); - - return matchPatterns( patterns, styles ); -} - -// Expand shorthand CSS properties and return them as an array: `[ property, value ]`. -// -// @param {module:engine/view/element~Element} element Element to read styles from. -// @param {String} style CSS property name. -// @returns {Array} Pairs of expanded properties and their respective values. -function expandStyle( element, style ) { - const styles = element.getNormalizedStyle( style ); - - if ( typeof styles == 'object' ) { - return Object.entries( styles ).map( - ( [ normalizedStyle, value ] ) => { - return [ `${ style }-${ normalizedStyle }`, value ]; - } - ); - } - - return []; + return matchPatterns( patterns, element.getStyleNames( true ), key => element.getStyle( key ) ); } /** diff --git a/packages/ckeditor5-engine/src/view/styles/background.js b/packages/ckeditor5-engine/src/view/styles/background.js index 93f458c497c..70ae4ced43e 100644 --- a/packages/ckeditor5-engine/src/view/styles/background.js +++ b/packages/ckeditor5-engine/src/view/styles/background.js @@ -41,6 +41,8 @@ export function addBackgroundRules( stylesProcessor ) { return ret; } ); + + stylesProcessor.setStyleRelation( 'background', [ 'background-color' ] ); } function normalizeBackground( value ) { diff --git a/packages/ckeditor5-engine/src/view/stylesmap.js b/packages/ckeditor5-engine/src/view/stylesmap.js index f5eb855640b..baeb489b837 100644 --- a/packages/ckeditor5-engine/src/view/stylesmap.js +++ b/packages/ckeditor5-engine/src/view/stylesmap.js @@ -352,13 +352,18 @@ export default class StylesMap { /** * Returns style property names as they would appear when using {@link #toString `#toString()`}. * + * TODO param * @returns {Array.} */ - getStyleNames() { + getStyleNames( deep = false ) { if ( this.isEmpty ) { return []; } + if ( deep ) { + return this._styleProcessor.getStyleNames( this._styles ); + } + const entries = this._getStylesEntries(); return entries.map( ( [ key ] ) => key ); @@ -561,6 +566,34 @@ export class StylesProcessor { return [ [ name, normalizedValue ] ]; } + /** + * TODO + * + * @param {Object} styles Object holding normalized styles. + * @returns {Array.} + */ + getStyleNames( styles ) { + // Find all extractable styles that have a value. + const deepStyleNames = Array.from( this._consumables.keys() ).filter( name => { + const style = this.getNormalized( name, styles ); + + if ( style && typeof style == 'object' ) { + return Object.keys( style ).length; + } + + return style; + } ); + + // For simple styles (for example `color`) we don't have a map of those styles + // but they are 1 to 1 with normalized object keys. + const styleNamesKeysSet = new Set( [ + ...deepStyleNames, + ...Object.keys( styles ) + ] ); + + return Array.from( styleNamesKeysSet.values() ); + } + /** * Returns related style names. * diff --git a/packages/ckeditor5-engine/tests/view/element.js b/packages/ckeditor5-engine/tests/view/element.js index 6e36a0771a5..e089ac5474a 100644 --- a/packages/ckeditor5-engine/tests/view/element.js +++ b/packages/ckeditor5-engine/tests/view/element.js @@ -892,6 +892,7 @@ describe( 'Element', () => { } ); } ); + // TODO add tests for getStyleNames( true ) describe( 'getStyleNames', () => { it( 'should return iterator with all style names', () => { const names = [ 'color', 'position' ]; diff --git a/packages/ckeditor5-engine/tests/view/matcher.js b/packages/ckeditor5-engine/tests/view/matcher.js index 039b144f6ab..97e338d9e4c 100644 --- a/packages/ckeditor5-engine/tests/view/matcher.js +++ b/packages/ckeditor5-engine/tests/view/matcher.js @@ -8,12 +8,18 @@ import Element from '../../src/view/element'; import Document from '../../src/view/document'; import { StylesProcessor } from '../../src/view/stylesmap'; import { addMarginRules } from '../../src/view/styles/margin'; +import { addBorderRules } from '../../src/view/styles/border'; +import { addBackgroundRules } from '../../src/view/styles/background'; describe( 'Matcher', () => { let document; beforeEach( () => { document = new Document( new StylesProcessor() ); + + addMarginRules( document.stylesProcessor ); + addBorderRules( document.stylesProcessor ); + addBackgroundRules( document.stylesProcessor ); } ); describe( 'add', () => { @@ -412,10 +418,8 @@ describe( 'Matcher', () => { }; const matcher = new Matcher( pattern ); - addMarginRules( document.stylesProcessor ); - const el1 = new Element( document, 'p', { style: 'margin: 1px' } ); - const el2 = new Element( document, 'p', { style: 'margin-left: darkblue' } ); + const el2 = new Element( document, 'p', { style: 'margin-left: 10px' } ); const el3 = new Element( document, 'p', { style: 'border: 1px solid' } ); let result = matcher.match( el1 ); @@ -435,6 +439,64 @@ describe( 'Matcher', () => { expect( matcher.match( el3 ) ).to.be.null; } ); + it( 'should match element deep styles', () => { + const pattern = { + styles: { + 'border-left-style': /.*/ + } + }; + const matcher = new Matcher( pattern ); + + const el1 = new Element( document, 'p', { style: 'border: 1px solid' } ); + const el2 = new Element( document, 'p', { style: 'border-style: solid' } ); + const el3 = new Element( document, 'p', { style: 'margin-left: darkblue' } ); + + let result = matcher.match( el1 ); + expect( result ).to.be.an( 'object' ); + expect( result ).to.have.property( 'element' ).that.equal( el1 ); + expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); + expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); + expect( result.match.styles[ 0 ] ).to.equal( 'border-left-style' ); + + result = matcher.match( el2 ); + expect( result ).to.be.an( 'object' ); + expect( result ).to.have.property( 'element' ).that.equal( el2 ); + expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); + expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); + expect( result.match.styles[ 0 ] ).to.equal( 'border-left-style' ); + + expect( matcher.match( el3 ) ).to.be.null; + } ); + + it( 'should match element deep styles when CSS shorthand is used', () => { + const pattern = { + styles: { + 'border-left': /.*/ + } + }; + const matcher = new Matcher( pattern ); + + const el1 = new Element( document, 'p', { style: 'border: 1px solid' } ); + const el2 = new Element( document, 'p', { style: 'border-style: solid' } ); + const el3 = new Element( document, 'p', { style: 'margin-left: darkblue' } ); + + let result = matcher.match( el1 ); + expect( result ).to.be.an( 'object' ); + expect( result ).to.have.property( 'element' ).that.equal( el1 ); + expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); + expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); + expect( result.match.styles[ 0 ] ).to.equal( 'border-left' ); + + result = matcher.match( el2 ); + expect( result ).to.be.an( 'object' ); + expect( result ).to.have.property( 'element' ).that.equal( el2 ); + expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); + expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); + expect( result.match.styles[ 0 ] ).to.equal( 'border-left' ); + + expect( matcher.match( el3 ) ).to.be.null; + } ); + it( 'should match element styles using an array', () => { const pattern = { styles: [ 'color' ] @@ -496,17 +558,35 @@ describe( 'Matcher', () => { }; const matcher = new Matcher( pattern ); const el1 = new Element( document, 'p', { style: 'border-top: 1px solid blue' } ); - const el2 = new Element( document, 'p', { style: 'border: 1px solid red' } ); + const el2 = new Element( document, 'p', { style: 'border-top-width: 3px' } ); const el3 = new Element( document, 'p', { style: 'color: red' } ); - const result = matcher.match( el1 ); + let result = matcher.match( el1 ); expect( result ).to.be.an( 'object' ); expect( result ).to.have.property( 'element' ).that.equal( el1 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); - expect( result.match.styles[ 0 ] ).to.equal( 'border-top' ); + expect( result.match.styles ).to.deep.equal( [ + 'border-color', + 'border-style', + 'border-width', + 'border-top', + 'border-top-color', + 'border-top-style', + 'border-top-width' + ] ); + + result = matcher.match( el2 ); + expect( result ).to.be.an( 'object' ); + expect( result ).to.have.property( 'element' ).that.equal( el2 ); + expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); + expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); + expect( result.match.styles ).to.deep.equal( [ + 'border-width', + 'border-top', + 'border-top-width' + ] ); - expect( matcher.match( el2 ) ).to.be.null; expect( matcher.match( el3 ) ).to.be.null; } ); diff --git a/packages/ckeditor5-engine/tests/view/stylesmap.js b/packages/ckeditor5-engine/tests/view/stylesmap.js index 834852e9b3c..084f6b9d756 100644 --- a/packages/ckeditor5-engine/tests/view/stylesmap.js +++ b/packages/ckeditor5-engine/tests/view/stylesmap.js @@ -254,6 +254,7 @@ describe( 'StylesMap', () => { } ); } ); + // TODO tests for getStyleNames( true ) describe( 'getStyleNames()', () => { it( 'should output empty array for empty styles', () => { expect( stylesMap.getStyleNames() ).to.deep.equal( [] ); From be9cead4df2e9409d738c15a96154f142b480ae2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Mon, 24 May 2021 11:28:12 +0200 Subject: [PATCH 13/28] Add method docs. --- packages/ckeditor5-engine/src/view/element.js | 2 +- .../ckeditor5-engine/src/view/stylesmap.js | 19 +++++++++++++++---- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/element.js b/packages/ckeditor5-engine/src/view/element.js index 37fae168dc7..2a1a7a02844 100644 --- a/packages/ckeditor5-engine/src/view/element.js +++ b/packages/ckeditor5-engine/src/view/element.js @@ -468,7 +468,7 @@ export default class Element extends Node { /** * Returns iterator that contains all style names. * - * TODO param + * @param {Boolean} [deep=false] Expand shorthand style properties and all return equivalent style representations. * @returns {Iterable.} */ getStyleNames( deep = false ) { diff --git a/packages/ckeditor5-engine/src/view/stylesmap.js b/packages/ckeditor5-engine/src/view/stylesmap.js index baeb489b837..ba3b8a0e37c 100644 --- a/packages/ckeditor5-engine/src/view/stylesmap.js +++ b/packages/ckeditor5-engine/src/view/stylesmap.js @@ -25,7 +25,7 @@ export default class StylesMap { * Keeps an internal representation of styles map. Normalized styles are kept as object tree to allow unified modification and * value access model using lodash's get, set, unset, etc methods. * - * When no style processor rules are defined the it acts as simple key-value storage. + * When no style processor rules are defined it acts as simple key-value storage. * * @private * @type {Object} @@ -350,9 +350,20 @@ export default class StylesMap { } /** - * Returns style property names as they would appear when using {@link #toString `#toString()`}. + * Returns all style properties names as they would appear when using {@link #toString `#toString()`}. * - * TODO param + * When `deep` is set to true and there's a shorthand style property set, it will also return all equivalent styles: + * + * // margin: 1em + * + * will be expanded to: + * + * // margin-left: 1em + * // margin-right: 1em + * // margin-top: 1em + * // margin-bottom: 1em + * + * @param {Boolean} [deep=false] Expand shorthand style properties and all return equivalent style representations. * @returns {Array.} */ getStyleNames( deep = false ) { @@ -567,7 +578,7 @@ export class StylesProcessor { } /** - * TODO + * Return all style properties. Expand shorthand properties (e.g. `margin`, `background`). * * @param {Object} styles Object holding normalized styles. * @returns {Array.} From beb629fcc66a91e81e902ee95fc8840cb64ec913 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Mon, 24 May 2021 12:15:45 +0200 Subject: [PATCH 14/28] Add tests for `getStyleNames`. --- .../ckeditor5-engine/tests/view/element.js | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/packages/ckeditor5-engine/tests/view/element.js b/packages/ckeditor5-engine/tests/view/element.js index e089ac5474a..d44b8784d51 100644 --- a/packages/ckeditor5-engine/tests/view/element.js +++ b/packages/ckeditor5-engine/tests/view/element.js @@ -9,6 +9,8 @@ import Element from '../../src/view/element'; import Text from '../../src/view/text'; import TextProxy from '../../src/view/textproxy'; import Document from '../../src/view/document'; +import { addBorderRules } from '../../src/view/styles/border'; +import { addMarginRules } from '../../src/view/styles/margin'; import { StylesProcessor } from '../../src/view/stylesmap'; describe( 'Element', () => { @@ -892,7 +894,6 @@ describe( 'Element', () => { } ); } ); - // TODO add tests for getStyleNames( true ) describe( 'getStyleNames', () => { it( 'should return iterator with all style names', () => { const names = [ 'color', 'position' ]; @@ -911,6 +912,48 @@ describe( 'Element', () => { } ); } ); + describe( 'getStyleNames - deep = true', () => { + it( 'should return all styles in an expanded form', () => { + addBorderRules( el.document.stylesProcessor ); + addMarginRules( el.document.stylesProcessor ); + + el._setStyle( { + margin: '1 em', + border: '2px dotted silver' + } ); + + const styles = Array.from( el.getStyleNames( true ) ); + + expect( styles ).to.deep.equal( [ + 'border', + 'border-color', + 'border-style', + 'border-width', + 'border-top', + 'border-right', + 'border-bottom', + 'border-left', + 'border-top-color', + 'border-right-color', + 'border-bottom-color', + 'border-left-color', + 'border-top-style', + 'border-right-style', + 'border-bottom-style', + 'border-left-style', + 'border-top-width', + 'border-right-width', + 'border-bottom-width', + 'border-left-width', + 'margin', + 'margin-top', + 'margin-right', + 'margin-bottom', + 'margin-left' + ] ); + } ); + } ); + describe( 'hasStyle', () => { it( 'should check if element has a style', () => { el._setStyle( 'padding-top', '10px' ); From 1f30627149711795406a1010a335499ccf630619 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Mon, 31 May 2021 13:04:43 +0200 Subject: [PATCH 15/28] Adding tests. --- packages/ckeditor5-engine/src/view/stylesmap.js | 9 +++------ packages/ckeditor5-engine/tests/view/matcher.js | 3 --- packages/ckeditor5-engine/tests/view/stylesmap.js | 13 ++++++++++++- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/stylesmap.js b/packages/ckeditor5-engine/src/view/stylesmap.js index ba3b8a0e37c..4967c0a60fb 100644 --- a/packages/ckeditor5-engine/src/view/stylesmap.js +++ b/packages/ckeditor5-engine/src/view/stylesmap.js @@ -354,14 +354,11 @@ export default class StylesMap { * * When `deep` is set to true and there's a shorthand style property set, it will also return all equivalent styles: * - * // margin: 1em + * stylesMap.setTo( 'margin: 1em' ) * * will be expanded to: * - * // margin-left: 1em - * // margin-right: 1em - * // margin-top: 1em - * // margin-bottom: 1em + * [ 'margin', 'margin-top', 'margin-right', 'margin-bottom', 'margin-left' ] * * @param {Boolean} [deep=false] Expand shorthand style properties and all return equivalent style representations. * @returns {Array.} @@ -578,7 +575,7 @@ export class StylesProcessor { } /** - * Return all style properties. Expand shorthand properties (e.g. `margin`, `background`). + * Return all style properties. Also expand shorthand properties (e.g. `margin`, `background`) if * * @param {Object} styles Object holding normalized styles. * @returns {Array.} diff --git a/packages/ckeditor5-engine/tests/view/matcher.js b/packages/ckeditor5-engine/tests/view/matcher.js index 97e338d9e4c..068cf536d69 100644 --- a/packages/ckeditor5-engine/tests/view/matcher.js +++ b/packages/ckeditor5-engine/tests/view/matcher.js @@ -90,8 +90,6 @@ describe( 'Matcher', () => { expect( matcher.match( el2 ) ).to.be.null; } ); - // TODO: Add test for making sure all attributes has to be matched (AND operator instead of OR). - it( 'should match all element attributes', () => { const pattern = { attributes: true @@ -409,7 +407,6 @@ describe( 'Matcher', () => { expect( matcher.match( el3 ) ).to.be.null; } ); - // TODO: Should I `addStyleProcessorRules()` by myself in Matcher? it( 'should match element styles when CSS shorthand is used', () => { const pattern = { styles: { diff --git a/packages/ckeditor5-engine/tests/view/stylesmap.js b/packages/ckeditor5-engine/tests/view/stylesmap.js index 084f6b9d756..526c9362d0a 100644 --- a/packages/ckeditor5-engine/tests/view/stylesmap.js +++ b/packages/ckeditor5-engine/tests/view/stylesmap.js @@ -254,7 +254,6 @@ describe( 'StylesMap', () => { } ); } ); - // TODO tests for getStyleNames( true ) describe( 'getStyleNames()', () => { it( 'should output empty array for empty styles', () => { expect( stylesMap.getStyleNames() ).to.deep.equal( [] ); @@ -271,5 +270,17 @@ describe( 'StylesMap', () => { expect( stylesMap.getStyleNames() ).to.deep.equal( [ 'foo' ] ); } ); + + it( 'should output full names for known style names - deep = true', () => { + stylesMap.setTo( 'margin: 1px' ); + + expect( stylesMap.getStyleNames( true ) ).to.deep.equal( [ + 'margin', 'margin-top', 'margin-right', 'margin-bottom', 'margin-left' + ] ); + + stylesMap.setTo( 'margin-top: 1px' ); + + expect( stylesMap.getStyleNames( true ) ).to.deep.equal( [ 'margin', 'margin-top' ] ); + } ); } ); } ); From 020950cc75afa122375f6a971c2f0291ec828a1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Mon, 31 May 2021 16:39:48 +0200 Subject: [PATCH 16/28] Update JSDocs. --- packages/ckeditor5-engine/src/view/matcher.js | 45 ++++++++++--------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/matcher.js b/packages/ckeditor5-engine/src/view/matcher.js index 321b3ef8dba..b55781d4360 100644 --- a/packages/ckeditor5-engine/src/view/matcher.js +++ b/packages/ckeditor5-engine/src/view/matcher.js @@ -242,47 +242,48 @@ function matchName( pattern, name ) { // Checks if an array of key/value pairs can be matched against provided patterns. // // Patterns can be provided in a following ways: -// - a boolean value matches any attribute with any value (or no value), -// - a RegExp expression or object matches any attribute name, -// - an object matches any attribute that has the same name as the object item's key, where object item's value is: -// - equal to `true`, which matches any attribute value, -// - a string that is equal to attribute value, -// - a regular expression that matches attribute value, -// - an array with items, where the item is: -// - a string that is equal to attribute value, -// - an object with `key` and `value` property, where `key` is a regular expression matching attribute name and -// `value` is either regular expression matching attribute value or a string equal to attribute value. +// - a boolean value matches any attribute with any value (or no value): // -// pattern: true, +// pattern: true // -// // or +// - a RegExp expression or object matches any attribute name: // // pattern: /h[1-6]/ // -// // or +// - an object matches any attribute that has the same name as the object item's key, where object item's value is: +// - equal to `true`, which matches any attribute value: +// +// pattern: { +// required: true +// } +// +// - a string that is equal to attribute value: // // pattern: { -// required: true, -// src: /https.*/, // rel: 'nofollow' // } // -// // or +// - a regular expression that matches attribute value, // -// pattern: [ 'data-property-1', 'data-property-2' ], +// pattern: { +// src: /https.*/ +// } // -// // or +// - an array with items, where the item is: +// - a string that is equal to attribute value: // -// pattern: [ 'data-test', 'data-foo' ], +// pattern: [ 'data-property-1', 'data-property-2' ], // -// // or +// - an object with `key` and `value` property, where `key` is a regular expression matching attribute name and +// `value` is either regular expression matching attribute value or a string equal to attribute value: // // pattern: [ +// { key: /data-property-.*/, value: true }, +// // or: // { key: /data-property-.*/, value: 'foobar' }, -// // or +// // or: // { key: /data-property-.*/, value: /foo.*/ } // ] -// } // // @param {Object} patterns Object with information about attributes to match. // @param {Array} items An array of key/value pairs, e.g.: From 3918316cdff18c52cf781bb74d7ca4f5fd05b1c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Mon, 31 May 2021 17:55:23 +0200 Subject: [PATCH 17/28] Exclude `style` and `class` attributes from `matchAttributes` method. --- packages/ckeditor5-engine/src/view/matcher.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/ckeditor5-engine/src/view/matcher.js b/packages/ckeditor5-engine/src/view/matcher.js index b55781d4360..04158bf0243 100644 --- a/packages/ckeditor5-engine/src/view/matcher.js +++ b/packages/ckeditor5-engine/src/view/matcher.js @@ -382,7 +382,13 @@ function isValueMatched( patternValue, itemKey, getter ) { // @param {module:engine/view/element~Element} element Element which attributes will be tested. // @returns {Array|null} Returns array with matched attribute names or `null` if no attributes were matched. function matchAttributes( patterns, element ) { - return matchPatterns( patterns, element.getAttributeKeys(), key => element.getAttribute( key ) ); + // Both `class` and `style` attributes are handled in `matchClasses()` and `matchStyles()` respectively. + const attributesToExclude = new Set( [ 'class', 'style' ] ); + const attributeKeys = [ ...element.getAttributeKeys() ].filter( + key => !attributesToExclude.has( key ) + ); + + return matchPatterns( patterns, attributeKeys, key => element.getAttribute( key ) ); } // Checks if classes of provided element can be matched against provided patterns. From 78903482eba65e03878a3c74c78696512035d487 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Fri, 4 Jun 2021 09:26:36 +0200 Subject: [PATCH 18/28] Add missing comments, revert excluding style and class from matched attributes. --- packages/ckeditor5-engine/src/view/matcher.js | 31 +++++++++++-------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/matcher.js b/packages/ckeditor5-engine/src/view/matcher.js index 04158bf0243..c9c877c19ff 100644 --- a/packages/ckeditor5-engine/src/view/matcher.js +++ b/packages/ckeditor5-engine/src/view/matcher.js @@ -293,8 +293,9 @@ function matchName( pattern, name ) { // [ 'rel', 'nofollow' ] // ] // +// @param {Function} valueGetter A function providing value for a given item key. // @returns {Array|null} Returns array with matched attribute names or `null` if no attributes were matched. -function matchPatterns( patterns, items, getter ) { +function matchPatterns( patterns, items, valueGetter ) { const normalizedPatterns = normalizePatterns( patterns ); const normalizedItems = Array.from( items ); const match = []; @@ -303,7 +304,7 @@ function matchPatterns( patterns, items, getter ) { normalizedItems.forEach( itemKey => { if ( isKeyMatched( patternKey, itemKey ) && - isValueMatched( patternValue, itemKey, getter ) + isValueMatched( patternValue, itemKey, valueGetter ) ) { match.push( itemKey ); } @@ -362,15 +363,15 @@ function isKeyMatched( patternKey, itemKey ) { } // @param {String|RegExp} patternValue A pattern representing a value we want to match. -// TODO param -// @param {String} itemValue An actual item value (e.g. `'http://example.com/'`, `'red'`) we're testing against pattern. +// @param {String} itemKey An item key, e.g. `background`, `href`, 'rel', etc. +// @param {Function} valueGetter A function used to provide a value for a given `itemKey`. // @returns {Boolean} -function isValueMatched( patternValue, itemKey, getter ) { +function isValueMatched( patternValue, itemKey, valueGetter ) { if ( patternValue === true ) { return true; } - const itemValue = getter( itemKey ); + const itemValue = valueGetter( itemKey ); return patternValue === itemValue || patternValue instanceof RegExp && patternValue.test( itemValue ); } @@ -382,13 +383,7 @@ function isValueMatched( patternValue, itemKey, getter ) { // @param {module:engine/view/element~Element} element Element which attributes will be tested. // @returns {Array|null} Returns array with matched attribute names or `null` if no attributes were matched. function matchAttributes( patterns, element ) { - // Both `class` and `style` attributes are handled in `matchClasses()` and `matchStyles()` respectively. - const attributesToExclude = new Set( [ 'class', 'style' ] ); - const attributeKeys = [ ...element.getAttributeKeys() ].filter( - key => !attributesToExclude.has( key ) - ); - - return matchPatterns( patterns, attributeKeys, key => element.getAttribute( key ) ); + return matchPatterns( patterns, element.getAttributeKeys(), key => element.getAttribute( key ) ); } // Checks if classes of provided element can be matched against provided patterns. @@ -430,6 +425,11 @@ function matchStyles( patterns, element ) { * // Match view element's name. * const pattern = { name: /^p/ }; * + * // Match view element with any attribute value. + * const pattern = { + * attributes: true + * } + * * // Match view element which has matching attributes. * const pattern = { * attributes: { @@ -454,6 +454,11 @@ function matchStyles( patterns, element ) { * classes: [ 'baz', 'bar', /foo.../ ] * }; * + * // Multiple attributes to match, value does not matter. + * const pattern = { + * attributes: [ 'title', 'href', /^data-foo.*$/ ] + * }; + * * // Match view element which has given styles. * const pattern = { * styles: { From dc6a0dd25cced9f5dc8a163919943df8e8cddbe4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Fri, 4 Jun 2021 09:27:55 +0200 Subject: [PATCH 19/28] Fix typo in a comment. --- packages/ckeditor5-engine/src/view/element.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-engine/src/view/element.js b/packages/ckeditor5-engine/src/view/element.js index 2a1a7a02844..fffed818e60 100644 --- a/packages/ckeditor5-engine/src/view/element.js +++ b/packages/ckeditor5-engine/src/view/element.js @@ -468,7 +468,7 @@ export default class Element extends Node { /** * Returns iterator that contains all style names. * - * @param {Boolean} [deep=false] Expand shorthand style properties and all return equivalent style representations. + * @param {Boolean} [deep=false] Expand shorthand style properties and return all equivalent style representations. * @returns {Iterable.} */ getStyleNames( deep = false ) { From 407afe4518b6a4fbeb9c2a23d6bdd905fe007944 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Fri, 4 Jun 2021 09:32:16 +0200 Subject: [PATCH 20/28] Update docs. --- packages/ckeditor5-engine/src/view/stylesmap.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-engine/src/view/stylesmap.js b/packages/ckeditor5-engine/src/view/stylesmap.js index 4967c0a60fb..a6c5db60485 100644 --- a/packages/ckeditor5-engine/src/view/stylesmap.js +++ b/packages/ckeditor5-engine/src/view/stylesmap.js @@ -575,7 +575,7 @@ export class StylesProcessor { } /** - * Return all style properties. Also expand shorthand properties (e.g. `margin`, `background`) if + * Return all style properties. Also expand shorthand properties (e.g. `margin`, `background`) if respective extractor is available. * * @param {Object} styles Object holding normalized styles. * @returns {Array.} From da2bbb7e6c059b237bfe7ed5499da16847b679d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Tue, 8 Jun 2021 14:25:19 +0200 Subject: [PATCH 21/28] Add warning logging to the invalid config pattern. --- packages/ckeditor5-engine/src/view/matcher.js | 16 +++++++++- .../ckeditor5-engine/tests/view/matcher.js | 32 ++++++++++++++++--- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/matcher.js b/packages/ckeditor5-engine/src/view/matcher.js index c9c877c19ff..7eb3a1ffebd 100644 --- a/packages/ckeditor5-engine/src/view/matcher.js +++ b/packages/ckeditor5-engine/src/view/matcher.js @@ -7,6 +7,7 @@ * @module engine/view/matcher */ +import { logWarning } from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; import { isPlainObject } from 'lodash-es'; /** @@ -338,7 +339,12 @@ function matchPatterns( patterns, items, valueGetter ) { function normalizePatterns( patterns ) { if ( Array.isArray( patterns ) ) { return patterns.map( pattern => { - if ( isPlainObject( pattern ) && pattern.key && pattern.value ) { + if ( isPlainObject( pattern ) ) { + if ( pattern.key === undefined || pattern.value === undefined ) { + // Documented at the end of matcher.js. + logWarning( 'matcher-patterns-pattern-missing-key-or-value', pattern ); + } + return [ pattern.key, pattern.value ]; } @@ -524,3 +530,11 @@ function matchStyles( patterns, element ) { * @property {Object} [attributes] Object with key-value pairs representing attributes to match. * Each object key represents attribute name. Value can be given as `String` or `RegExp`. */ + +/** + * The key-value matcher pattern is missing key or value. Both must be present. + * Refer the documentation: {@link module:engine/view/matcher~MatcherPattern}. + * + * @param {Object} pattern Pattern with missing properties. + * @error matcher-patterns-pattern-missing-key-or-value + */ diff --git a/packages/ckeditor5-engine/tests/view/matcher.js b/packages/ckeditor5-engine/tests/view/matcher.js index 068cf536d69..c3f2b50e12b 100644 --- a/packages/ckeditor5-engine/tests/view/matcher.js +++ b/packages/ckeditor5-engine/tests/view/matcher.js @@ -3,6 +3,8 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ +/* global console */ + import Matcher from '../../src/view/matcher'; import Element from '../../src/view/element'; import Document from '../../src/view/document'; @@ -318,11 +320,10 @@ describe( 'Matcher', () => { } ); it( 'should match element class names using an array', () => { - const pattern = { classes: [ 'foobar', 'foobaz' ] }; + const pattern = { classes: [ 'foo', 'bar' ] }; const matcher = new Matcher( pattern ); - const el1 = new Element( document, 'p', { class: 'foobar foobaz' } ); - - const el2 = new Element( document, 'p', { class: 'foobaz' } ); + const el1 = new Element( document, 'p', { class: 'foo bar' } ); + const el2 = new Element( document, 'p', { class: 'bar' } ); const el3 = new Element( document, 'p', { class: 'qux' } ); const result = matcher.match( el1 ); @@ -330,7 +331,9 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'element' ).that.equal( el1 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'classes' ).that.is.an( 'array' ); - expect( result.match.classes[ 0 ] ).equal( 'foobar' ); + expect( result.match.classes.length ).equal( 2 ); + expect( result.match.classes[ 0 ] ).equal( 'foo' ); + expect( result.match.classes[ 1 ] ).equal( 'bar' ); expect( matcher.match( el2 ) ).to.be.null; expect( matcher.match( el3 ) ).to.be.null; @@ -587,6 +590,25 @@ describe( 'Matcher', () => { expect( matcher.match( el3 ) ).to.be.null; } ); + it( 'should display warning when key->value pattern is missing either key or value', () => { + const pattern = { + styles: [ + { key: /border-.*/ } + ] + }; + const warnStub = sinon.stub( console, 'warn' ); + const matcher = new Matcher( pattern ); + const el1 = new Element( document, 'p', { style: 'border-top: 1px solid blue' } ); + + matcher.match( el1 ); + + sinon.assert.calledOnceWithMatch( + warnStub, + 'matcher-patterns-pattern-missing-key-or-value', + pattern.styles[ 0 ] + ); + } ); + it( 'should allow to use function as a pattern', () => { const match = { name: true }; const pattern = element => { From 8805829e23ddd010d1f3852f98cfc5c08e4d776f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Thu, 10 Jun 2021 16:15:59 +0200 Subject: [PATCH 22/28] Review fixes. --- packages/ckeditor5-engine/src/view/element.js | 6 +- packages/ckeditor5-engine/src/view/matcher.js | 2 +- .../ckeditor5-engine/src/view/stylesmap.js | 8 +- .../ckeditor5-engine/tests/view/matcher.js | 96 +++++++++++-------- .../ckeditor5-engine/tests/view/stylesmap.js | 20 +++- 5 files changed, 83 insertions(+), 49 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/element.js b/packages/ckeditor5-engine/src/view/element.js index fffed818e60..331d0b33635 100644 --- a/packages/ckeditor5-engine/src/view/element.js +++ b/packages/ckeditor5-engine/src/view/element.js @@ -468,11 +468,11 @@ export default class Element extends Node { /** * Returns iterator that contains all style names. * - * @param {Boolean} [deep=false] Expand shorthand style properties and return all equivalent style representations. + * @param {Boolean} [expand=false] Expand shorthand style properties and return all equivalent style representations. * @returns {Iterable.} */ - getStyleNames( deep = false ) { - return this._styles.getStyleNames( deep ); + getStyleNames( expand = false ) { + return this._styles.getStyleNames( expand ); } /** diff --git a/packages/ckeditor5-engine/src/view/matcher.js b/packages/ckeditor5-engine/src/view/matcher.js index 7eb3a1ffebd..90a8ac38600 100644 --- a/packages/ckeditor5-engine/src/view/matcher.js +++ b/packages/ckeditor5-engine/src/view/matcher.js @@ -7,8 +7,8 @@ * @module engine/view/matcher */ -import { logWarning } from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; import { isPlainObject } from 'lodash-es'; +import { logWarning } from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; /** * View matcher class. diff --git a/packages/ckeditor5-engine/src/view/stylesmap.js b/packages/ckeditor5-engine/src/view/stylesmap.js index a6c5db60485..3da063df2f6 100644 --- a/packages/ckeditor5-engine/src/view/stylesmap.js +++ b/packages/ckeditor5-engine/src/view/stylesmap.js @@ -352,7 +352,7 @@ export default class StylesMap { /** * Returns all style properties names as they would appear when using {@link #toString `#toString()`}. * - * When `deep` is set to true and there's a shorthand style property set, it will also return all equivalent styles: + * When `expand` is set to true and there's a shorthand style property set, it will also return all equivalent styles: * * stylesMap.setTo( 'margin: 1em' ) * @@ -360,15 +360,15 @@ export default class StylesMap { * * [ 'margin', 'margin-top', 'margin-right', 'margin-bottom', 'margin-left' ] * - * @param {Boolean} [deep=false] Expand shorthand style properties and all return equivalent style representations. + * @param {Boolean} [expand=false] Expand shorthand style properties and all return equivalent style representations. * @returns {Array.} */ - getStyleNames( deep = false ) { + getStyleNames( expand = false ) { if ( this.isEmpty ) { return []; } - if ( deep ) { + if ( expand ) { return this._styleProcessor.getStyleNames( this._styles ); } diff --git a/packages/ckeditor5-engine/tests/view/matcher.js b/packages/ckeditor5-engine/tests/view/matcher.js index c3f2b50e12b..eed643ecdd6 100644 --- a/packages/ckeditor5-engine/tests/view/matcher.js +++ b/packages/ckeditor5-engine/tests/view/matcher.js @@ -98,7 +98,7 @@ describe( 'Matcher', () => { }; const matcher = new Matcher( pattern ); const el1 = new Element( document, 'p', { title: 'foobar' } ); - const el2 = new Element( document, 'p', { title: '' } ); + const el2 = new Element( document, 'p', { title: '', alt: 'alternative' } ); const el3 = new Element( document, 'p' ); let result = matcher.match( el1 ); @@ -107,7 +107,7 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'element' ).and.equal( el1 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'attributes' ).that.is.an( 'array' ); - expect( result.match.attributes[ 0 ] ).equal( 'title' ); + expect( result.match.attributes ).to.deep.equal( [ 'title' ] ); result = matcher.match( el2 ); @@ -115,7 +115,7 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'element' ).and.equal( el2 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'attributes' ).that.is.an( 'array' ); - expect( result.match.attributes[ 0 ] ).equal( 'title' ); + expect( result.match.attributes ).to.deep.equal( [ 'title', 'alt' ] ); expect( matcher.match( el3 ) ).to.be.null; } ); @@ -136,8 +136,7 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'attributes' ).that.is.an( 'array' ); expect( result.match.attributes.length ).equal( 2 ); - expect( result.match.attributes[ 0 ] ).equal( 'data-foo' ); - expect( result.match.attributes[ 1 ] ).equal( 'data-bar' ); + expect( result.match.attributes ).to.deep.equal( [ 'data-foo', 'data-bar' ] ); expect( matcher.match( el2 ) ).to.be.null; expect( matcher.match( el3 ) ).to.be.null; @@ -161,7 +160,7 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'attributes' ).that.is.an( 'array' ); expect( result.match.attributes.length ).equal( 1 ); - expect( result.match.attributes[ 0 ] ).equal( 'data-bar' ); + expect( result.match.attributes ).to.deep.equal( [ 'data-bar' ] ); expect( matcher.match( el2 ) ).to.be.null; expect( matcher.match( el3 ) ).to.be.null; @@ -185,7 +184,7 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'attributes' ).that.is.an( 'array' ); expect( result.match.attributes.length ).equal( 1 ); - expect( result.match.attributes[ 0 ] ).equal( 'data-bar' ); + expect( result.match.attributes ).to.deep.equal( [ 'data-bar' ] ); expect( matcher.match( el2 ) ).to.be.null; expect( matcher.match( el3 ) ).to.be.null; @@ -209,7 +208,7 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'attributes' ).that.is.an( 'array' ); expect( result.match.attributes.length ).equal( 1 ); - expect( result.match.attributes[ 0 ] ).equal( 'data-bar' ); + expect( result.match.attributes ).to.deep.equal( [ 'data-bar' ] ); expect( matcher.match( el2 ) ).to.be.null; expect( matcher.match( el3 ) ).to.be.null; @@ -245,7 +244,7 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'match' ).that.has.property( 'attributes' ).that.is.an( 'array' ); - expect( result.match.attributes[ 0 ] ).equal( 'title' ); + expect( result.match.attributes ).to.deep.equal( [ 'title' ] ); expect( matcher.match( el2 ) ).to.be.null; expect( matcher.match( el3 ) ).to.be.null; } ); @@ -266,14 +265,14 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'element' ).that.equal( el1 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'attributes' ).that.is.an( 'array' ); - expect( result.match.attributes[ 0 ] ).equal( 'title' ); + expect( result.match.attributes ).to.deep.equal( [ 'title' ] ); result = matcher.match( el2 ); expect( result ).to.be.an( 'object' ); expect( result ).to.have.property( 'element' ).that.equal( el2 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'attributes' ).that.is.an( 'array' ); - expect( result.match.attributes[ 0 ] ).equal( 'title' ); + expect( result.match.attributes ).to.deep.equal( [ 'title' ] ); expect( matcher.match( el3 ) ).to.be.null; } ); @@ -293,14 +292,14 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'element' ).that.equal( el1 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'attributes' ).that.is.an( 'array' ); - expect( result.match.attributes[ 0 ] ).equal( 'title' ); + expect( result.match.attributes ).to.deep.equal( [ 'title' ] ); result = matcher.match( el2 ); expect( result ).to.be.an( 'object' ); expect( result ).to.have.property( 'element' ).that.equal( el2 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'attributes' ).that.is.an( 'array' ); - expect( result.match.attributes[ 0 ] ).equal( 'title' ); + expect( result.match.attributes ).to.deep.equal( [ 'title' ] ); expect( matcher.match( el3 ) ).to.be.null; } ); @@ -315,7 +314,7 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'element' ).that.equal( el1 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'classes' ).that.is.an( 'array' ); - expect( result.match.classes[ 0 ] ).equal( 'foobar' ); + expect( result.match.classes ).to.deep.equal( [ 'foobar' ] ); expect( new Matcher( { classes: 'baz' } ).match( el1 ) ).to.be.null; } ); @@ -332,8 +331,7 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'classes' ).that.is.an( 'array' ); expect( result.match.classes.length ).equal( 2 ); - expect( result.match.classes[ 0 ] ).equal( 'foo' ); - expect( result.match.classes[ 1 ] ).equal( 'bar' ); + expect( result.match.classes ).to.deep.equal( [ 'foo', 'bar' ] ); expect( matcher.match( el2 ) ).to.be.null; expect( matcher.match( el3 ) ).to.be.null; @@ -351,14 +349,14 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'element' ).that.equal( el1 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'classes' ).that.is.an( 'array' ); - expect( result.match.classes[ 0 ] ).equal( 'foobar' ); + expect( result.match.classes ).to.deep.equal( [ 'foobar' ] ); result = matcher.match( el2 ); expect( result ).to.be.an( 'object' ); expect( result ).to.have.property( 'element' ).that.equal( el2 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'classes' ).that.is.an( 'array' ); - expect( result.match.classes[ 0 ] ).equal( 'foobaz' ); + expect( result.match.classes ).to.deep.equal( [ 'foobaz' ] ); expect( matcher.match( el3 ) ).to.be.null; } ); @@ -377,7 +375,7 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'element' ).that.equal( el1 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); - expect( result.match.styles[ 0 ] ).equal( 'color' ); + expect( result.match.styles ).to.deep.equal( [ 'color' ] ); expect( matcher.match( el2 ) ).to.be.null; expect( new Matcher( { styles: { color: 'blue' } } ).match( el1 ) ).to.be.null; } ); @@ -398,14 +396,14 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'element' ).that.equal( el1 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); - expect( result.match.styles[ 0 ] ).to.equal( 'color' ); + expect( result.match.styles ).to.deep.equal( [ 'color' ] ); result = matcher.match( el2 ); expect( result ).to.be.an( 'object' ); expect( result ).to.have.property( 'element' ).that.equal( el2 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); - expect( result.match.styles[ 0 ] ).to.equal( 'color' ); + expect( result.match.styles ).to.deep.equal( [ 'color' ] ); expect( matcher.match( el3 ) ).to.be.null; } ); @@ -427,14 +425,14 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'element' ).that.equal( el1 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); - expect( result.match.styles[ 0 ] ).to.equal( 'margin-left' ); + expect( result.match.styles ).to.deep.equal( [ 'margin-left' ] ); result = matcher.match( el2 ); expect( result ).to.be.an( 'object' ); expect( result ).to.have.property( 'element' ).that.equal( el2 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); - expect( result.match.styles[ 0 ] ).to.equal( 'margin-left' ); + expect( result.match.styles ).to.deep.equal( [ 'margin-left' ] ); expect( matcher.match( el3 ) ).to.be.null; } ); @@ -456,14 +454,14 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'element' ).that.equal( el1 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); - expect( result.match.styles[ 0 ] ).to.equal( 'border-left-style' ); + expect( result.match.styles ).to.deep.equal( [ 'border-left-style' ] ); result = matcher.match( el2 ); expect( result ).to.be.an( 'object' ); expect( result ).to.have.property( 'element' ).that.equal( el2 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); - expect( result.match.styles[ 0 ] ).to.equal( 'border-left-style' ); + expect( result.match.styles ).to.deep.equal( [ 'border-left-style' ] ); expect( matcher.match( el3 ) ).to.be.null; } ); @@ -485,14 +483,14 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'element' ).that.equal( el1 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); - expect( result.match.styles[ 0 ] ).to.equal( 'border-left' ); + expect( result.match.styles ).to.deep.equal( [ 'border-left' ] ); result = matcher.match( el2 ); expect( result ).to.be.an( 'object' ); expect( result ).to.have.property( 'element' ).that.equal( el2 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); - expect( result.match.styles[ 0 ] ).to.equal( 'border-left' ); + expect( result.match.styles ).to.deep.equal( [ 'border-left' ] ); expect( matcher.match( el3 ) ).to.be.null; } ); @@ -511,14 +509,14 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'element' ).that.equal( el1 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); - expect( result.match.styles[ 0 ] ).to.equal( 'color' ); + expect( result.match.styles ).to.deep.equal( [ 'color' ] ); result = matcher.match( el2 ); expect( result ).to.be.an( 'object' ); expect( result ).to.have.property( 'element' ).that.equal( el2 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); - expect( result.match.styles[ 0 ] ).to.equal( 'color' ); + expect( result.match.styles ).to.deep.equal( [ 'color' ] ); expect( matcher.match( el3 ) ).to.be.null; } ); @@ -539,14 +537,14 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'element' ).that.equal( el1 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); - expect( result.match.styles[ 0 ] ).to.equal( 'color' ); + expect( result.match.styles ).to.deep.equal( [ 'color' ] ); result = matcher.match( el2 ); expect( result ).to.be.an( 'object' ); expect( result ).to.have.property( 'element' ).that.equal( el2 ); expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.has.property( 'styles' ).that.is.an( 'array' ); - expect( result.match.styles[ 0 ] ).to.equal( 'color' ); + expect( result.match.styles ).to.deep.equal( [ 'color' ] ); expect( matcher.match( el3 ) ).to.be.null; } ); @@ -590,7 +588,7 @@ describe( 'Matcher', () => { expect( matcher.match( el3 ) ).to.be.null; } ); - it( 'should display warning when key->value pattern is missing either key or value', () => { + it( 'should display warning when key->value pattern is missing key', () => { const pattern = { styles: [ { key: /border-.*/ } @@ -604,9 +602,32 @@ describe( 'Matcher', () => { sinon.assert.calledOnceWithMatch( warnStub, - 'matcher-patterns-pattern-missing-key-or-value', + 'matcher-pattern-missing-key-or-value', pattern.styles[ 0 ] ); + + sinon.restore(); + } ); + + it( 'should display warning when key->value pattern is missing value', () => { + const pattern = { + styles: [ + { value: 'red' } + ] + }; + const warnStub = sinon.stub( console, 'warn' ); + const matcher = new Matcher( pattern ); + const el1 = new Element( document, 'p', { style: 'border-top: 1px solid blue' } ); + + matcher.match( el1 ); + + sinon.assert.calledOnceWithMatch( + warnStub, + 'matcher-pattern-missing-key-or-value', + pattern.styles[ 0 ] + ); + + sinon.restore(); } ); it( 'should allow to use function as a pattern', () => { @@ -666,8 +687,7 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.is.an( 'object' ); expect( result.match ).to.have.property( 'attributes' ).that.is.an( 'array' ); - expect( result.match.attributes[ 0 ] ).to.equal( 'name' ); - expect( result.match.attributes[ 1 ] ).to.equal( 'title' ); + expect( result.match.attributes ).to.deep.equal( [ 'name', 'title' ] ); } ); it( 'should match multiple classes', () => { @@ -685,8 +705,7 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.is.an( 'object' ); expect( result.match ).to.have.property( 'classes' ).that.is.an( 'array' ); - expect( result.match.classes[ 0 ] ).to.equal( 'foo' ); - expect( result.match.classes[ 1 ] ).to.equal( 'bar' ); + expect( result.match.classes ).to.deep.equal( [ 'foo', 'bar' ] ); } ); it( 'should match multiple styles', () => { @@ -710,8 +729,7 @@ describe( 'Matcher', () => { expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); expect( result ).to.have.property( 'match' ).that.is.an( 'object' ); expect( result.match ).to.have.property( 'styles' ).that.is.an( 'array' ); - expect( result.match.styles[ 0 ] ).to.equal( 'color' ); - expect( result.match.styles[ 1 ] ).to.equal( 'position' ); + expect( result.match.styles ).to.deep.equal( [ 'color', 'position' ] ); } ); } ); diff --git a/packages/ckeditor5-engine/tests/view/stylesmap.js b/packages/ckeditor5-engine/tests/view/stylesmap.js index 526c9362d0a..0978d1f397c 100644 --- a/packages/ckeditor5-engine/tests/view/stylesmap.js +++ b/packages/ckeditor5-engine/tests/view/stylesmap.js @@ -6,13 +6,14 @@ import StylesMap, { StylesProcessor } from '../../src/view/stylesmap'; import encodedImage from './_utils/encodedimage.txt'; import { addMarginRules } from '../../src/view/styles/margin'; +import { addBorderRules } from '../../src/view/styles/border'; import { getBoxSidesValueReducer } from '../../src/view/styles/utils'; describe( 'StylesMap', () => { - let stylesMap; + let stylesMap, stylesProcessor; beforeEach( () => { - const stylesProcessor = new StylesProcessor(); + stylesProcessor = new StylesProcessor(); // Define simple "foo" shorthand normalizers, similar to the "margin" shorthand normalizers, for testing purposes. stylesProcessor.setNormalizer( 'foo', value => ( { @@ -282,5 +283,20 @@ describe( 'StylesMap', () => { expect( stylesMap.getStyleNames( true ) ).to.deep.equal( [ 'margin', 'margin-top' ] ); } ); + + it( 'should output full names for known style names - deep = true - other extractors must not affect the output', () => { + // Let's add this line to ensure that only matching extractors are used to expand style names. + addBorderRules( stylesProcessor ); + + stylesMap.setTo( 'margin: 1px' ); + + expect( stylesMap.getStyleNames( true ) ).to.deep.equal( [ + 'margin', 'margin-top', 'margin-right', 'margin-bottom', 'margin-left' + ] ); + + stylesMap.setTo( 'margin-top: 1px' ); + + expect( stylesMap.getStyleNames( true ) ).to.deep.equal( [ 'margin', 'margin-top' ] ); + } ); } ); } ); From b47a065af789e5892ab60b45f4b8921eb9987abf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Thu, 10 Jun 2021 17:16:52 +0200 Subject: [PATCH 23/28] Adding missing changes. --- packages/ckeditor5-engine/src/view/matcher.js | 59 +++++++++++++++---- 1 file changed, 47 insertions(+), 12 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/matcher.js b/packages/ckeditor5-engine/src/view/matcher.js index 90a8ac38600..6defab11de0 100644 --- a/packages/ckeditor5-engine/src/view/matcher.js +++ b/packages/ckeditor5-engine/src/view/matcher.js @@ -8,6 +8,7 @@ */ import { isPlainObject } from 'lodash-es'; + import { logWarning } from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; /** @@ -321,19 +322,51 @@ function matchPatterns( patterns, items, valueGetter ) { return match; } -// Bring all the possible pattern forms to an array of objects with `key` and `value` property. -// For example: +// Bring all the possible pattern forms to an array of arrays where first item is a key and second is a value. +// +// Examples: +// +// Boolean pattern value: +// +// true +// +// to +// +// [ [ true, true ] ] +// +// Textual pattern value: +// +// 'attribute-name-or-class-or-style' +// +// to +// +// [ [ 'attribute-name-or-class-or-style', true ] ] // -// { -// src: /https:.*/ -// } +// Regular expression: +// +// /^data-.*$/ +// +// to +// +// [ [ /^data-.*$/, true ] ] +// +// Objects (plain or with `key` and `value` specified explicitly): +// +// { +// src: /^https:.*$/ +// } +// +// or +// +// [ { +// key: 'src', +// value: /^https:.*$/ +// } ] // // to: // -// { -// key: 'src', -// value: /https:.*/ -// } +// [ [ 'src', /^https:.*$/ ] ] +// // @param {Object|Array} patterns // @returns {Array|null} Returns an array of objects or null if provided patterns were not in an expected form. function normalizePatterns( patterns ) { @@ -342,7 +375,7 @@ function normalizePatterns( patterns ) { if ( isPlainObject( pattern ) ) { if ( pattern.key === undefined || pattern.value === undefined ) { // Documented at the end of matcher.js. - logWarning( 'matcher-patterns-pattern-missing-key-or-value', pattern ); + logWarning( 'matcher-pattern-missing-key-or-value', pattern ); } return [ pattern.key, pattern.value ]; @@ -351,7 +384,9 @@ function normalizePatterns( patterns ) { // Assume the pattern is either String or RegExp. return [ pattern, true ]; } ); - } else if ( isPlainObject( patterns ) ) { + } + + if ( isPlainObject( patterns ) ) { return Object.entries( patterns ); } @@ -536,5 +571,5 @@ function matchStyles( patterns, element ) { * Refer the documentation: {@link module:engine/view/matcher~MatcherPattern}. * * @param {Object} pattern Pattern with missing properties. - * @error matcher-patterns-pattern-missing-key-or-value + * @error matcher-pattern-missing-key-or-value */ From 98a630e2d983e9acea4de27788991e79fc13471c Mon Sep 17 00:00:00 2001 From: Jacek Bogdanski Date: Mon, 14 Jun 2021 09:52:55 +0200 Subject: [PATCH 24/28] Renaming missing deep - expand changes. --- packages/ckeditor5-engine/src/view/stylesmap.js | 4 ++-- packages/ckeditor5-engine/tests/view/element.js | 2 +- packages/ckeditor5-engine/tests/view/stylesmap.js | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/stylesmap.js b/packages/ckeditor5-engine/src/view/stylesmap.js index 3da063df2f6..71107624658 100644 --- a/packages/ckeditor5-engine/src/view/stylesmap.js +++ b/packages/ckeditor5-engine/src/view/stylesmap.js @@ -582,7 +582,7 @@ export class StylesProcessor { */ getStyleNames( styles ) { // Find all extractable styles that have a value. - const deepStyleNames = Array.from( this._consumables.keys() ).filter( name => { + const expandedStyleNames = Array.from( this._consumables.keys() ).filter( name => { const style = this.getNormalized( name, styles ); if ( style && typeof style == 'object' ) { @@ -595,7 +595,7 @@ export class StylesProcessor { // For simple styles (for example `color`) we don't have a map of those styles // but they are 1 to 1 with normalized object keys. const styleNamesKeysSet = new Set( [ - ...deepStyleNames, + ...expandedStyleNames, ...Object.keys( styles ) ] ); diff --git a/packages/ckeditor5-engine/tests/view/element.js b/packages/ckeditor5-engine/tests/view/element.js index d44b8784d51..576a3e45064 100644 --- a/packages/ckeditor5-engine/tests/view/element.js +++ b/packages/ckeditor5-engine/tests/view/element.js @@ -912,7 +912,7 @@ describe( 'Element', () => { } ); } ); - describe( 'getStyleNames - deep = true', () => { + describe( 'getStyleNames - expand = true', () => { it( 'should return all styles in an expanded form', () => { addBorderRules( el.document.stylesProcessor ); addMarginRules( el.document.stylesProcessor ); diff --git a/packages/ckeditor5-engine/tests/view/stylesmap.js b/packages/ckeditor5-engine/tests/view/stylesmap.js index 0978d1f397c..e2dd33ea529 100644 --- a/packages/ckeditor5-engine/tests/view/stylesmap.js +++ b/packages/ckeditor5-engine/tests/view/stylesmap.js @@ -272,7 +272,7 @@ describe( 'StylesMap', () => { expect( stylesMap.getStyleNames() ).to.deep.equal( [ 'foo' ] ); } ); - it( 'should output full names for known style names - deep = true', () => { + it( 'should output full names for known style names - expand = true', () => { stylesMap.setTo( 'margin: 1px' ); expect( stylesMap.getStyleNames( true ) ).to.deep.equal( [ @@ -284,7 +284,7 @@ describe( 'StylesMap', () => { expect( stylesMap.getStyleNames( true ) ).to.deep.equal( [ 'margin', 'margin-top' ] ); } ); - it( 'should output full names for known style names - deep = true - other extractors must not affect the output', () => { + it( 'should output full names for known style names - expand = true - other extractors must not affect the output', () => { // Let's add this line to ensure that only matching extractors are used to expand style names. addBorderRules( stylesProcessor ); From 3cbc8b17889a5a3886598f62b212a36b2cb55efb Mon Sep 17 00:00:00 2001 From: Jacek Bogdanski Date: Mon, 14 Jun 2021 12:50:01 +0200 Subject: [PATCH 25/28] Improved documentation for new matcher options, added missing tests. --- packages/ckeditor5-engine/src/view/matcher.js | 170 +++++++++++++++--- .../ckeditor5-engine/tests/view/matcher.js | 54 +++++- 2 files changed, 199 insertions(+), 25 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/matcher.js b/packages/ckeditor5-engine/src/view/matcher.js index 6defab11de0..06a81002f9a 100644 --- a/packages/ckeditor5-engine/src/view/matcher.js +++ b/packages/ckeditor5-engine/src/view/matcher.js @@ -462,62 +462,186 @@ function matchStyles( patterns, element ) { * const pattern = /^p/; * * If `MatcherPattern` is given as an `Object`, all the object's properties will be matched with view element properties. + * Let's examine available pattern options: * - * // Match view element's name. - * const pattern = { name: /^p/ }; + * Matching view element: + * + * // Match view element's name using String: + * const pattern = { name: 'p' }; + * + * // or by providing RegExp: + * const pattern = { name: /^(ul|ol)$/ }; + * + * // The name can also be skipped to match any view element with matching attributes: + * const pattern = { + * attributes: { + * 'title': true + * } + * }; + * + * Matching view element attributes: * * // Match view element with any attribute value. * const pattern = { + * name: 'p', * attributes: true - * } + * }; + * + * // Match view element which has matching attributes (String). + * const pattern = { + * name: 'figure', + * attributes: 'title' // Match title attribute (can be empty). + * }; * - * // Match view element which has matching attributes. + * // Match view element which has matching attributes (RegExp). * const pattern = { + * name: 'figure', + * attributes: /^data-.*$/ // Match attributes starting with `data-` e.g. `data-foo` with any value (can be empty). + * }; + * + * // Match view element which has matching attributes (Object). + * const pattern = { + * name: 'figure', * attributes: { - * title: 'foobar', // Attribute title should equal 'foobar'. - * foo: /^\w+/, // Attribute foo should match /^\w+/ regexp. - * bar: true // Attribute bar should be set (can be empty). + * title: 'foobar', // Match `title` attribute with 'foobar' value. + * alt: true, // Match `alt` attribute with any value (can be empty). + * 'data-type': /^(jpg|png)$/ // Match `data-type` attribute with `jpg` or `png` value. * } * }; * - * // Match view element which has given class. + * // Match view element which has matching attributes (Array). * const pattern = { - * classes: 'foobar' + * name: 'figure', + * attributes: [ + * 'title', // Match `title` attribute (can be empty). + * /^data-*$/, // Match attributes starting with `data-` e.g. `data-foo` with any value (can be empty). +* ] * }; * - * // Match view element class using regular expression. + * // Match view element which has matching attributes (key-value-pairs). * const pattern = { - * classes: /foo.../ + * name: 'input', + * attributes: [ + * { + * key: 'type', // Match `type` as an attribute key. + * value: /^(text|number|date)$/ }, // Match `text`, `number` or `date` values. + * { + * key: /^data-.*$/, // Match attributes starting with `data-` e.g. `data-foo`. + * value: true // Match any value (can be empty). +* } +* ] * }; +* + * Matching view element styles: * - * // Multiple classes to match. + * // Match view element with any style. * const pattern = { - * classes: [ 'baz', 'bar', /foo.../ ] + * name: 'p', + * styles: true + * }; +* + * // Match view element which has matching styles (String). + * const pattern = { + * name: 'p', + * styles: 'color' // Match attributes with `color` style. * }; * - * // Multiple attributes to match, value does not matter. + * // Match view element which has matching styles (RegExp). * const pattern = { - * attributes: [ 'title', 'href', /^data-foo.*$/ ] + * name: 'p', + * styles: /^border.*$/ // Match view element with any border style. * }; * - * // Match view element which has given styles. + * // Match view element which has matching styles (Object). * const pattern = { - * styles: { - * position: 'absolute', - * color: /^\w*blue$/ + * name: 'p', + * attributes: { + * color: /rgb\((\d{1,3}), (\d{1,3}), (\d{1,3})\)/, // Match `color` in RGB format only. + * 'font-weight': 600, // Match `font-weight` only if it's `600`. + * 'text-decoration': true // Match any text decoration. * } * }; * - * // Match view element which has many custom data attributes. + * // Match view element which has matching styles (Array). + * const pattern = { + * name: 'p', + * attributes: [ + * 'color', // Match `color` with any value. + * /^border.*$/, // Match all border properties. +* ] + * }; + * + * // Match view element which has matching styles (key-value-pairs). * const pattern = { + * name: 'p', * attributes: [ - * { key: /data-.+/, value: true } + * { + * key: 'color', // Match `color` as an property key. + * value: /rgb\((\d{1,3}), (\d{1,3}), (\d{1,3})\)/, // Match RGB format only. + * { + * key: /^border.*$/, // Match any border style. + * value: true // Match any value. +* } +* ] + * }; + * + * Matching view element classes: + * + * // Match view element with any class. + * const pattern = { + * name: 'p', + * classes: true + * }; +* + * // Match view element which has matching class (String). + * const pattern = { + * name: 'p', + * classes: 'highlighted' // Match `highlighted` class. + * }; + * + * // Match view element which has matching classes (RegExp). + * const pattern = { + * name: 'figure', + * classes: /^image-side-(left|right)$/ // Match `image-side-left` or `image-side-right` class. + * }; + * + * // Match view element which has matching classes (Object). + * const pattern = { + * name: 'p', + * classes: { + * highlighted: true, // Match `highlighted` class. + * marker: true // Match `marker` class. + * } + * }; + * + * // Match view element which has matching classes (Array). + * const pattern = { + * name: 'figure', + * classes: [ + * 'image', // Match `image` class. + * /^image-side-(left|right)$/ // Match `image-side-left` or `image-side-right` class. * ] - * } + * }; + * + * // Match view element which has matching classes (key-value-pairs). + * const pattern = { + * name: 'figure', + * classes: [ + * { + * key: 'image', // Match `image` class. + * value: true + * { + * key: /^image-side-(left|right)$/, // Match `image-side-left` or `image-side-right` class. + * value: true +* } +* ] + * }; + * + * Pattern can combine multiple properties allowing for more complex view element matching: * - * // Pattern with multiple properties. * const pattern = { * name: 'span', + * attributes: [ 'title' ], * styles: { * 'font-weight': 'bold' * }, diff --git a/packages/ckeditor5-engine/tests/view/matcher.js b/packages/ckeditor5-engine/tests/view/matcher.js index eed643ecdd6..03272df841a 100644 --- a/packages/ckeditor5-engine/tests/view/matcher.js +++ b/packages/ckeditor5-engine/tests/view/matcher.js @@ -337,6 +337,56 @@ describe( 'Matcher', () => { expect( matcher.match( el3 ) ).to.be.null; } ); + it( 'should match element class names using an object', () => { + const pattern = { + classes: { + foo: true, + bar: true + } + }; + + const matcher = new Matcher( pattern ); + const el1 = new Element( document, 'p', { class: 'foo bar' } ); + const el2 = new Element( document, 'p', { class: 'bar' } ); + const el3 = new Element( document, 'p', { class: 'qux' } ); + + const result = matcher.match( el1 ); + expect( result ).to.be.an( 'object' ); + expect( result ).to.have.property( 'element' ).that.equal( el1 ); + expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); + expect( result ).to.have.property( 'match' ).that.has.property( 'classes' ).that.is.an( 'array' ); + expect( result.match.classes.length ).equal( 2 ); + expect( result.match.classes ).to.deep.equal( [ 'foo', 'bar' ] ); + + expect( matcher.match( el2 ) ).to.be.null; + expect( matcher.match( el3 ) ).to.be.null; + } ); + + it( 'should match element class names using key->value pairs', () => { + const pattern = { + classes: [ + { key: 'foo', value: true }, + { key: /^b.*$/, value: true } + ] + }; + + const matcher = new Matcher( pattern ); + const el1 = new Element( document, 'p', { class: 'foo bar' } ); + const el2 = new Element( document, 'p', { class: 'bar' } ); + const el3 = new Element( document, 'p', { class: 'qux' } ); + + const result = matcher.match( el1 ); + expect( result ).to.be.an( 'object' ); + expect( result ).to.have.property( 'element' ).that.equal( el1 ); + expect( result ).to.have.property( 'pattern' ).that.equal( pattern ); + expect( result ).to.have.property( 'match' ).that.has.property( 'classes' ).that.is.an( 'array' ); + expect( result.match.classes.length ).equal( 2 ); + expect( result.match.classes ).to.deep.equal( [ 'foo', 'bar' ] ); + + expect( matcher.match( el2 ) ).to.be.null; + expect( matcher.match( el3 ) ).to.be.null; + } ); + it( 'should match element class names using RegExp', () => { const pattern = { classes: /fooba./ }; const matcher = new Matcher( pattern ); @@ -437,7 +487,7 @@ describe( 'Matcher', () => { expect( matcher.match( el3 ) ).to.be.null; } ); - it( 'should match element deep styles', () => { + it( 'should match element expanded styles', () => { const pattern = { styles: { 'border-left-style': /.*/ @@ -466,7 +516,7 @@ describe( 'Matcher', () => { expect( matcher.match( el3 ) ).to.be.null; } ); - it( 'should match element deep styles when CSS shorthand is used', () => { + it( 'should match element expanded styles when CSS shorthand is used', () => { const pattern = { styles: { 'border-left': /.*/ From 0ad487842cf2d4a96dd296404051dab8d655f403 Mon Sep 17 00:00:00 2001 From: Jacek Bogdanski Date: Mon, 14 Jun 2021 13:30:16 +0200 Subject: [PATCH 26/28] Docs minor rewording. --- packages/ckeditor5-engine/src/view/matcher.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/matcher.js b/packages/ckeditor5-engine/src/view/matcher.js index 06a81002f9a..9c5e1ab2ce5 100644 --- a/packages/ckeditor5-engine/src/view/matcher.js +++ b/packages/ckeditor5-engine/src/view/matcher.js @@ -518,7 +518,7 @@ function matchStyles( patterns, element ) { * ] * }; * - * // Match view element which has matching attributes (key-value-pairs). + * // Match view element which has matching attributes (key-value pairs). * const pattern = { * name: 'input', * attributes: [ @@ -571,7 +571,7 @@ function matchStyles( patterns, element ) { * ] * }; * - * // Match view element which has matching styles (key-value-pairs). + * // Match view element which has matching styles (key-value pairs). * const pattern = { * name: 'p', * attributes: [ @@ -623,7 +623,7 @@ function matchStyles( patterns, element ) { * ] * }; * - * // Match view element which has matching classes (key-value-pairs). + * // Match view element which has matching classes (key-value pairs). * const pattern = { * name: 'figure', * classes: [ From c3249dd6cc75a54fd13efb2c0d51667b5e08724a Mon Sep 17 00:00:00 2001 From: Jacek Bogdanski Date: Mon, 14 Jun 2021 13:40:39 +0200 Subject: [PATCH 27/28] Added info that matcher is matching all or nothing. --- packages/ckeditor5-engine/src/view/matcher.js | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/matcher.js b/packages/ckeditor5-engine/src/view/matcher.js index 9c5e1ab2ce5..18c0b107e54 100644 --- a/packages/ckeditor5-engine/src/view/matcher.js +++ b/packages/ckeditor5-engine/src/view/matcher.js @@ -462,7 +462,8 @@ function matchStyles( patterns, element ) { * const pattern = /^p/; * * If `MatcherPattern` is given as an `Object`, all the object's properties will be matched with view element properties. - * Let's examine available pattern options: + * If the view element won't meet all of the object's pattern properties, the match won't happen. + * Available `Object` matching properties: * * Matching view element: * @@ -515,7 +516,7 @@ function matchStyles( patterns, element ) { * attributes: [ * 'title', // Match `title` attribute (can be empty). * /^data-*$/, // Match attributes starting with `data-` e.g. `data-foo` with any value (can be empty). -* ] + * ] * }; * * // Match view element which has matching attributes (key-value pairs). @@ -528,10 +529,10 @@ function matchStyles( patterns, element ) { * { * key: /^data-.*$/, // Match attributes starting with `data-` e.g. `data-foo`. * value: true // Match any value (can be empty). -* } -* ] + * } + * ] * }; -* + * * Matching view element styles: * * // Match view element with any style. @@ -539,7 +540,7 @@ function matchStyles( patterns, element ) { * name: 'p', * styles: true * }; -* + * * // Match view element which has matching styles (String). * const pattern = { * name: 'p', @@ -568,7 +569,7 @@ function matchStyles( patterns, element ) { * attributes: [ * 'color', // Match `color` with any value. * /^border.*$/, // Match all border properties. -* ] + * ] * }; * * // Match view element which has matching styles (key-value pairs). @@ -581,8 +582,8 @@ function matchStyles( patterns, element ) { * { * key: /^border.*$/, // Match any border style. * value: true // Match any value. -* } -* ] + * } + * ] * }; * * Matching view element classes: @@ -592,7 +593,7 @@ function matchStyles( patterns, element ) { * name: 'p', * classes: true * }; -* + * * // Match view element which has matching class (String). * const pattern = { * name: 'p', @@ -633,8 +634,8 @@ function matchStyles( patterns, element ) { * { * key: /^image-side-(left|right)$/, // Match `image-side-left` or `image-side-right` class. * value: true -* } -* ] + * } + * ] * }; * * Pattern can combine multiple properties allowing for more complex view element matching: From ea439f17c2b131a9b5ea5d2d672cd7452e50ae0e Mon Sep 17 00:00:00 2001 From: Jacek Bogdanski Date: Mon, 14 Jun 2021 15:03:15 +0200 Subject: [PATCH 28/28] Rewording. --- packages/ckeditor5-engine/src/view/matcher.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-engine/src/view/matcher.js b/packages/ckeditor5-engine/src/view/matcher.js index 18c0b107e54..dafb46fcb2a 100644 --- a/packages/ckeditor5-engine/src/view/matcher.js +++ b/packages/ckeditor5-engine/src/view/matcher.js @@ -462,7 +462,7 @@ function matchStyles( patterns, element ) { * const pattern = /^p/; * * If `MatcherPattern` is given as an `Object`, all the object's properties will be matched with view element properties. - * If the view element won't meet all of the object's pattern properties, the match won't happen. + * If the view element does not meet all of the object's pattern properties, the match will not happen. * Available `Object` matching properties: * * Matching view element: