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

An editor with GHS and Font plugins will split a singular span into two separate ones #16120

Open
mabryl opened this issue Mar 28, 2024 · 4 comments
Labels
package:font package:html-support squad:core Issue to be handled by the Core team. support:2 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@mabryl
Copy link
Contributor

mabryl commented Mar 28, 2024

📝 Provide detailed reproduction steps (if any)

  1. Pick an editor build that contains GHS, FontColor and FontBackroundColor
  2. Provide the following HTML to the editor: <span custom-attribute="foo" style="background:red; color:green;">test</span>
  3. See how the data changes once it's inserted into the editor

✔️ Expected result

In an editor build that contains just GHS, the singular span is retained:

image

❌ Actual result

Once you include FontColor and FontBackgroundColor in the editor, these plugins will take precedence over GHS and a second span will be inserted:

image

❓ Possible solution

This is only reproducible if there's something non-standard in the data that GHS needs to handle separately. In the example above, the issue is not reproducible if your remove custom-attribute="foo" from the original data.

Possibly related to #15757

📃 Other details

  • Browser: cross-browser
  • OS: cross-OS
  • First affected CKEditor version: N/A
  • Installed CKEditor plugins: GeneralHtmlSupport, FontColor, FontBackgroundColor

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

@mabryl mabryl added type:bug This issue reports a buggy (incorrect) behavior. package:font support:2 An issue reported by a commercially licensed client. squad:core Issue to be handled by the Core team. package:html-support labels Mar 28, 2024
@Witoso
Copy link
Member

Witoso commented Apr 2, 2024

@niegowski I assume this is expected after our changes (and is a “won't fix” on our side). We fixed the bug, and background is consumed properly by the Font plugin (and will be handled by this plugin in the UI), and custom-attribute is handled by GHS. Therefore, the engine correctly outputs two span's.

@niegowski
Copy link
Contributor

@niegowski I assume this is expected after our changes (and is a “won't fix” on our side). We fixed the bug, and background is consumed properly by the Font plugin (and will be handled by this plugin in the UI), and custom-attribute is handled by GHS. Therefore, the engine correctly outputs two span's.

This is unrelated to the recent fix for background. It can be reproduced with code as simple as <span custom-attribute="foo" style="color:green;">test</span>. This is related to the view AttributeElement priority. These spans could get merged into a single span but those features generate AttributeElements at different priorities and this makes it non-mergeable. The font color feature uses priority 7 and the generic GHS span uses the default 10. I'm not sure why we use different priorities for font features. We use 5 for links because they should not get split in HTML if we have part of it formatted, for example, bolded.

@Reinmar
Copy link
Member

Reinmar commented Apr 3, 2024

As far, we pull the font styles out to prevent situations like this one: https://jsfiddle.net/Lcjzntkd/

image

@Witoso
Copy link
Member

Witoso commented Apr 4, 2024

Rel: #2291

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:font package:html-support squad:core Issue to be handled by the Core team. support:2 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

No branches or pull requests

4 participants