Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

The image upcast converter does not consume the src attribute #11530

Closed
Reinmar opened this issue Mar 30, 2022 · 1 comment · Fixed by #11531
Closed

The image upcast converter does not consume the src attribute #11530

Reinmar opened this issue Mar 30, 2022 · 1 comment · Fixed by #11531
Assignees
Labels
package:html-support package:image squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@Reinmar
Copy link
Member

Reinmar commented Mar 30, 2022

📝 Provide detailed reproduction steps (if any)

The src attribute appears in GHS's htmlAttributes:

The problem is here:

export function getImgViewElementMatcher( editor, matchImageType ) {
if ( editor.plugins.has( 'ImageInlineEditing' ) !== editor.plugins.has( 'ImageBlockEditing' ) ) {
return { name: 'img' };
}
const imageUtils = editor.plugins.get( 'ImageUtils' );
return element => {
// Check if view element is an `img`.
if ( !imageUtils.isInlineImageView( element ) ) {
return null;
}
// The <img> can be standalone, wrapped in <figure>...</figure> (ImageBlock plugin) or
// wrapped in <figure><a>...</a></figure> (LinkImage plugin).
const imageType = element.findAncestor( imageUtils.isBlockImageView ) ? 'imageBlock' : 'imageInline';
if ( imageType !== matchImageType ) {
return null;
}
return { name: true };
};
}

The consume pattern returned by the callback does not include src.

The problem is though that we cannot easily just add the src to all patterns as we must still upcast images without src. So, the line

needs to be moved to the inside of this callback because only the callback is able to use a different match and consume patterns (match only name,  consume all).

Additionally, there's also a related issue  #11327 that perhaps might be fixed together with this one.


If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@Reinmar Reinmar added the type:bug This issue reports a buggy (incorrect) behavior. label Mar 30, 2022
@Reinmar
Copy link
Member Author

Reinmar commented Mar 30, 2022

Additionally, there's also a related issue  #11327 that perhaps might be fixed together with this one.

Upon further analysis of both these scenarios, the low-level matcher and consumable calls should work more or less like this:

match = matcher.match( { name: 'img' } )

if ( !match )
    return;

if ( !consumable.test( { name: 'img' } ) )
    return;

consumable.consume( { name: true } );
createImage();

if ( !consumable.test( { attributes: [ 'src' ] } ) ) {
    setSrc();
    consumable.consume( { attributes: [ 'src' ] } )
}

However, IDK how to implement all these callbacks that we have right now (view patterns, model callbacks, two separate converters for block and image features) to ensure that in all cases the above will happen.

@Reinmar Reinmar added package:image package:html-support squad:core Issue to be handled by the Core team. labels Mar 30, 2022
@CKEditorBot CKEditorBot added the status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. label Mar 30, 2022
@CKEditorBot CKEditorBot added status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. and removed status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. labels Mar 30, 2022
dawidurbanski added a commit that referenced this issue Mar 30, 2022
Fix (image): The image upcast converter should consume the src attribute. Closes #11530.
@CKEditorBot CKEditorBot removed the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Mar 30, 2022
@CKEditorBot CKEditorBot added this to the iteration 52 milestone Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:html-support package:image squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants