Don't render escaped HTML on plain text #21841

Gusted · 2022-11-16T22:18:13Z

While viewing a plain readme file that has ambiguous characters, it currently will write HTML for escaped characters, however it's being rendered as a plain text and thus will not actually render the HTML elements.
Only show the escape status, but render the content as plain text without trying to show the ambigious characters(escape button will have no effect due to this).
Related https://codeberg.org/Codeberg/Community/issues/796

Before:

After:

- While viewing a plain readme file that has ambiguous characters, it currently will write HTML for escaped characters, however it's being rendered as a plain text and thus will not actually render the HTML elements. - Only show the escape status, but render the content as plain text without trying to show the ambigious characters(escape button will have no effect due to this). - Related https://codeberg.org/Codeberg/Community/issues/796

Gusted · 2022-11-16T22:20:40Z

@zeripath, this feels hacky, but trying to render the plain text as HTML feels even more hacky.

zeripath · 2022-11-16T22:28:15Z

Yup this PR doesn't feel right to me.

Gusted · 2022-11-17T11:13:01Z

What should be the expected behavior be, indeed render the HTML? Or disable this feature altogether for plain READMEs?

zeripath · 2022-11-22T21:38:11Z

So there are two things we need to do:

In reality we should drop the ambiguous warnings for rendered content
- We should be like VSCode here and github and only warn about these on Source code views
We still need to do something about rendering README.txt files though, because we have to handle broken characters.
- I think these should just be rendered as <pre> and we can stick the <code> bits in as necessary.

silverwind · 2022-11-27T12:42:23Z

Related: I would suggest making this box yellow instead of red. Also, I think we should reduce text to just one paragraph, those two are nearly identical. And finally, I still feel the warning shows up too often on innocuous code.

wxiaoguang · 2022-11-27T13:04:02Z

Related: I would suggest making this box yellow instead of red. Also, I think we should reduce text to just one paragraph, those two are nearly identical. And finally, I still feel the warning shows up too often on innocuous code.

Agree, many warnings do not make sense:

Some common characters should not be treaded as ambiguous Unicode characters #20999

Gusted · 2022-12-01T19:05:03Z

So there are two things we need to do:

1. In reality we should drop the ambiguous warnings for rendered content
   
   * We should be like VSCode here and github and only warn about these on Source code views

2. We still need to do something about rendering README.txt files though, because we have to handle broken characters.
   
   * I think these should just be rendered as  and we can stick the ```
      bits in as necessary.
     ````
   ``
   `

` `

So still stick in the html elements? But don't display them? I'm sorry I'm not following on your second point.

silverwind

Better than before

zeripath

I think this is still the wrong thing to do - instead we should just render the warning for rendered pages.

zeripath · 2022-12-03T14:46:08Z

See #22017

…mbiguous characters As recognised in go-gitea#21841 the rendering of plain text files is somewhat incorrect when there are ambiguous characters as the html code is double escaped. In fact there are several more problems here. We have a residual isRenderedHTML which is actually simply escaping the file - not rendering it. This is badly named and gives the wrong impression. There is also unusual behaviour whether the file is called a Readme or not and there is no way to get to the source code if the file is called README. In reality what should happen is different depending on whether the file is being rendered a README at the bottom of the directory view or not. 1. If it is rendered as a README on a directory - it should simply be escaped and rendered as `<pre>` text. 2. If it is rendered as a file then it should be rendered as source code. This PR therefore does: 1. Rename IsRenderedHTML to IsRenderedPlainText 2. Readme files rendered at the bottom of the directory are rendered without line numbers 3. Otherwise plain text files are rendered as source code. Replace go-gitea#21841 Signed-off-by: Andrew Thornton <art27@cantab.net>

Gusted · 2022-12-04T13:06:36Z

Seems to be replaced by #22017

zeripath · 2022-12-04T14:10:49Z

Thanks @Gusted for your work

…mbiguous characters (#22017) As recognised in #21841 the rendering of plain text files is somewhat incorrect when there are ambiguous characters as the html code is double escaped. In fact there are several more problems here. We have a residual isRenderedHTML which is actually simply escaping the file - not rendering it. This is badly named and gives the wrong impression. There is also unusual behaviour whether the file is called a Readme or not and there is no way to get to the source code if the file is called README. In reality what should happen is different depending on whether the file is being rendered a README at the bottom of the directory view or not. 1. If it is rendered as a README on a directory - it should simply be escaped and rendered as `<pre>` text. 2. If it is rendered as a file then it should be rendered as source code. This PR therefore does: 1. Rename IsRenderedHTML to IsPlainText 2. Readme files rendered at the bottom of the directory are rendered without line numbers 3. Otherwise plain text files are rendered as source code. Replace #21841 Signed-off-by: Andrew Thornton <art27@cantab.net> Signed-off-by: Andrew Thornton <art27@cantab.net> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

…mbiguous characters (go-gitea#22017) Backport go-gitea#22017 As recognised in go-gitea#21841 the rendering of plain text files is somewhat incorrect when there are ambiguous characters as the html code is double escaped. In fact there are several more problems here. We have a residual isRenderedHTML which is actually simply escaping the file - not rendering it. This is badly named and gives the wrong impression. There is also unusual behaviour whether the file is called a Readme or not and there is no way to get to the source code if the file is called README. In reality what should happen is different depending on whether the file is being rendered a README at the bottom of the directory view or not. 1. If it is rendered as a README on a directory - it should simply be escaped and rendered as `<pre>` text. 2. If it is rendered as a file then it should be rendered as source code. This PR therefore does: 1. Rename IsRenderedHTML to IsPlainText 2. Readme files rendered at the bottom of the directory are rendered without line numbers 3. Otherwise plain text files are rendered as source code. Replace go-gitea#21841 Signed-off-by: Andrew Thornton <art27@cantab.net> Signed-off-by: Andrew Thornton <art27@cantab.net> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

…mbiguous characters (#22017) (#22160) Backport #22017 As recognised in #21841 the rendering of plain text files is somewhat incorrect when there are ambiguous characters as the html code is double escaped. In fact there are several more problems here. We have a residual isRenderedHTML which is actually simply escaping the file - not rendering it. This is badly named and gives the wrong impression. There is also unusual behaviour whether the file is called a Readme or not and there is no way to get to the source code if the file is called README. In reality what should happen is different depending on whether the file is being rendered a README at the bottom of the directory view or not. 1. If it is rendered as a README on a directory - it should simply be escaped and rendered as `<pre>` text. 2. If it is rendered as a file then it should be rendered as source code. This PR therefore does: 1. Rename IsRenderedHTML to IsPlainText 2. Readme files rendered at the bottom of the directory are rendered without line numbers 3. Otherwise plain text files are rendered as source code. Replace #21841 Signed-off-by: Andrew Thornton <art27@cantab.net> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 16, 2022

silverwind approved these changes Dec 1, 2022

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Dec 1, 2022

lafriks approved these changes Dec 3, 2022

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Dec 3, 2022

Merge branch 'main' into dont-render-escape-plain

915b734

zeripath suggested changes Dec 3, 2022

View reviewed changes

zeripath mentioned this pull request Dec 3, 2022

Ensure that plain files are rendered correctly even when containing ambiguous characters #22017

Merged

Gusted closed this Dec 4, 2022

zeripath mentioned this pull request Dec 18, 2022

Ensure that plain files are rendered correctly even when containing ambiguous characters (#22017) #22160

Merged

Gusted deleted the dont-render-escape-plain branch December 20, 2022 21:51

go-gitea locked and limited conversation to collaborators May 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't render escaped HTML on plain text #21841

Don't render escaped HTML on plain text #21841

Gusted commented Nov 16, 2022 •

edited

Loading

Gusted commented Nov 16, 2022

zeripath commented Nov 16, 2022

Gusted commented Nov 17, 2022

zeripath commented Nov 22, 2022 •

edited

Loading

silverwind commented Nov 27, 2022 •

edited

Loading

wxiaoguang commented Nov 27, 2022

Gusted commented Dec 1, 2022

silverwind left a comment

zeripath left a comment

zeripath commented Dec 3, 2022 •

edited

Loading

Gusted commented Dec 4, 2022

zeripath commented Dec 4, 2022

Don't render escaped HTML on plain text #21841

Don't render escaped HTML on plain text #21841

Conversation

Gusted commented Nov 16, 2022 • edited Loading

Gusted commented Nov 16, 2022

zeripath commented Nov 16, 2022

Gusted commented Nov 17, 2022

zeripath commented Nov 22, 2022 • edited Loading

silverwind commented Nov 27, 2022 • edited Loading

wxiaoguang commented Nov 27, 2022

Gusted commented Dec 1, 2022

silverwind left a comment

Choose a reason for hiding this comment

zeripath left a comment

Choose a reason for hiding this comment

zeripath commented Dec 3, 2022 • edited Loading

Gusted commented Dec 4, 2022

zeripath commented Dec 4, 2022

Gusted commented Nov 16, 2022 •

edited

Loading

zeripath commented Nov 22, 2022 •

edited

Loading

silverwind commented Nov 27, 2022 •

edited

Loading

zeripath commented Dec 3, 2022 •

edited

Loading