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

Runkit: can't place HTML tags in embed #14470

Open
SiddharthShyniben opened this issue Aug 11, 2021 · 8 comments
Open

Runkit: can't place HTML tags in embed #14470

SiddharthShyniben opened this issue Aug 11, 2021 · 8 comments
Labels
area: media images, videos, podcasts, liquid tags, etc. bug always open for contribution

Comments

@SiddharthShyniben
Copy link
Contributor

SiddharthShyniben commented Aug 11, 2021

Describe the bug

If you try to put any HTML tags in embeds:

const element = '<br>';

We get some really weird behavior. If you place a <br> (or any closing or self-closing tags), it just disappears:

Screen Shot 2021-08-11 at 8 56 45 AM

If you place an <h1>, or a <hr>:

{% runkit %}
// Some other code
foo();
bar();

const element = '<h1>';
{% endrunkit %}

The whole thing disappears:

Screen Shot 2021-08-11 at 8 58 10 AM

To Reproduce

  1. Create a post
  2. Create a RunKit embed with any HTML tags anywhere in it
  3. See weird behavior

Expected behavior

The tags are shown properly.

Screenshots

Screen Shot 2021-08-11 at 8 56 45 AM

Screen Shot 2021-08-11 at 8 58 10 AM

Desktop (please complete the following information):

  • OS, version: MacOS Sierra (10.12.6)
  • Browser, version: Chrome 92.0.4515.131 (Official Build) (x86_64)

Smartphone (please complete the following information):

  • Device: Vivo V2029
  • OS, version: FunTouch OS PD2034BF_EX_A_1.70.17 (Basically android)
  • Browser, version: Chrome 92.0.4515.131

Additional context

NA

@github-actions
Copy link
Contributor

Thanks for the issue, we will take it into consideration! Our team of engineers is busy working on many types of features, please give us time to get back to you.

Feature requests that require more discussion may be closed. Read more about our feature request process on forem.dev.

To our amazing contributors: issues labeled type: bug are always up for grabs, but for feature requests, please wait until we add a ready for dev before starting to work on it.

To claim an issue to work on, please leave a comment. If you've claimed the issue and need help, please ping @forem/oss. The OSS Community Manager or the engineers on OSS rotation will follow up.

For full info on how to contribute, please check out our contributors guide.

@SiddharthShyniben
Copy link
Contributor Author

Even weirder: If you place a runkit embed with a <br> after an embed with an <hr> (demo):

{% runkit %}
// Some other code
foo();
bar();

const element = '<h1>';
{% endrunkit %}

{% runkit %}
const element = '<br>';
{% endrunkit %}

Then the first block doesn't disappear anymore

@citizen428 citizen428 added area: liquid tags bug always open for contribution bug smash Approved bugs for the DEV community bug smash labels Aug 11, 2021
@squarebat
Copy link
Contributor

I'll take it

@rhymes
Copy link
Contributor

rhymes commented Aug 20, 2021

@squarebat all yours, thanks!

@squarebat squarebat removed their assignment Aug 28, 2021
@squarebat
Copy link
Contributor

unassigned myself since I won't be able to work on this for two weeks. Seeing that it is a part of bug smash, I want to leave it open to other contributors. Happy debugging!

@vishaldeepak
Copy link
Contributor

vishaldeepak commented Sep 23, 2021

Need a bit of help form the Forem Team on this.
Looks like defining html tags in Markdown actually converts them in readable tags on browser. Is this desired behaviour?
Screenshot 2021-09-23 at 11 37 45 AM
Screenshot 2021-09-23 at 11 37 34 AM

Looks like the problem/related code is somewhere here in the parser.rb file.

html = markdown.render(escaped_content)
sanitized_content = sanitize_rendered_markdown(html)

@citizen428
Copy link
Contributor

Looks like defining html tags in Markdown actually converts them in readable tags on browser

This is part of how Markdown works, see https://daringfireball.net/projects/markdown/syntax#html.

@vishaldeepak
Copy link
Contributor

vishaldeepak commented Sep 25, 2021

The problem is around runkit_tag.rb:76

    content = Nokogiri::HTML.parse(super)
    parsed_content = content.xpath("//html/body").text

The second line of code strips all tags. The first idea would to fix this line, but the problem is in earlier code parser.rb-finalize():19, adds other tags via various sanitize and markdown processing. For example this text entered in edit Post

this is a runkit
{% runkit %}
   x='<h1>';
   y='<h2>';
{% endrunkit %}

Some more code

Would create the final output in the runkit_tag.rb:76 as follows

 "<br>\n   x='</p><h1>';<br>\n   y='<h2>';<br>\n"

Now we do not know which tags to remove and which to keep.

Proposed solution

Add codeticks inside between runkit and endrunkit so that they are processed as code. A simplified solution would look like this

content.gsub!(/{% runkit %}/m, "{% runkit %}\n```\n")
content.gsub!(/{% endrunkit %}/m, "```\n{% endrunkit %}")

This would force the processing of Redcarpet to treat it as block of code in html_rouge.rb:11

@cmgorton cmgorton added area: media images, videos, podcasts, liquid tags, etc. and removed area: liquid tags bug smash Approved bugs for the DEV community bug smash labels Nov 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: media images, videos, podcasts, liquid tags, etc. bug always open for contribution
Projects
None yet
Development

No branches or pull requests

6 participants