ckeditor / ckeditor5

Powerful rich text editor framework with a modular architecture, modern integrations, and features like collaborative editing.
https://ckeditor.com/ckeditor-5
Other
8.78k stars 3.64k forks source link

GHS: Add support for <hr> element #12973

Open Mgsy opened 1 year ago

Mgsy commented 1 year ago

📝 Provide a description of the improvement

Currently, it's impossible to use styled <hr> element, as GHS doesn't render it in the editing view, because it is marked as an unsafe element. On the other hand, adding the horizontal line feature enables rendering, but the styling is lost.

This ticket is about adding support for <hr> element in GHS, so it will render in the editing view and keep the original styling.

Example markup:

<hr style="height: 5px;background: teal;margin: 20px 0;box-shadow: 0px 0px 4px 2px rgba(204,204,204,1);">

Related: #9960


If you'd like to see this improvement implemented, add a 👍 reaction to this post.

cscomfort commented 1 year ago

Not only is it impossible to style an <hr> tag, you cannot insert a custom styled one via the paid-for premium "Content Template" feature. The reason, so far as I can tell, that the <hr> tag cannot be styled is because of the drag and drop CKE5 feature that takes over the element. The only success I've had working around this is to create a content template that wraps the tag in a two parent <div>s and then write a corresponding style.

<div class="hr-3">
  <div>
    <hr>
  </div>
</div>
.hr-3 hr {
  border-width: 3px;
}

We should not have to write styles that force us to create unnecessary markup and bleed over to our public stylesheets in order to work around a CKE UI feature.

Witoso commented 1 year ago

@cscomfort the reason is mentioned in the OP, as hr is marked as unsafe, and most likely it doesn't have an integration with GHS or Style dropdown.

<span data-ck-unsafe-element="hr" style="background:teal;box-shadow:0px 0px 4px 2px rgba(204,204,204,1);height:5px;margin:20px 0;"></span>

Which, I must say, is a bit confusing, especially the unsafe part. @ckeditor/ckeditor-5-core-developers, is there a reason why the hr isn't safe?

oleq commented 1 year ago

@cscomfort the reason is mentioned in the OP, as hr is marked as unsafe, and most likely it doesn't have an integration with GHS or Style dropdown.

<span data-ck-unsafe-element="hr" style="background:teal;box-shadow:0px 0px 4px 2px rgba(204,204,204,1);height:5px;margin:20px 0;"></span>

Which, I must say, is a bit confusing, especially the unsafe part. @ckeditor/ckeditor-5-core-developers, is there a reason why the hr isn't safe?

https://github.com/ckeditor/ckeditor5/blob/a411cd749442e369022bafa2dbf1c29a9fd035ca/packages/ckeditor5-html-support/src/schemadefinitions.ts#L21-L22

More in https://github.com/ckeditor/ckeditor5/issues/12228.

On a side note, https://ckeditor5.github.io/docs/nightly/ckeditor5/latest/api/module_html-support_generalhtmlsupportconfig-GeneralHtmlSupportConfig.html#member-allowEmpty is related but for inline elements only. It's a new config, though and it hasn't been released yet.

Probably @niegowski would know more about it.

niegowski commented 1 year ago

It falls back to a custom element handling (hr is not defined in the data schema), making it an unsafe element.

agbockus commented 7 months ago

Just checking in to see if there has been any update or movement on solving this issue that's not linked above? Or is there a workaround available? Thanks!

matbk49751 commented 7 months ago

Why is adding a class attribute to the hr element considered unsafe? Is it primarily due to security reasons, or are there other concerns related to data schema and element handling? I tried the below. But, I'm still not able to add class attributes to the hr element inside source mode.

I'm able to add a css class onto a div/span element. Wouldn't that be deemed unsafe as well? That's why this doesn't make sense to me.

schema.extend('horizontalLine', {
      inheritAllFrom: '$blockObject',
      isEmpty: false,
      allowAttributes: ['class'],
});
Witoso commented 7 months ago

@matbk49751 unsafe meaning editor may not correctly interpret it, not because it poses some security threat (not the best choice of word, I agree) but only in a GHS setting (as well as Style dropdown which is connected to GHS). But it's possible to enable the way you were trying to, you just need to inform the editor how to convert this attribute. Example:

editor.model.schema.extend( 'horizontalLine', { allowAttributes: [ 'hrClass' ] } )

editor.conversion.attributeToAttribute( {
    model: 'hrClass',
    view: 'class'
} );
matbk49751 commented 6 months ago

@matbk49751 unsafe meaning editor may not correctly interpret it, not because it poses some security threat (not the best choice of word, I agree) but only in a GHS setting (as well as Style dropdown which is connected to GHS). But it's possible to enable the way you were trying to, you just need to inform the editor how to convert this attribute. Example:

editor.model.schema.extend( 'horizontalLine', { allowAttributes: [ 'hrClass' ] } )

editor.conversion.attributeToAttribute( {
    model: 'hrClass',
    view: 'class'
} );

Thank you! That worked.

sofiaolinda commented 6 months ago

Hi @matbk49751, How did you manage to resolve this? I don't understand where I should insert this. I need to insert this into my Drupal project

matbk49751 commented 6 months ago

Hi @matbk49751, How did you manage to resolve this? I don't understand where I should insert this. I need to insert this into my Drupal project

Hi @sofiaolinda ! To integrate CKEditor5 with Drupal 10, start by creating a CKEditor5 build within your custom Drupal module. Afterward, export the corresponding classes to Drupal, enabling seamless access to your CKEditor5 values. Below, you'll find a list of modules and guides that proved helpful throughout this process. Feel free to reach out if you have any questions!

https://www.drupal.org/project/editor_advanced_link https://www.qed42.com/insights/everything-you-need-to-know-about-ckeditor-5-integration-for-drupal-9#top https://redfinsolutions.com/blog/how-build-custom-ckeditor5-plugins-drupal

sofiaolinda commented 6 months ago

@matbk49751 Thank you very much for your availability. Can you share your example? I'm still having difficulties. I'm trying to create a Ckeditor5 plugin for this. I already have another custom plugin to insert a character and it is working. But for this change in the hr tag I'm not able to, for example, I don't understand which library I will have to import into horizontalLine.js of my plugin

import { Plugin } from 'ckeditor5/src/core'; // ???

Structure in Drupal : -| docroot/ --| modules/ ---| custom/ -----| mymodule_ckeditor5/ -------| css/ -------| js/ ---------| build/ ---------| ckeditor5_plugins/ -----------| horizontalLine/ -------------| src/ ---------------| index.js ---------------| horizontalLine.js -----| node_modules/ -----| mymodule_ckeditor5.ckeditor5.yml -----| mymodule_ckeditor5.info.yml -----| mymodule_ckeditor5.libraries.yml -----| package.json ...

matbk49751 commented 5 months ago

@matbk49751 Thank you very much for your availability. Can you share your example? I'm still having difficulties. I'm trying to create a Ckeditor5 plugin for this. I already have another custom plugin to insert a character and it is working. But for this change in the hr tag I'm not able to, for example, I don't understand which library I will have to import into horizontalLine.js of my plugin

import { Plugin } from 'ckeditor5/src/core'; // ???

Structure in Drupal : -| docroot/ --| modules/ ---| custom/ -----| mymodule_ckeditor5/ -------| css/ -------| js/ ---------| build/ ---------| ckeditor5_plugins/ -----------| horizontalLine/ -------------| src/ ---------------| index.js ---------------| horizontalLine.js -----| node_modules/ -----| mymodule_ckeditor5.ckeditor5.yml -----| mymodule_ckeditor5.info.yml -----| mymodule_ckeditor5.libraries.yml -----| package.json ...

Hi @sofiaolinda ! Your module structure looks good. But, are you compiling your ckeditor5 build with webpack like the editor_advanced_link module? You want to make sure that you are importing the manifest file for ckeditor5, that way you can access their class names and not have to install the entire Ckeditor5 twice (Since Drupal 10 already has it installed in core) . example of this below.

webpack.config.js

plugins: [
  // It is possible to require the ckeditor5-dll.manifest.json used in
  // core/node_modules rather than having to install CKEditor 5 here.
  // However, that requires knowing the location of that file relative to
  // where your module code is located.
  new webpack.DllReferencePlugin({
    manifest: require('./node_modules/ckeditor5/build/ckeditor5-dll.manifest.json'), // eslint-disable-line global-require, import/no-unresolved
    scope: 'ckeditor5/src',
    name: 'CKEditor5.dll',
  }),
],
sofiaolinda commented 5 months ago

Hi @matbk49751 Thank you very much once again for your availability. Something strange happens in my code that I can't identify. 1 - Yes, I installed the "editor_advanced_link" module and activated the option: "Limit allowed HTML tags..." and the compilation is being carried out using webpack.config.js, including the configuration you showed is the same, the file .js is being created. But I'm not sure I understood when you said "compiling your ckeditor5 build with webpack like the editor_advanced_link module". Is there anything else I need to do? 2 - I realize that I can't use the name "horizontalLine", it's as if this name causes conflict with the standard Ckeditor5 plugin, I don't have any answer, so I changed it to "horizontalLinePlugin". In this case I get a javascript error: "CKEditorError: schema-cannot-extend-missing-item {"itemName":"horizontalLine"}" 3 - My index.js file looked like this:

import { Plugin } from 'ckeditor5/src/core'; class HorizontalLine extends Plugin { init() { const editor = this.editor; editor.model.schema.extend( 'horizontalLine', { allowAttributes: [ 'hrClass' ] } ) editor.conversion.attributeToAttribute( { model: 'hrClass', view: 'class' } ); } } export default { 'HorizontalLine': HorizontalLine }

Can you give me any more tips? Thanks!

matbk49751 commented 5 months ago

Hi @matbk49751 Thank you very much once again for your availability. Something strange happens in my code that I can't identify. 1 - Yes, I installed the "editor_advanced_link" module and activated the option: "Limit allowed HTML tags..." and the compilation is being carried out using webpack.config.js, including the configuration you showed is the same, the file .js is being created. But I'm not sure I understood when you said "compiling your ckeditor5 build with webpack like the editor_advanced_link module". Is there anything else I need to do? 2 - I realize that I can't use the name "horizontalLine", it's as if this name causes conflict with the standard Ckeditor5 plugin, I don't have any answer, so I changed it to "horizontalLinePlugin". In this case I get a javascript error: "CKEditorError: schema-cannot-extend-missing-item {"itemName":"horizontalLine"}" 3 - My index.js file looked like this:

import { Plugin } from 'ckeditor5/src/core'; class HorizontalLine extends Plugin { init() { const editor = this.editor; editor.model.schema.extend( 'horizontalLine', { allowAttributes: [ 'hrClass' ] } ) editor.conversion.attributeToAttribute( { model: 'hrClass', view: 'class' } ); } } export default { 'HorizontalLine': HorizontalLine }

Can you give me any more tips? Thanks!

Hi @sofiaolinda Can you post your work on your Github Repo and send me the link, so I can take a look? This is more of a Drupal question rather than a Ckeditor5 question. Thank you!

jasonmacer commented 4 months ago

@matbk49751 unsafe meaning editor may not correctly interpret it, not because it poses some security threat (not the best choice of word, I agree) but only in a GHS setting (as well as Style dropdown which is connected to GHS). But it's possible to enable the way you were trying to, you just need to inform the editor how to convert this attribute. Example:

editor.model.schema.extend( 'horizontalLine', { allowAttributes: [ 'hrClass' ] } )

editor.conversion.attributeToAttribute( {
    model: 'hrClass',
    view: 'class'
} );

Thank you! That worked.

@matbk49751 can i ask where you went to update this code in drupal/module file?

Thank you!

ducviethaboo commented 4 months ago

@matbk49751 unsafe meaning editor may not correctly interpret it, not because it poses some security threat (not the best choice of word, I agree) but only in a GHS setting (as well as Style dropdown which is connected to GHS). But it's possible to enable the way you were trying to, you just need to inform the editor how to convert this attribute. Example:

editor.model.schema.extend( 'horizontalLine', { allowAttributes: [ 'hrClass' ] } )

editor.conversion.attributeToAttribute( {
    model: 'hrClass',
    view: 'class'
} );

Thank you! That worked.

@matbk49751 can i ask where you went to update this code in drupal/module file?

Thank you!

First you have to create custom ckeditor5 plugin, following https://redfinsolutions.com/blog/how-build-custom-ckeditor5-plugins-drupal , then implement that code in init() function declare in your js file. Check this: https://git.drupalcode.org/issue/drupal-3398223/-/commit/267d9928115d86cf068165f8f4142f31c020f02d

jasonmacer commented 4 months ago

Good morning @ducviethaboo, if I'm understanding the second link you posted, I'm assuming I can make those changes and additions in my module and it will allow the setting of classes for the hr element?

I ask this because I am running Drupal core 10.2.4 and it did not have CKEditor5 as a core module. I had to add it as a normal module and enable it. Drupal core did have "CKEditor 4 LTS - WYSIWYG HTML editor" 1.0.1 but it appears that you need a license to use this.

matbk49751 commented 3 months ago

Good morning @ducviethaboo, if I'm understanding the second link you posted, I'm assuming I can make those changes and additions in my module and it will allow the setting of classes for the hr element?

I ask this because I am running Drupal core 10.2.4 and it did not have CKEditor5 as a core module. I had to add it as a normal module and enable it. Drupal core did have "CKEditor 4 LTS - WYSIWYG HTML editor" 1.0.1 but it appears that you need a license to use this.

Hi @jasonmacer ! To answer your first question, Yes. the second link is a good guide to follow for creating a drupal module allowing you to have the ability to add a css class to hr. Furthermore, Ckeditor5 should come out-of-the-box with Drupal 10. I recommend using Ckeditor5 over Ckeditor4 for this example. Let me know if you have questions.