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

Do we want important features dependent on shortcodes feature? #7715

Closed
davidhayden opened this issue Nov 22, 2020 · 18 comments · Fixed by sebastienros/shortcodes#13
Closed
Milestone

Comments

@davidhayden
Copy link
Contributor

I decided to disable the shortcodes feature on a client website because I wanted to see if it was responsible for mangling sample code in a knowledge base that included brackets ("[" and "]"), only to realize that disabling shortcodes meant I had to disable the Content Fields and HTML features that have a dependency on it.

Is there a reason we have a hard dependency on the nice-to-have shortcodes feature in other key CMS features? Can't we instead enable features in those modules when and if the shortcodes feature is enabled? Either way, I don't believe I have an option to opt-out like I do with the HTML sanitizer.

shortcodes-dependency

@Skrypt
Copy link
Contributor

Skrypt commented Nov 22, 2020

I think the best approach to this would be to add something like a <code>[ something ]</code> that would escape the shortcodes to try to render.

See : https://github.com/sebastienros/shortcodes#escaping-tags

@davidhayden
Copy link
Contributor Author

If it ignored code blocks that would certainly help.

I think the problem is bigger though.

I should be able to disable the shortcodes feature without disabling all my CMS editors. If I disable shortcodes, I disable all Content Fields, HtmlBodyPart, MarkdownPart, and possibly other important features.

I should be able to opt-out of shortcode processing in a field, like HTML Sanitizing. This is how I fixed it for my client really quickly, but I think this is just a bandaid for the bigger problem above.

shortcode-settings

@deanmarcussen
Copy link
Member

For infos @davidhayden

The reason we (I) integrated shortcodes, as a non optional feature, was primarily because they provide a default behaviour to include media files into content.

i.e. without them you cannot use media from the media library inside any html content blocks.

To be fair, when we first implemented all we had was the [image] tag, which wouldn't mangle other [ ...

@davidhayden
Copy link
Contributor Author

Maybe something changed or I'm experiencing something strange @deanmarcussen , because attributes in all sample code are completely swallowed up by the shortcode processor. If I have an attribute in sample code in an HTML Field, like [HttpGet], the shortcode processor removes it completely from the code. In order to display it, I have to escape it as [[HttpGet]], which I shouldn't have to do.

It gets trickier when you have something like [Route("api\[controller]")], because this cannot be fixed as [[Route("api\[[controller]]")]]. It requires a different combination of brackets.

I temporarily added a setting, but it would be nice to have the shortcodes feature play nicely with code samples and other uses of brackets that are not shortcodes.

@deanmarcussen
Copy link
Member

I don't think anything has changed @davidhayden - was just explaining why it is not an optional feature.

But what might be a nice feature for @sebastienros shortcodes processor, is to return the value, if there is no shortcode registered against that name - not sure of the practicalities of that within the parser (particularly with the nested examples above)

@sebastienros
Copy link
Member

sebastienros commented Dec 10, 2020

I am thinking that we could configure the parser to ignore some sections, ideally configurable, like by stating that ` or ``` code blocks should not be parsed. Same with using <pre></pre> or <code></code> but that could be hard to manage.

Another option would be to use a shortcode to use raw content, like [raw]...[/raw]. (preferred option)
Final option would be to render the unknown ones as is, but that's also dangerous since it could display arguments.

@sebastienros sebastienros added this to the 1.0.x milestone Dec 10, 2020
@sebastienros
Copy link
Member

I assume the default behavior could also return the original text if no shortcode matches ...
I am also looking at how wordpress is handling this

@sebastienros
Copy link
Member

Wordpress seems to ignore shortcodes that are unknown or don't have the correct arguments.
Right now our behavior is the strip the shortcode. Should it be an option on the library?

@davidhayden
Copy link
Contributor Author

I think so. I thought it would be the default functionality and was surprised when it stripped the text.

@davidhayden
Copy link
Contributor Author

Awesome work @sebastienros and thank you 👍

I did some quick testing in an HTMLBodyPart to verify the change is not stripping unknown shortcodes, executing known shortcodes, and still escaping tags. This solves my concerns, and the issue can be closed from my perspective.

@Skrypt
Copy link
Contributor

Skrypt commented Dec 16, 2020

Did we document the final solution?

@hishamco
Copy link
Member

I upgrade Shortcodes to 1.0.0 in #7988 which allow to skip unknown shortcodes

@Skrypt
Copy link
Contributor

Skrypt commented Dec 16, 2020

So the solution is to use any unknown shortcode like :

<code><p>[escape] Html.Raw escaped text [known_shortcode][known_shortcode] [escape] hello world<p></code>

Please fix my example if I'm wrong and add it to the documentation.

@sebastienros
Copy link
Member

sebastienros commented Dec 16, 2020

which allow to skip unknown shortcodes

=> which skips unknown shortcodes

The solution is to not do anything in OC, the library has a different behavior that fixes this issue

@sebastienros
Copy link
Member

But we might still need to add a custom shortcode to escape everything on purpose, with [raw][/raw]. This can't be done with a custom shortcode since the inner text would be parsed first. It has to be done in the shortcodes library. I couldn't find how Wordpress is handling this though.

@Skrypt
Copy link
Contributor

Skrypt commented Dec 16, 2020

[[shortcode_here]TEXT HERE.[/shortcode_here]]

But I think this already works? I leave you guys at it. I just want to have this documented 😉

@sebastienros
Copy link
Member

Yes that worked, and it still works, but when you write a blog post and you need to add some c# code with attributes, you don't want to have to do it on every attruibute. Then you have arrays, ...

What should be documented? How to escape shortcodes?

@Skrypt
Copy link
Contributor

Skrypt commented Dec 16, 2020

I think escaping shortcodes is already documented. I meant documenting how to achieve what @davidhayden wanted to achieve. Maybe I need to read more about the issue and I'm lost but I feel like either the issue remains or that there is some examples that could be shown in the documentation.

@sebastienros sebastienros modified the milestones: 1.0.x, 1.0 Sep 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants