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

fix(#4460): refactor post escape #4472

Merged
merged 1 commit into from
Aug 14, 2020
Merged

fix(#4460): refactor post escape #4472

merged 1 commit into from
Aug 14, 2020

Conversation

SukkaW
Copy link
Member

@SukkaW SukkaW commented Aug 11, 2020

What does it do?

Basically the PR is reverting #4254.

Given a code block like this:

```html
<body>
  <!-- here goes the rest of the page -->
</body>
```

backtick_code_block will use prismjs to convert the code block into html:

<!--hexoPostRenderEscape:<pre class="line-numbers language-html" data-language="html"><code class="language-html"><span class="token tag"><span class="token tag"><span class="token punctuation">&lt;</span>body</span><span class="token punctuation">></span></span>
<span class="token comment">&lt;!-- here goes the rest of the page --></span>
<span class="token tag"><span class="token tag"><span class="token punctuation">&lt;/</span>body</span><span class="token punctuation">></span></span><span aria-hidden="true" class="line-numbers-rows"><span></span><span></span><span></span></span></code></pre>:hexoPostRenderEscape-->

<!--hexoPostRenderEscape: & :hexoPostRenderEscape--> is added by Hexo to avoid missing paragraph issue.

Why they are needed, what is "missing paragraph" issue: #4171 (comment)

However, instead of ignoring the comment, marked.js.org will add extra <p> tag:

<!--hexoPostRenderEscape:<pre class="line-numbers language-html" data-language="html"><code class="language-html"><span class="token tag"><span class="token tag"><span class="token punctuation">&lt;</span>body</span><span class="token punctuation">></span></span>
  <span class="token comment">&lt;!-- here goes the rest of the page --></span>
<p><span class="token tag"><span class="token tag"><span class="token punctuation">&lt;/</span>body</span><span class="token punctuation">&gt;</span></span><span aria-hidden="true" class="line-numbers-rows"><span></span><span></span><span></span></span></code></pre>:hexoPostRenderEscape--&gt;</p>

With extra <p> tag the code block is now broken and :hexoPostRenderEscape--&gt; can not be matched & removed.

Ref: marked.js demo: https://marked.js.org/demo/

So to prevent marked.js from breaking the html, the PR reverts the behavior introduced by #4254

The performance is not affected.

How to test

git clone -b fix-4460 https://github.com/sukkaw/hexo.git
cd hexo
npm install
npm test

Screenshots

Pull request tasks

  • Add test cases for the changes.
  • Passed the CI test.

cc @jerryc127.

@coveralls
Copy link

coveralls commented Aug 11, 2020

Coverage Status

Coverage increased (+0.0005%) to 98.295% when pulling 75acca6 on SukkaW:fix-4460 into b7d15b9 on hexojs:master.

@SukkaW
Copy link
Member Author

SukkaW commented Aug 11, 2020

@curbengh @stevenjoezhang

We should also suggret user stick to highlight.js, since prismjs is not completely safe and robust enough: PrismJS/prism#2516

@stevenjoezhang
Copy link
Member

Yes, the HTML escaping issue may destroy the structure of the entire page.

@curbengh
Copy link
Contributor

We should also suggret user stick to highlight.js, since prismjs is not completely safe and robust enough:

Syntax highlight still says highlight,js is the default. Configuration page hasn't mention prism yet.

Do you want to switch the priority?

// Since prismjs have better performance, so prismjs should have higher priority.
if (prismCfg.enable) {
if (!prismHighlight) prismHighlight = require('hexo-util').prismHighlight;
const options = {
lineNumber: prismCfg.line_number,
tab: prismCfg.tab_replace,
isPreprocess: prismCfg.preprocess,
lang
};
content = prismHighlight(content, options);

@jerryc127
Copy link

jerryc127 commented Aug 12, 2020

如果书写codeblock在列表下,会同时出现‘<!–hexoPostRenderEscape:’ 和‘:hexoPostRenderEscape–>’

1.在md里,列表 与 代码块没有空行,会渲染出错

image

  1. 在md里,列表 与 代码块空行,但是代码块前面有空格,会渲染出错

image

  1. 在md里,列表 与 代码块空行,代码块前面没有空格,渲染才正常

image

测试代码

1. 测试

```diff
+comments:
+  # Up to two comments system, the first will be shown as default
+  # Choose: Disqus/Disqusjs/Livere/Gitalk/Valine/Utterances/Facebook Comments
+  use:
+  # - Valine
+  # - Disqus
+  text: true # Display the comment name next to the button
+  # lazyload: The comment system will be load when comment element enters the browser's viewport.
+  # If you set it to false, the comment count will be invalid
+  lazyload: false
+  count: false # Display comment count in top_img

disqus:
-  enable: false
-  count: false # dispaly comment count in top_img

disqusjs:
-  enable: false
-  count: false # dispaly comment count in top_img

livere:
-  enable: false

gitalk:
-  enable: false
-  count: false # dispaly comment count in top_img

valine:
-  enable: false # if you want use valine,please set this value is true
-  count: false # dispaly comment count in top_img

utterances:
-  enable: false

facebook_comments:
-  enable: false
-  count: false
```

@curbengh
Copy link
Contributor

@jerryc127

May I have your prismjs config? I only tested with:

# _config.yml
prismjs:
  enable: true

Have you tested this PR? $ npm install hexo@SukkaW/hexo#fix-4460

@jerryc127
Copy link

@jerryc127

May I have your prismjs config? I only tested with:

# _config.yml
prismjs:
  enable: true

Have you tested this PR? $ npm install hexo@SukkaW/hexo#fix-4460

I test the pr. it solved.
but i find the problem still appear in Tag Plugins.
It hasn't show the ‘<!–hexoPostRenderEscape:’ and ‘:hexoPostRenderEscape–>’, still cannot render correct

  1. In markdown, use the Tag Plugins, such as 'pullquote', there is no blank line in the list and code block, it will rendering errors
  2. In markdown, use the Tag Plugins, such as 'pullquote', the list and code block have blank lines, but there are spaces in front of the code block, it will rendering errors

image

{% pullquote %}

1. Test Test
```diff
+comments:
+  # Up to two comments system, the first will be shown as default
+  # Choose: Disqus/Disqusjs/Livere/Gitalk/Valine/Utterances/Facebook Comments
+  use:
+  # - Valine
+  # - Disqus
+  text: true # Display the comment name next to the button
+  # lazyload: The comment system will be load when comment element enters the browser's viewport.
+  # If you set it to false, the comment count will be invalid
+  lazyload: false
+  count: false # Display comment count in top_img

disqus:
-  enable: false
-  count: false # dispaly comment count in top_img

disqusjs:
-  enable: false
-  count: false # dispaly comment count in top_img

livere:
-  enable: false

gitalk:
-  enable: false
-  count: false # dispaly comment count in top_img

valine:
-  enable: false # if you want use valine,please set this value is true
-  count: false # dispaly comment count in top_img

utterances:
-  enable: false

facebook_comments:
-  enable: false
-  count: false
```

{% endpullquote %}
```

the setting

highlight:
  enable: false
  line_number: true
  auto_detect: false
  tab_replace: ''
  wrap: true
  hljs: false
prismjs:
  enable: true
  preprocess: true
  line_number: true
  tab_replace: ''

@SukkaW
Copy link
Member Author

SukkaW commented Aug 13, 2020

Do you want to switch the priority?

@curbengh No worry, only prismjs.enable = true will let prismjs has higher priority.

@SukkaW
Copy link
Member Author

SukkaW commented Aug 13, 2020

@stevenjoezhang @curbengh

Since the prismjs escaping has been fixed, the PR is ready for review now.

@curbengh
Copy link
Contributor

but i find the problem still appear in Tag Plugins.

Rendered fine for me. I noticed you had an extra triple backtick at the last line.

@jerryc127
Copy link

但我發現問題仍然出現在標籤插件中。

對我來說很好。我注意到您在最後一行有一個額外的三重奏。

umm...
i try the landscape theme
it still cannot render correct
i dont konw why

image

image

@SukkaW
Copy link
Member Author

SukkaW commented Aug 13, 2020

@jerryc127 Would you mind update your status at #4460 instead? The PR is mainly focused on fixing hexoPostRenderEscape being shown.

Copy link
Contributor

@curbengh curbengh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WFM

@SukkaW SukkaW merged commit 463025c into hexojs:master Aug 14, 2020
@curbengh curbengh mentioned this pull request Aug 14, 2020
5 tasks
@curbengh curbengh mentioned this pull request Aug 21, 2020
2 tasks
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 this pull request may close these issues.

5 participants