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 escaped html characters in search result #1501

Merged

Conversation

wxwxwxwx9
Copy link
Contributor

@wxwxwxwx9 wxwxwxwx9 commented Mar 4, 2021

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • Others, please explain:

Resolves #1486.

Overview of changes:
Used v-html to render unescaped html characters (in place of the original mustache expression).

Anything you'd like to highlight / discuss:
NIL

Testing instructions:
npm run test

Serve the user guide and search for "include" keyword.

&lt; and &gt; that originally appears should now be replaced by < and > respectively, since the characters are no longer escaped.

Proposed commit message: (wrap lines at 72 characters)
Fix escaped html characters in search result.

Certain html characters are automatically escaped for
security reasons. However, it is not needed in our context
as the html is supplied by the user.

Let's prevent html characters from getting escaped so as
to render the html correctly.


Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No blatantly unrelated changes
  • Pinged someone for a review!

@jonahtanjz
Copy link
Contributor

Thanks @wxwxwxwx9 for fixing this :) I've tested it and it seems to be working well 👍

On a separate note, since in our context usage of v-html doesn't cause any security threat, I was thinking that we should disable the eslint rule for no-v-html so that we can resolve all the eslint warnings during the ci checks but that can be a separate discussion/issue. What do you think about this?

@wxwxwxwx9
Copy link
Contributor Author

@jonahtanjz thanks for the review!

On a separate note, since in our context usage of v-html doesn't cause any security threat, I was thinking that we should disable the eslint rule for no-v-html so that we can resolve all the eslint warnings during the ci checks but that can be a separate discussion/issue. What do you think about this?

Yup! I agree with you. We can do that in another PR :-)

Copy link
Contributor

@ang-zeyu ang-zeyu left a comment

Choose a reason for hiding this comment

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

Lgtm 👍

On a separate note, since in our context usage of v-html doesn't cause any security threat, I was thinking that we should disable the eslint rule for no-v-html so that we can resolve all the eslint warnings during the ci checks but that can be a separate discussion/issue. What do you think about this?

Yup! I agree with you. We can do that in another PR :-)

It's a good rule to have in case anything slips by, I think, even if it means spewing these out constantly.

We could deal with these carefully on a case by case basis though. (eslint-disable-next-line)

@ang-zeyu ang-zeyu added this to the v3.0 milestone Mar 6, 2021
@ang-zeyu ang-zeyu merged commit 053353c into MarkBind:master Mar 6, 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 this pull request may close these issues.

Special characters not rendered correctly in search results
3 participants