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

[Bug]: Tag(s) added after page load do not render #5

Closed
MarketingPip opened this issue May 24, 2022 · 8 comments · May be fixed by #7
Closed

[Bug]: Tag(s) added after page load do not render #5

MarketingPip opened this issue May 24, 2022 · 8 comments · May be fixed by #7
Labels
bug Something isn't working enhancement New feature or request help wanted Extra attention is needed

Comments

@MarketingPip
Copy link
Member

MarketingPip commented May 24, 2022

Contact Details

No response

What happened?

When inserting a tag into a page after page load - the other tags do not render.

Reported first - Is there a way to get an existing element to render new markdown that wasn't initially there? · Issue #3

What type of browser are you seeing the problem on?

Firefox, Chrome, Safari, Microsoft Edge

Code to produce this issue.

<p id="addElement">Add A Markdown Tag</button>
<md># Hello</md>
    <input type="text" name="enter" class="enter" value="<md> # Hello World</md>" id="AddMDTag"/>
    <input type="button" value="click" onclick="addElement();"/>
</body>

<script src="https://cdn.jsdelivr.net/gh/MarketingPipeline/Markdown-Tag/markdown-tag.js"></script> 


<script type="text/javascript">function addElement()
{
InputField = document.getElementById('AddMDTag');
    var Paragraph = document.getElementById('addElement');
    Paragraph.innerHTML += InputField.value
}</script>
@tomyo
Copy link

tomyo commented May 24, 2022

Hi! I also run into this issue playing around.

One hypothetical way to solve this could be:

  1. Accept children in a <slot> and listen to the slotchange event.
  2. re-run the converter when that happens.

To apply the mentioned idea, registering the component as a customElement would be needed.

I mention that because having a quick look at the code I was surprised to see that the <github-md> custom element is not registered as one. Why not?

@MarketingPip
Copy link
Member Author

tomyo

To be frankly honest - I was looking for a solution to add Markdown support to another repo under MarketingPipeline. I thought instead of wasting time adding Markdown to future websites I might want markdown support on it'd be better to take an approach at trying to solve a way to just add it whenever needed.

Myself (being a noob in JavaScript) - grabbed a parser. Added a tag with no impression it would work - but I figured, I could try right? And it did - so I just rolled with it.

I thought it was a more than cool & useful concept - and thought it might solve someone else's problem!

But - there is obviously lot's of room for improvement on this project hence released under the GPL-3.0 license so people with more experience like yourself can give input / feedback & support that benefits all the user's of Markdown-Tag

@DarkenLM
Copy link
Collaborator

Currently porting MarkdownTag to a WebComponent.
As of now, you can update the markdown on the fly instantly by modifying the HTML itself, or programatically using a setInput function.
My current focus is attempting to prevent XSS attacks, however my research has concluded that the only way to prevent it is programatically, as it is impossible to access the element before the DOM is rendered.

@MarketingPip
Copy link
Member Author

Currently porting MarkdownTag to a WebComponent. As of now, you can update the markdown on the fly instantly by modifying the HTML itself, or programatically using a setInput function. My current focus is attempting to prevent XSS attacks, however my research has concluded that the only way to prevent it is programatically, as it is impossible to access the element before the DOM is rendered.

This is out of my skill set as of right now - but dropping this here to help you.

ShowDown

       Showdown doesn't sanitize the input. This is by design since markdown relies on it to allow certain features to be correctly parsed into HTML. This, however, means XSS injection is quite possible.

Please refer to the wiki article Markdown's XSS Vulnerability (and how to mitigate it) for more information.

Marked

        Warning: Marked does not [sanitize](https://marked.js.org/#/USING_ADVANCED.md#options) the output HTML. Please use a sanitize library, like [DOMPurify](https://github.com/cure53/DOMPurify) (recommended), [sanitize-html](https://github.com/apostrophecms/sanitize-html) or [insane](https://github.com/bevacqua/insane) on the output HTML! rotating_light

The added discussion as well - I am hoping you can drop input into What would the preferred Markdown Tag be?

Tho based on your fork I think I know your input hahah! Tho would be great if we could get some more input from others before making the final decision.

Awesome work as well! ps; you have been invited to this repo if you didn't join already.

@DarkenLM
Copy link
Collaborator

I'm using DOMPurify to sanitize the HTML, and is called on every parser before generating the HTML from the markdown.
For the input, it is required by spec to have a hiphen, so I went ahead with md-tag.
And I already joined, yes, thank you for the invitation.

@tomyo
Copy link

tomyo commented Jul 7, 2022

I went with a very simple approach on my own, and called it mark-down, just to play with the idea mentioned above. Looks like that approach does works just fine! :)

Sharing the repo here, in case it can serve as inspiration, the same way this repo inspired me to do that one. Thanks!

@MarketingPip
Copy link
Member Author

MarketingPip commented Jul 7, 2022

@tomyo - we are actually in the process of updating with a web component if by chance you are looking for something to contribute too!

@MarketingPip
Copy link
Member Author

This issue has been marked as resolved.

To render changes / elements added call the function like so -

renderMarkdown()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants