Richeditor 'Code view' strips 'form' tag even though it's in the 'Allowed tags' list

ranmichael commented 6 years ago

Expected behavior

Html <form> tag and its attributes are saved.

Actual behavior

Html <form> and </form> tags are automatically stripped

Reproduce steps

Edit a page using StaticPages plug-in
In the richeditor, go to 'code view'

Try to save a valid form, for example:


<form>
First name:<br>
<input type="text" name="firstname"><br>
Last name:<br>
<input type="text" name="lastname">
</form>


- Hit save (or ctrl+s)

Notice that the opening and closing `<form>` tags (and attirbutes, if present) disappear, while the `<input>` tags are successfully saved.

##### October build
Tested both on builds 419 and 430.

alxy commented 6 years ago

The allowed tags section is for the codeeditor, not the richeditor widget. I would like to question. wether it is a good idea to allow arbitrary HTML tags in the richeditor. Can't you use a snippet for that purpose?

ranmichael commented 6 years ago

Thanks @alxy for your attention.

The allowed tags section is for the codeeditor

Exactly. I am using the 'code view' accessible via the RichEditor. What am I missing?

Can't you use a snippet for that purpose?

Actually, I first tried to use a snippet for this, but there is an issue with rendering snippets in Static Pages: Snippets work in the built-in 'Content' Richeditor or 'simply' added RichEditors.

But, when I add a RichEditor in a Repeater, there is a parsing problem of snippets. I get: <figure data-snippet="basicContactForm"> </figure> Instead of the actual form.

Edit: Just to be clear on what I was originally trying to achieve: Add a form inside a Repeater. (Allowing a different form for each Repeater item)

Snippet fails (need to open a bug for this)
Workaround I: <form></form> within a RichEditor (not the best practice) fails. (It gets stripped - should be possible - hence this Bug Open)
Workaround II: I added a text field in the repeater called 'formName' and I inject the partial with the input of 'formName' from the user... Ugly. But works, and you can validate the user input.

Any thoughts?

LukeTowers commented 6 years ago

@ranmichael The real bug here is with the snippet support, not the richeditor. The codeeditor is an entirely separate formwidget from the richeditor, it is not the "code view" for the richeditor. The richeditor should not (and will not) be allowing <form> tags

ranmichael commented 6 years ago

Thanks @LukeTowers for the clarifications. I figured it might be stripped on purpose - but in that case it is weird that form <input> tags are not stripped... no? Is that the bug then?

On a side note, Snippets, which are an excellent idea, are giving me a hard time since I started using them. Sometimes snippets break also in the default 'Content' tab in Static pages.

i.e. I get a <figure> in the html instead of the actual code - when modifying the text around a snippet in the static page RichEditor.

(But work when doing the same modification directly on the file with Sublime Text Editor)... Was wondering if anyone else experiences this.

Thanks again

Grawl commented 6 years ago

@LukeTowers

The richeditor should not (and will not) be allowing <form> tags

why?

LukeTowers commented 6 years ago

Because the richeditor is not an HTML editor, it's a WYSIWYG editor for non-technical users. Why do you need to have form tags in the editor? What are you trying to achieve? @Grawl

Grawl commented 6 years ago

Many users know HTML, it's not a rocket science. I want to add a Google AdSense ad code loking like this:

<script async src="//pagead2.googlesyndication.com/pagead/js/adsbygoogle.js"></script>
<ins class="adsbygoogle"
     style="display:block; text-align:center;"
     data-ad-layout="in-article"
     data-ad-format="fluid"
     data-ad-client="ca-pub-#############"
     data-ad-slot="#########"></ins>
<script>
     (adsbygoogle = window.adsbygoogle || []).push({});
</script>

I added script and ins tags in all fields (except of "tags to remove") in editor settings but still this code is removed from editor on save or code / WYSIWYG toggle.

alxy commented 6 years ago

@Grawl Use a snippet in that case, it's really the perfect use case. If you are not using the staticpages plugin, there is a plugin on the marketplace that enables snippets for any richeditor field.

LukeTowers commented 6 years ago

@Grawl this really is better suited to a snippet or a component. Allowing users to paste script and form tags is not the best solution. I do agree though that the if you've told the EditorSettings that you specifically don't want them removed and they are still being removed then that's something to look into.

Grawl commented 6 years ago

Please, let me control what I want to have on my frontend, okay? If I want to use <my-calc data-options='{ "foo": "bar" }'></my-calc> to insert a calculator inside any article, why editor removes this HTML when I turn on HTML edit mode? Why I want to use HTML editor mode if I cannot add any custom HTML I cannot add with editor buttons?

I want to control my website. Please, let me control my content or I will change visual editor plugin with any other that will not remove code I want to save.

LukeTowers commented 6 years ago

Froala supports this, at least in the latest version: https://codepen.io/anon/pen/aqEPqQ; so it should be working in October. I know we need to update the distributed version; so perhaps doing that will fix it. @daftspunk?

Grawl commented 6 years ago

I just added <my-calc data-options='{ "foo": "bar" }'></my-calc> to editor in this pen and it has been removed on HTML mode toggle.

LukeTowers commented 6 years ago

@Grawl you have to add data-options to the list of allowed attributes.

Grawl commented 6 years ago

but <my-calc> element is also removed

LukeTowers commented 6 years ago

Because it has a not allowed attribute.

Grawl commented 6 years ago

Okay it's working on this pen if I add htmlAllowedEmptyTags: ['mycalc'] to JS config.

But in October when I add mycalc to "allowed tags", "allowed empty tags" and "tags to not wrap" fields it's not working.

LukeTowers commented 6 years ago

Like I said, it's possible that updating the distributed version of Froala is required to fix this.

datune commented 6 years ago

@LukeTowers You can safely change the status of this ticket to a confirmed bug, because I can easily reproduce it.

Steps to reproduce:

Add rich editor field
Click the code view button in your textarea rich editor widget
Enter: <caption>My Caption</caption>
Hit Save or ctrl+s and observe how the caption tags are replaced with p tags

A few notes: Yes, the caption tag is set by default in the allowed tags settings (/backend/system/settings/update/october/backend/editor). I'm not sure why this is happening in the first place, it seems that the allowed tag setting in the backend is completely ignored. Is this intentional?

Are the backend editor settings related only to the code view editor widget, and NOT the code view integrated into the rich editor (ie. froala) code view? (If so, we should document this properly, cause it's easy to get confused...)

I have to say, I agree with Grawl, IF I allow my users to use the (froala) code view editor, I want them to be able to add tags that I allow, while still being able to use WYSIWYG.

datune commented 6 years ago

@LukeTowers I even tried extending the DEFAULTS object, but it makes no difference. I'm out of ideas atm. I feel like the code below should make this work.

+function ($) { "use strict";
    $(document).render(function() {
        if ($.FroalaEditor) {
            $.FroalaEditor.DEFAULTS = $.extend($.FroalaEditor.DEFAULTS, {
                htmlAllowedTags: ['a', 'abbr', 'address', 'area', 'article', 'aside', 'audio', 'b', 'base', 'bdi', 'bdo', 'blockquote', 'br', 'button', 'canvas', 'caption', 'cite', 'code', 'col', 'colgroup', 'datalist', 'dd', 'del', 'details', 'dfn', 'dialog', 'div', 'dl', 'dt', 'em', 'embed', 'fieldset', 'figcaption', 'figure', 'footer', 'form', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'header', 'hgroup', 'hr', 'i', 'iframe', 'img', 'input', 'ins', 'kbd', 'keygen', 'label', 'legend', 'li', 'link', 'main', 'map', 'mark', 'menu', 'menuitem', 'meter', 'nav', 'noscript', 'object', 'ol', 'optgroup', 'option', 'output', 'p', 'param', 'pre', 'progress', 'queue', 'rp', 'rt', 'ruby', 's', 'samp', 'script', 'style', 'section', 'select', 'small', 'source', 'span', 'strike', 'strong', 'sub', 'summary', 'sup', 'table', 'tbody', 'td', 'textarea', 'tfoot', 'th', 'thead', 'time', 'title', 'tr', 'track', 'u', 'ul', 'var', 'video', 'wbr']                
            });
        }        
    })
}(window.jQuery);

After the above, I wanted to see how it behaves in a sandbox, and this is where it starts to get weird. I've tested it using this fiddle, and, it does not work either?!?

Just load the fiddle, hit the code view button, enter <caption>My Caption</caption>, toggle the code view button twice, and you'll see the caption is replaced by a p tag.

What am I missing here?

tfprado commented 5 years ago

Not sure if I should open a new issue, but I am interested in being able to define allowed html attributes for the rich editor (Project needs to use Vue Js and allow content editors to edit pages using a WYSIWYG editor).

I had originally found this closed issue #2576 and played with the code along with the Froala options page.

+function ($) { "use strict";
    $(document).render(function() {
        if ($.FroalaEditor) {
            $.FroalaEditor.DEFAULTS = $.extend($.FroalaEditor.DEFAULTS, {
                htmlAllowedAttrs: ['v-model', 'v-if', 'v-bind', 'v-html', 'a', 'abbr', 'address', 'area', 'article', 'aside', 'audio', 'b', 'base', 'bdi', 'bdo', 'blockquote', 'br', 'button', 'canvas', 'caption', 'cite', 'code', 'col', 'colgroup', 'datalist', 'dd', 'del', 'details', 'dfn', 'dialog', 'div', 'dl', 'dt', 'em', 'embed', 'fieldset', 'figcaption', 'figure', 'footer', 'form', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'header', 'hgroup', 'hr', 'i', 'iframe', 'img', 'input', 'ins', 'kbd', 'keygen', 'label', 'legend', 'li', 'link', 'main', 'map', 'mark', 'menu', 'menuitem', 'meter', 'nav', 'noscript', 'object', 'ol', 'optgroup', 'option', 'output', 'p', 'param', 'pre', 'progress', 'queue', 'rp', 'rt', 'ruby', 's', 'samp', 'script', 'style', 'section', 'select', 'small', 'source', 'span', 'strike', 'strong', 'sub', 'summary', 'sup', 'table', 'tbody', 'td', 'textarea', 'tfoot', 'th', 'thead', 'time', 'title', 'tr', 'track', 'u', 'ul', 'var', 'video', 'wbr']
            });
        }        
    })
}(window.jQuery);

I have not had any luck making it work (making any changes continues to strip the attributes), and was about to open a new issue when I found this.

Setting allowed attributes could be a great addition for developers who would like to use Vue/Angular/React with October. Have there been any further leads on fixing the issue?

Edit: I tried switching the the default attributes on the octobercms/modules/backend/formwidgets/richeditor/assets/js/build-min.js file as well with no success.

LukeTowers commented 5 years ago

@tfprado it's on my TODO list to update Froala, so that might end up helping.

github-actions[bot] commented 5 years ago

This issue will be closed and archived in 3 days, as there has been no activity in the last 30 days. If this issue is still relevant or you would like to see action on it, please respond and we will get the ball rolling.

thomas4Bitcraft commented 4 years ago

I have found a way to get additional HTML-Tags working in the backend:

Create JS File:

$( document ).ready(function() {
    if ($.FroalaEditor) {
        $.FroalaEditor.DEFAULTS = $.extend($.FroalaEditor.DEFAULTS, {
            htmlAllowedTags: ['a', 'abbr', 'address', 'area', 'article', 'aside', 'audio', 'b', 'base', 'bdi', 'bdo', 'blockquote', 'br', 'button', 'canvas', 'caption', 'cite', 'code', 'col', 'colgroup', 'datalist', 'dd', 'del', 'details', 'dfn', 'dialog', 'div', 'dl', 'dt', 'em', 'embed', 'fieldset', 'figcaption', 'figure', 'footer', 'form', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'header', 'hgroup', 'hr', 'i', 'iframe', 'img', 'input', 'ins', 'kbd', 'keygen', 'label', 'legend', 'li', 'link', 'main', 'map', 'mark', 'menu', 'menuitem', 'meter', 'nav', 'noscript', 'object', 'ol', 'optgroup', 'option', 'output', 'p', 'param', 'pre', 'progress', 'queue', 'rp', 'rt', 'ruby', 's', 'samp', 'script', 'style', 'section', 'select', 'small', 'source', 'span', 'strike', 'strong', 'sub', 'summary', 'sup', 'table', 'tbody', 'td', 'textarea', 'tfoot', 'th', 'thead', 'time', 'title', 'tr', 'track', 'u', 'ul', 'var', 'video', 'wbr', 'i']
        });
    }
});

And add the file to october on boot:

public function boot()
    {
        // Check if we are currently in backend module.
        if (!App::runningInBackend()) {
            return;
        }

        // Listen for `backend.page.beforeDisplay` event and inject js to current controller instance.
        Event::listen('backend.page.beforeDisplay', function($controller, $action, $params) {
            $controller->addJs('/plugins/.../.../classes/editor.js');
        });
    }

github-actions[bot] commented 4 years ago

This issue will be closed and archived in 3 days, as there has been no activity in the last 30 days. If this issue is still relevant or you would like to see it actioned, please respond and we will re-open this issue. If this issue is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

michaeI commented 4 years ago

Thats a bummer, the bug still presists, I have added "i" tag to allowed list of empty tags but still is getting removed ... the workaround to it is to use   but it will break the alignment of your icon... Btw. what is the point of removing empty tags? is this kind of safety procedure? ;-)

bennothommo commented 4 years ago

@michaeI I believe Froala strips them because it assumes it cannot visually present empty tags (for example, a <strong></strong> tag with no content shows nothing on screen).

github-actions[bot] commented 4 years ago

This issue will be closed and archived in 3 days, as there has been no activity in the last 30 days. If this issue is still relevant or you would like to see it actioned, please respond and we will re-open this issue. If this issue is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

RomainMazB commented 4 years ago

Hi, Just to add an information, this issue exists since 2017 and I'm surprised nobody mentioned the allowedBlankTags option of Froala: https://froala.com/wysiwyg-editor/v1-2/docs/options/#allowedBlankTags

@michael To avoid this, you just need to create a custom script, target the RichEditor and call this method:

$('.rich-editor-selector').editable('option', 'allowedBlankTags', ['i']);

Hope this will help, maybe this issue is not an issue? :)

LukeTowers commented 4 years ago

@RomainMazB could you add support for that to the backend editor settings page?

RomainMazB commented 4 years ago

@LukeTowers sure! ~~Only for the i tag?~~

~~Or maybe the list provided by @thomas4Bitcraft here? https://github.com/octobercms/october/issues/3305#issuecomment-590124470~~

Oh, looking at the richeditor config I understand, you want me to add an options the user could provide from the yaml file to allow custom tags?

The same way we can provide other options

LukeTowers commented 4 years ago

Default to the i tag but it would be a settings field so anyone could add their own preferences to it.

LukeTowers commented 4 years ago

So yeah

RomainMazB commented 4 years ago

Ok. You can count on me.

LukeTowers commented 4 years ago

@michaeI @datune can you test #5126?

github-actions[bot] commented 4 years ago

This issue will be closed and archived in 3 days, as there has been no activity in the last 60 days. If this issue is still relevant or you would like to see it actioned, please respond and we will re-open this issue. If this issue is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

github-actions[bot] commented 4 years ago

This issue will be closed and archived in 3 days, as there has been no activity in the last 60 days. If this issue is still relevant or you would like to see it actioned, please respond and we will re-open this issue. If this issue is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

bennothommo commented 4 years ago

I assume this has been fixed by #5126, but let me know if this is not the case and I will reopen.

octobercms / october