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

JupyterLab removing HTML style attribs #1812

Closed
AbdealiJK opened this issue Feb 27, 2017 · 27 comments
Closed

JupyterLab removing HTML style attribs #1812

AbdealiJK opened this issue Feb 27, 2017 · 27 comments

Comments

@AbdealiJK
Copy link
Member

@AbdealiJK AbdealiJK commented Feb 27, 2017

In Jupyter Notebook I can make a markdown cell with

<img src="resources/a.png" height="200px" width=" 200px" style="display: inline-block;"/> VS <img src="resources/b.png" height="200px" width=" 200px" style="display: inline-block;"/>

and it rendered nicely as:

Pic1 VS Pic2

In JupyterLab this does not work anymore and it shows instead:

    Pic1
     VS
    Pic2
@blink1073
Copy link
Member

@blink1073 blink1073 commented Feb 27, 2017

Thanks @AbdealiJK, it looks like we need to add style to the list of allowed tags for images here.

@AbdealiJK
Copy link
Member Author

@AbdealiJK AbdealiJK commented Feb 28, 2017

@blink1073 Could the style tag be added as allowed for all tags ? Doing a style for a <div> or <p> can also be useful.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Feb 28, 2017

I think we can generally support the style tag, as long as it is properly sanitized (I take back my earlier comment about allowing it wholesale, CSS is also susceptible to attack).

@AbdealiJK
Copy link
Member Author

@AbdealiJK AbdealiJK commented Mar 11, 2017

Did a quick analysis of the tags we allow and the tags that Jupyter Notebook allows - As I'm assuming we want feature parity. Here's a summary:

Tags

  • Allowed in sanitize-html: h3, h4, h5, h6, blockquote, p, a, ul, ol, nl, li, b, i, strong, em, strike, code, hr, br, div, table, thead, caption, tbody, tr, th, td, pre
  • Allowed in JLab: + svg, h1, h2, img, span
  • Allowed in google-caja (marked as safe): a, abbr, acronym, address, area, article, aside, audio, b, bdi, bdo, big, blockquote, br, button, canvas, caption, center, cite, code, col, colgroup, command, data, datalist, dd, del, details, dfn, dir, div, dl, dt, em, fieldset, figcaption, figure, font, footer, form, h1, h2, h3, h4, h5, h6, header, hgroup, hr, i, iframe, img, input, ins, kbd, label, legend, li, map, mark, menu, meter, nav, nobr, ol, optgroup, option, output, p, pre, progress, q, s, samp, section, select, small, source, span, strike, strong, sub, summary, sup, table, tbody, td, textarea, tfoot, th, thead, time, tr, track, tt, u, ul, var, video, wbr
  • Allowed in Jupyter Notebook: (but it has an argument on whether to allow style tags)

Note: The google-caja list can be gotten in Jupyter Notebook JS console with:

for (var key in Jupyter.security.caja.html4.ELEMENTS) { if (!(Jupyter.security.caja.html4.ELEMENTS[key] & Jupyter.security.caja.html4.eflags.UNSAFE)) {console.log(key)}; }

From all the tags that google-caja knows of, it marks the following as unsafe: base, basefont, body, dialog, frame, frameset, head, html, isindex, keygen, link, meta, noembed, noframes, noscript, object, param, script, style, title

@AbdealiJK
Copy link
Member Author

@AbdealiJK AbdealiJK commented Mar 11, 2017

Let me know if we want all of these. i can also check which attributes in each of these are allowed in google-caja.

Do we want to use google-caja in JLab to get this list ? or should we write our own list using google-caja as reference ?

EDIT: Here's what caja does with attributes - http://pastebin.com/RMkxkYDE Jupyter Notebook allows all data-* attributed on top of this list and also allows img::src even if it's not parseable so that data uri are accepted

@blink1073
Copy link
Member

@blink1073 blink1073 commented Mar 11, 2017

I think the right answer here is to use the google-caja package instead of sanitize-html and port this file to TypeScript. The one barrier to doing so is that there is no typings file written for google-caja. If you'd like to take it on this file is a good reference.

@AbdealiJK
Copy link
Member Author

@AbdealiJK AbdealiJK commented Mar 12, 2017

Could you point me to which googlecaja we would like to use in npm? There seem to be many packages that create a npm bundle out of it.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Mar 12, 2017

Looking at this more, I'm not sure there is much we can do until googlearchive/caja#1977 is fixed. I'd prefer to err on the side of over-sanitizing rather than use an unsupported sanitization library. However, I think we can welcome a PR that makes the HTML tag handling consistent with the classic notebook.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Mar 12, 2017

@minrk, you maintain a bower package for google-caja, what are your thoughts on this?

@minrk
Copy link
Contributor

@minrk minrk commented Mar 13, 2017

I did the bower one so we could use it in the notebook. We can do the same for npm in jupyterlab. Would you like me to sketch out how to make that package? Or maybe use someone else's if they have already made it like @AbdealiJK suggests?

@blink1073
Copy link
Member

@blink1073 blink1073 commented Mar 13, 2017

We could start with https://www.npmjs.com/package/google-caja and fork your package as an npm package if we run into trouble.

@minrk
Copy link
Contributor

@minrk minrk commented Mar 13, 2017

If someone's already packaged it, it makes sense to start from there. It doesn't bode super well that the package hasn't received an update in four years and the owner has deleted their GitHub account, though.

@AbdealiJK
Copy link
Member Author

@AbdealiJK AbdealiJK commented Mar 13, 2017

I've found a lot of packages for google-Caja on Npm , but most of them haven't been updated for more than a year and that looks like a big negative to me.
The nicest one looks like https://github.com/mapbox/sanitize-caja which is a component for map box. We may want to make a jupyterlab/google-caja like the other things we're splitting out

@minrk
Copy link
Contributor

@minrk minrk commented Mar 13, 2017

I imagine the only thing needed to turn my caja bower package into npm would be a package.json and possibly index.js, so that should be pretty quick. @blink1073 feel free to make a PR on that repo (and/or migrate it to JupyterLab, if you think that's a good idea).

@blink1073
Copy link
Member

@blink1073 blink1073 commented Mar 13, 2017

I'm inclined to try the mapbox one and not try and maintain our own fork unless it is missing something.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Mar 14, 2017

Moving this to 1.0 as it is a feature parity issue and may take some time.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Nov 28, 2017

Good news from sanitize-html:

1.16.0: support for sanitizing inline CSS styles, by specifying the allowed attributes and a regular expression for each. 

@SylvainCorlay
Copy link
Member

@SylvainCorlay SylvainCorlay commented Mar 17, 2018

Note: this issue also applies to divs.

The following renders a noice blue rectangle in the classic notebook, but the css is stripped in jupyterlab.

<div style="background: blue;
            width: 100%;
            height: 200px;
            line-height: 200px;
            color: white;
            text-align: center;">
Noice!
</div>

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Mar 20, 2018

Looks like <sup> is another tag that needs to be added to the sanitizer whitelist: #4160.

@pangyouzhen
Copy link

@pangyouzhen pangyouzhen commented Mar 25, 2018

<colspan>,<rowspan> don't work.

<table class="tg">
  <tr>
    <th class="tg-s6z2"></th>
    <th class="tg-s6z2" colspan="2" style="text-align:center">Col</th>
  </tr>
  <tr>
    <td class="tg-s6z2"></td>
    <td class="tg-s6z2">Col1</td>
    <td class="tg-s6z2">Col2</td>
  </tr>
  <tr>
    <td class="tg-s6z2">A</td>
    <td class="tg-s6z2">1</td>
    <td class="tg-s6z2">2</td>
  </tr>
  <tr>
    <td class="tg-s6z2">B</td>
    <td class="tg-s6z2">3</td>
    <td class="tg-s6z2">4<br></td>
  </tr>
</table>

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Apr 12, 2018

<kbd> is another one: #4374.

@biermeester
Copy link
Member

@biermeester biermeester commented Aug 1, 2018

I'm currently working on this. See #5012 if you want to provide further input.

@blink1073 blink1073 removed this from the 0.34 milestone Aug 13, 2018
@blink1073 blink1073 added this to the 0.35 milestone Aug 13, 2018
@blink1073 blink1073 removed this from the 0.35 milestone Sep 5, 2018
@blink1073 blink1073 added this to the 1.0 milestone Sep 5, 2018
@philt31
Copy link

@philt31 philt31 commented Sep 10, 2018

Hello, do you know when this option will be available in a clonable version of jupyterlab. I tried the following markdown with Version 0.35.0a0:
Some Markdown text with some blue text.

but I'm still not seeing coloured text.

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Sep 10, 2018

It works on JupyterLab master:

screen shot 2018-09-10 at 9 10 20 am

It will be released with the next release of JupyterLab past 0.34.

@biermeester
Copy link
Member

@biermeester biermeester commented Sep 10, 2018

I tried installing from master the other day, using a docker container:

RUN git clone https://github.com/jupyterlab/jupyterlab.git
RUN cd /jupyterlab && \
    pip install -e . && \
    jlpm install && \
    jupyter lab build

But then the latest released tag version seems to get installed (at least, that's what pip freeze told me). I only could get the CSS styles to work when I did a jlpm run build and ran in dev-mode.

Also, I tried styling Markdown in a notebook, not in a Markdown file. But that shouldn't make a difference, should it?

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Sep 10, 2018

I only could get the CSS styles to work when I did a jlpm run build and ran in dev-mode.

Correct - that's how you run master.

But that shouldn't make a difference, should it?

Correct - it shouldn't make a difference.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

9 participants