Add text dashboard toggle for enabling markdown

[chihuahua]

Added a checkbox to the text dashboard that lets the user control
whether to enable markdown view of text widgets. The
markdownEnabled boolean property is stored within local storage.
The property defaults to true.

If markdown is enabled, we parse text as markdown and render it.
This matches current behavior.

If markdown is disabled, text is rendered raw, and line numbers are
provided.

Fixes #830.

[nfelt]

Hmm, this sort of addresses #830 but I am not sure it's really a complete solution. The logic here only escapes the HTML output from the markdown processing (which happens server-side), so it shows up as literal HTML text, tags and all. This does mean that if someone's generating text that might include HTML tags, this would keep some of those tags from getting interpreted (because various HTML tags like <b> pass through the markdown interpretation).

But it doesn't fix the issues caused by the markdown interpretation itself inserting new tags. To illustrate this, we can look at three levels of text interpretation:

1. Some text.
2. Some <em>text</em>.
3. Some *text*.

This PR gets us from 1 to 2, but I think the real goal should be getting to 3, which shows the actual raw text summary output. Note that this is harder though, since the markdown interpretation is done on the server and can't just be skipped from the browser without plumbing through a toggle.

I actually wonder whether it might be better to just move the markdown rendering to the browser-side. It makes the sanitization story a little simpler since then we don't have to just "trust" output from the server to be sanitized; we can ensure that it's sanitized as late as possible before insertion into the DOM. But we might have trouble finding a sufficiently good sanitizer. The markdown element that comes with polymer uses marked.js but the sanitization logic it uses seems fairly XSS-prone.

[jart]

One question I would ask is why not have a markdown boolean on the text summary metadata? That might be a better way to solve the problem, but I'm not sure if it's something we have the time to do right now.

[chihuahua]

Thanks for the catch! Indeed, this PR currently just renders the HTML instead of the original text.

A boolean on the text summary metadata seems to work! We might also want to let the user adjust later, but it seems like in many cases, the user's intent to use markdown is clear at the point of writing the summary.

[jart]

If the user knows its markdown at summary time, then I like the idea of having a more minimal UI.

[jart]

But with that said, this has already been written, so I'm open to either merging or closing. I'll leave it to you guys.

[chihuahua]

Actually yeah, I also like the idea of specifying whether to display as markdown when the summary is written, especially since folks know when the summary is being written.

Closing this PR. I can take on that task at some point once other priorities are addressed. Or someone else can take it on. This shouldn't be too big of a feature - some quick backend and frontend changes.

[nfelt]

I think the issue with using a summary-time flag is just backwards compatibility. Users will already have summary data written out that's both supposed to be markdown and supposed to not be markdown. Right now it all gets interpreted as markdown, annoying the not-supposed-to-be-markdown folks, but if we change the default behavior (in the absence of an explicit summary flag) it would probably annoy the supposed-to-be-markdown folks.

The only way to make TensorBoard work reliably for existing text summary data (or for that matter, for code using existing libraries/helper modules that use text summaries with no flag) would be adding a toggle.

I think a flag at the summary writing time would still make sense, but perhaps more as a default position for the toggle. FWIW, having a toggle would also be useful to people who are trying to write markdown but where the markdown was misrendered, so that they can see what the actual raw output was.

[chihuahua]

Come to think, that makes sense. In many cases, a user might error-correct later and seek to turn off markdown. Or debug misrendered markdown.

That shouldn't too big of a change either. :) We add a backend route for raw markdown. And then add the frontend UI widgets for toggling the feature.

[nfelt]

What do you think of doing the markdown rendering client-side? Then we wouldn't need a separate backend route to toggle display on and off. (I also feel like it makes it a bit easier to reason about the XSS risks if the data is treated as untrusted until the very moment it's injected into the DOM, rather than having it get sanitized at the backend and then the frontend just treats it as trusted, but we'd need to find a JS HTML sanitizer or combined Markdown parser+sanitizer that we're confident in.)

[chihuahua]

That would save requests to the backend! I think you or @jart separately noted reservations about how logic for the marked-element component sanitized markdown, right? Using that component might work as a quick first pass. Several projects throughout Google use it.