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

All editor features should consume attributes they put in the model during upcast #11532

Closed
oleq opened this issue Mar 30, 2022 · 3 comments
Closed
Assignees
Labels
package:code-block package:engine package:html-support package:image package:language package:media-embed package:style resolution:expired This issue was closed due to lack of feedback. squad:core Issue to be handled by the Core team. status:stale type:bug This issue reports a buggy (incorrect) behavior.

Comments

@oleq
Copy link
Member

oleq commented Mar 30, 2022

📝 Provide detailed reproduction steps (if any)

In #11530 we came across an issue with GHS (double-)consuming the src of an image and creating a mess. The issue was crazy but the fix pretty straightforward.

While fixing this my gut told me there could be other features that also touch attributes in the upcast pipeline but not consume them, which would spawn dozens of #11530 clones in the future. 

To check this, I created a simple plugin

function( editor ) {
    editor.data.upcastDispatcher.on( 'element', ( evt, data, conversionApi ) => {
        console.group( `element:${ data.viewItem.name }` );

        for ( const [ key ] of data.viewItem.getAttributes() ) {
            if ( conversionApi.consumable.test( data.viewItem, { attributes: key } ) ) {
                console.warn( `Looks like "${ key }" was not consumed.`, data.viewItem );
            }
        }

        console.groupEnd();
    }, { priority: 'lowest' } );
} 

and ran it against the all-features manual test. You can add it straight to plugins: [] or better yet: put it in the implementation of Paragraph because it's everywhere and you can then run just any test (manual or automatic). 

I discovered there are plenty of plugins that leave things unconsumed, these are just a few examples:

CodeBlock

Using code blocks + GHS + text direction

<span lang="plaintext" dir="undefined"><pre><code class="language-plaintext">foo</code></pre></span>

MediaEmbed

TextPartLanguage

ImageInline

Image again

More Image

We need to review all of them at patch them up because otherwise we'll hit it again and again while working on GHS. Just keep in mind that some attributes may not need to be consumed if they were ignored by the feature (not transferred to the model).


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

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

oleq commented Mar 30, 2022

Side thought: Could my plugin be used to test things automatically? Or as a dev tool? In the inspector?

@CKEditorBot
Copy link
Collaborator

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

@CKEditorBot
Copy link
Collaborator

We've closed your issue due to inactivity over the last year. We understand that the issue may still be relevant. If so, feel free to open a new one (and link this issue to it).

@CKEditorBot CKEditorBot added the resolution:expired This issue was closed due to lack of feedback. label Nov 1, 2023
@CKEditorBot CKEditorBot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:code-block package:engine package:html-support package:image package:language package:media-embed package:style resolution:expired This issue was closed due to lack of feedback. squad:core Issue to be handled by the Core team. status:stale type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

No branches or pull requests

4 participants