[SVCS-418] Render MD Files at the Frontend #305

cslzchen · 2017-12-07T21:18:24Z

Ticket

https://openscience.atlassian.net/browse/SVCS-418
This PR replaces: #272

Purpose

Credit goes to @AddisonSchiller 🎆 🎆 .

Currently MFR and the OSF wiki pages do not use the same flavor of markdown to render .md files. This ticket/PR fixes the inconsistency.

Changes

Before (The MFR Way)

MFR uses python's markdown library to parse the .md file in backend and send the rendered HTML content directly to the template to render..

After (The OSF Way)

MFR passes the WB download URL to the template. The frontend uses XMLHttpRequest to fetch the raw content of the MD file. markdown-it and its plugins sanitize and render the raw MD content directly on the page.

Difference between OSF and MFR

Scripts in MFR are customized and ES5-downgraded due to no package manager and no Babel.

Side Effects

No

Issues

No

QA Notes

Try several MD files and compare the rendering with OSF Wiki. They should looks the same (mostly).

@cslzchen Create a staging OSF folder containing a variety of MD files for tests

Deployment Notes

@felliott Please update this confluence page

cslzchen

For Myself:

Before this PR, MFR relies on the python library markdown. After this PR, MFR delegates the rendering to front-end using javascript library markdown-it (similar to OSF Wiki). However, please elaborate more on the difference between OSF Wiki and MFR Markdown
Add comment in each external javascript file: where is the original copy? what is the version? what is our customization? ...
Fix style (using eslint and learn from our front-end team)
Make sure that the scripts are ES5 compatible. MFR is different from OSF and Ember since the scripts in our code base go directly to the server.

cslzchen · 2017-12-07T21:29:51Z

mfr/extensions/md/static/js/markdown-it-sanitizer.min.js

@@ -0,0 +1,2 @@
+/*! markdown-it-sanitizer 0.4.3 https://github.com/svbergerem/markdown-it-sanitizer @license MIT */


cslzchen

Ready for Phase 2 Review. @felliott (But let me update the PR description first 🎆 )

At lease one of the following is necessary:

Comprehensive test .md files for QA and Dev testing
Comprehensive python tests, which is more challenging since rendering is performed by javascript.

The travis error is unrelated. Somehow the test.zip contains a .DS_Store for .zip renderer. A separate PR to fix this is recommended.

cslzchen · 2017-12-08T19:16:23Z

tests/extensions/md/test_renderer.py

+        in_body = 'The rain---not the reign---in\nSpain.\n\n&lt;script&gt;\nalert("Hello world");\n&lt;/script&gt;'
+        assert in_body in body
+
+    # TODO: it is hard to test since rendering is performed by front-end javascript, if possible test the following:


It is hard to test front-end based rending for .md and there a lot to test. I will try to add more tests before/during phase 2 review if we think it is necessary.

Not sure how relevant this is, but I mostly just tried to test bleach quickly here. It may be worth while to make sure proper tags are being bleached, and javascript would get escaped out etc. The above is already testing for that a little.
Might be worth a parameterized test to test all the BLEACH_WHITELIST tags or whatever that setting got called.

cslzchen · 2017-12-08T19:21:05Z

.eslintrc.js

@@ -0,0 +1,18 @@
+module.exports = {


Add a basic configuration for eslint.

cslzchen · 2017-12-08T19:22:15Z

.gitignore

@@ -11,19 +11,29 @@ Thumbs.db
 *.swp
 *~

+# Node Modules
+######################
+/node_modules/


Update .gitignore to exclude ./node_modules/ (here) and ./src/ (below).

cslzchen · 2017-12-08T19:25:33Z

mfr/extensions/md/render.py


    def render(self):
        """Render a markdown file to html."""
        with open(self.file_path, 'r') as fp:
-            body = markdown.markdown(fp.read(), extensions=[EscapeHtml()])
+            body = bleach.clean(fp.read(), **md_settings.BLEACH_WHITELIST)


Although MFR uses bleach the same way OSF does, the library is not exactly the same due to python version difference.

Not sure if it got noted somewhere else, but more specifically it causes problems with unclosed tags.

Mark down is great! <p> okay </p other stuff more stuff

will usually just cut off the last 2 lines and end at at the 'okay' in a completed <p> tag

I see. My test didn't have the extra lines which made me believe that python35's bleach closes unclosed tags correctly. I will test this again. Thanks for the follow up @AddisonSchiller

I can't remember the exact behavior, but I do recall it working correctly sometimes with unclosed tags, and other times displaying the above behavior. May need a bit more investigation to figure out the full extent of it. Bleach also had some github issues that explained the behavior as well. They may be linked on an old comment somewhere, or on the old PR. May be worth checking those out.

felliott

Hey @cslzchen,

Two things for this pass:

The test_zip failure is because bleach pulls in html5lib, which wasn't installed before. BeautifulSoup (used in test_zip) uses the python html parser by default, but will switch to html5lib if available. The test_zip test should be updated to explicitly state which parser it's using. I'm okay with that happening as part of this ticket.
I tested this using the README.md file in the PR. This PR does not handle that file very well. It both sanitizes the <PLUGIN_NAME> strings and adds closing tags for them to the end of the file (</plugin_name>). Then something (maybe the markdown-it sanitizer) is doubly-encoding the < to &lt;. Is there a way we can avoid this? Can we load the markdown dynamically and only pass it through the markdown-it sanitizer?

Cheers,
@felliott

cslzchen · 2018-03-05T19:35:56Z

@felliott Thanks for the review. Need to take another look since it's been a while. Hopefully later this week.

coveralls · 2018-05-02T22:45:08Z

Coverage decreased (-0.1%) to 70.907% when pulling 5e4f4fd on cslzchen:feature/svcs-418-improve-md into 7304d3b on CenterForOpenScience:develop.

cslzchen

Finally, ready for review once more! 🎆

cslzchen · 2018-05-03T14:11:59Z

mfr/extensions/md/render.py

-        with open(self.file_path, 'r') as fp:
-            body = markdown.markdown(fp.read(), extensions=[EscapeHtml()])
-            return self.TEMPLATE.render(base=self.assets_url, body=body)
+        return self.TEMPLATE.render(base=self.assets_url, url=self.metadata.download_url)


MD renderer is now of type direct-to-wb instead of through-renderer. Only pass the WB download URL to the template.

cslzchen · 2018-05-03T14:14:35Z

mfr/extensions/md/templates/viewer.mako

+<script>
+
+    ## How to load ``markdown-it``: https://github.com/markdown-it/markdown-it#simple
+    var MarkdownIt = window.markdownit;


Use window.<LIBRARIES> to load the libraries which have already been wrapped with root.<LIBRARIES> due to MFR lacking a package manager.

cslzchen · 2018-05-03T14:22:44Z

mfr/extensions/md/templates/viewer.mako

+    wb_request.onload = function () {
+        document.getElementById("mdViewer").innerHTML = markdown.render(wb_request.response);
+        ## Force the host to resize
+        window.pymChild.sendHeight();


iframe has a min height of 150 pixels, which is the height before its innerHTML is updated. Without this line, it doesn't resize to proper height unless there is a certain user action (scrolling, clicking, resizing, etc.) on the page.

cslzchen · 2018-05-03T14:24:57Z

tests/extensions/md/test_renderer.py


+    def test_render_md(self, mock_renderer):
+        body = mock_renderer.render()
+        assert mock_renderer.metadata.download_url in body


The only thing that needs testing is that the download URL are correctly passed to and parsed by the template.

- MFR uses the same markdown-it.js and its plugins as OSF for rendering MD. However, due to lack of package managing and Babel, all full scripts are manually eslinted and babeled if necessary. .min scripts are kept intact.

- Instead of sending the HTML rendered by python markdown, MD renderer passes the WB download URL directly to the template. - Direct-to-wb replaces through-renderer as the new dispatch type.

- MFR frontend makes XHR to fetch the raw content directly from WB with CORS enforced. Markdownit and its plugins sanitize and render the content before updating the MFR viewer.

Empty __init__.py and unused fixtures/files are removed. The new test only verifies the download url. The acutual render process is done in the browser.

cslzchen mentioned this pull request Dec 7, 2017

[SVCS-418] MFR Markdown update + Mathjax #272

Closed

cslzchen commented Dec 7, 2017

View reviewed changes

cslzchen force-pushed the feature/svcs-418-improve-md branch 6 times, most recently from d3406b1 to 2374b64 Compare December 8, 2017 19:23

cslzchen commented Dec 8, 2017

View reviewed changes

cslzchen changed the title ~~[SVCS-418] [WIP] Improve md rendering~~ [SVCS-418] Improve md rendering Dec 8, 2017

cslzchen added the Final Review label Dec 8, 2017

felliott requested changes Mar 5, 2018

View reviewed changes

cslzchen added the Add'l Dev label Mar 5, 2018

cslzchen force-pushed the feature/svcs-418-improve-md branch from 2374b64 to 99ad810 Compare March 16, 2018 21:32

cslzchen force-pushed the feature/svcs-418-improve-md branch 2 times, most recently from 3e72b69 to 8a29a81 Compare April 17, 2018 16:31

cslzchen removed the Final Review label Apr 20, 2018

cslzchen force-pushed the feature/svcs-418-improve-md branch from 8a29a81 to edf8ae5 Compare May 2, 2018 22:42

cslzchen force-pushed the feature/svcs-418-improve-md branch from edf8ae5 to 9cd46de Compare May 3, 2018 04:26

cslzchen changed the title ~~[SVCS-418] Improve md rendering~~ [SVCS-418] Render MD files at the Frontend May 3, 2018

cslzchen added Final Review and removed Add'l Dev labels May 3, 2018

cslzchen force-pushed the feature/svcs-418-improve-md branch from 9cd46de to bbbc038 Compare May 3, 2018 14:07

cslzchen commented May 3, 2018

View reviewed changes

cslzchen changed the title ~~[SVCS-418] Render MD files at the Frontend~~ [SVCS-418] Render MD Files at the Frontend May 3, 2018

cslzchen assigned felliott Jun 18, 2018

cslzchen added 2 commits June 19, 2018 09:51

Add static js and css for markdownit and plugins

4790e52

- MFR uses the same markdown-it.js and its plugins as OSF for rendering MD. However, due to lack of package managing and Babel, all full scripts are manually eslinted and babeled if necessary. .min scripts are kept intact.

MD renderer drops python markdown

a8da260

- Instead of sending the HTML rendered by python markdown, MD renderer passes the WB download URL directly to the template. - Direct-to-wb replaces through-renderer as the new dispatch type.

cslzchen added 3 commits June 19, 2018 09:51

Render MD files at the frontend, similar to OSF

a66e702

- MFR frontend makes XHR to fetch the raw content directly from WB with CORS enforced. Markdownit and its plugins sanitize and render the content before updating the MFR viewer.

Add a README for the MD renderer explaining quirks

044ab11

Update tests for the new MD renderer

5e4f4fd

Empty __init__.py and unused fixtures/files are removed. The new test only verifies the download url. The acutual render process is done in the browser.

cslzchen force-pushed the feature/svcs-418-improve-md branch from bbbc038 to 5e4f4fd Compare June 19, 2018 13:52

cslzchen added the Needs ⛺ 🔥 label Jul 23, 2018

felliott removed the Needs ⛺ 🔥 label Jul 30, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SVCS-418] Render MD Files at the Frontend #305

[SVCS-418] Render MD Files at the Frontend #305

cslzchen commented Dec 7, 2017 •

edited

Loading

cslzchen left a comment •

edited

Loading

cslzchen Dec 7, 2017

cslzchen left a comment •

edited

Loading

cslzchen Dec 8, 2017

AddisonSchiller Dec 15, 2017

cslzchen Dec 8, 2017

cslzchen Dec 8, 2017

cslzchen Dec 8, 2017

AddisonSchiller Dec 15, 2017

cslzchen Dec 15, 2017 •

edited

Loading

AddisonSchiller Dec 15, 2017

felliott left a comment

cslzchen commented Mar 5, 2018

coveralls commented May 2, 2018 •

edited

Loading

cslzchen left a comment

cslzchen May 3, 2018

cslzchen May 3, 2018

cslzchen May 3, 2018

cslzchen May 3, 2018

		@@ -0,0 +1,2 @@
		/! markdown-it-sanitizer 0.4.3 https://github.com/svbergerem/markdown-it-sanitizer @license MIT /

[SVCS-418] Render MD Files at the Frontend #305

Are you sure you want to change the base?

[SVCS-418] Render MD Files at the Frontend #305

Conversation

cslzchen commented Dec 7, 2017 • edited Loading

Ticket

Purpose

Changes

Before (The MFR Way)

After (The OSF Way)

Difference between OSF and MFR

Side Effects

Issues

QA Notes

Deployment Notes

cslzchen left a comment • edited Loading

Choose a reason for hiding this comment

For Myself:

Choose a reason for hiding this comment

cslzchen left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cslzchen Dec 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

felliott left a comment

Choose a reason for hiding this comment

cslzchen commented Mar 5, 2018

coveralls commented May 2, 2018 • edited Loading

cslzchen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cslzchen commented Dec 7, 2017 •

edited

Loading

cslzchen left a comment •

edited

Loading

cslzchen left a comment •

edited

Loading

cslzchen Dec 15, 2017 •

edited

Loading

coveralls commented May 2, 2018 •

edited

Loading