Translate markdown with angle brackets to HTML (fixes #476) #497
Conversation
There was a problem hiding this comment.
Hi @jcads, thank you for working on this issue. This solution looks like a great start, I have some input below.
I believe the cause of this bug is since < and > are reserved characters in HTML, the browser mistook <provider-name> as an HTML tag. To fix this, we could use character entities to display these special characters instead.
For example, if we replace the special characters with their equivalent HTML entities, such as:
htmlText = text.replace("<", "<");
and then pass it to parse(htmlText), the browser would then be able to read "<".
Wondering what @wlach thinks about this approach. If this seems reasonable, should we also take care of the other edge cases, such as &?
src/components/Markdown.svelte
Outdated
| {#if text.includes('<')} | ||
| {#if inline} | ||
| {text} | ||
| {:else} | ||
| {#each lines as text} | ||
| <p style="margin: 0">{text}</p> | ||
| {/each} | ||
| {/if} |
There was a problem hiding this comment.
We should always use parse/parseInline to render text in this component as the goal is to display markdown content.
There was a problem hiding this comment.
Got it @Iinh. Thank you.
There was a problem hiding this comment.
Hi @jcads -- thanks for the PR I think this solution is not quite right. As @Iinh, we want to continue using parse/parseInline to render the text.
What's going on here is the markdown renderer is creating <provider-name> as an HTML element. We can confirm this by looking at the developer console:
I think a shorter/cleaner solution would be to enhance the renderer object to escape these objects.
https://marked.js.org/using_pro#renderer
I think you might need to add a render for tag types, but am not 100% sure. Give it a try and see!
wlach
changed the title
Thanks @Iinh! This is actually a better approach than what I suggested. I think the markdown parser will automatically handle characters like |
This fixes pagination when navigating through long lists of metrics (the descriptions weren't being updated after #497)
This fixes pagination when navigating through long lists of metrics (the descriptions weren't being updated after #497)
PR #497 fixed some cases, but not all. Angle brackets in code blocks have their own escaping behaviour, which doesn't work well with the adhoc work we're doing. Strictly a string like this: <provider> is considered HTML in markdown. However this *probably* isn't how we want to interpret it in the Glean Dictionary. We'll address this by modifying the renderer when it encounters an HTML tag, rather than doing a naive regex replace against the whole markdown string.
This is pedantic, but strictly something called <provider-name> is considered an HTML tag unless it's in a code block (backticks). See mozilla/glean-dictionary#549 and mozilla/glean-dictionary#497. I'm going to fix this upstream but figured I might as well file a PR here to fix the underlying issue.
PR #497 fixed some cases, but not all. Angle brackets in code blocks have their own escaping behaviour, which doesn't work well with the adhoc work we're doing. Strictly a string like this: <provider> is considered HTML in markdown. However this *probably* isn't how we want to interpret it in the Glean Dictionary. We'll address this by modifying the renderer when it encounters an HTML tag, rather than doing a naive regex replace against the whole markdown string.
This is pedantic, but strictly something called <provider-name> is considered an HTML tag unless it's in a code block (backticks). See mozilla/glean-dictionary#549 and mozilla/glean-dictionary#497. I'm going to fix this upstream but figured I might as well file a PR here to fix the underlying issue.
…9243) This is pedantic, but strictly something called <provider-name> is considered an HTML tag unless it's in a code block (backticks). See mozilla/glean-dictionary#549 and mozilla/glean-dictionary#497. I'm going to fix this upstream but figured I might as well file a PR here to fix the underlying issue.
https://deploy-preview-497--glean-dictionary-dev.netlify.app/apps/fenix/metrics/browser_search_with_ads
Fixes #476.
Pull Request checklist
fixes, if applicable)