Skip to content
This repository has been archived by the owner on Sep 3, 2022. It is now read-only.

DataLab HTML outputs should execute any contained script elements #330

Closed
drewbryant opened this issue May 12, 2015 · 5 comments
Closed

Comments

@drewbryant
Copy link
Contributor

In order to support the DataLab charting extensions, it's necessary that HTML cell outputs with embedded script elements are executed.

For IPython, this is done automatically by JQuery whenever the HTML output content is written to the output DOM element.

@drewbryant
Copy link
Contributor Author

Apparently the Angular team's recommendation for getting <script> tag execution within HTML snippets is "use JQuery".

Looks like there are some lighter weight alternatives though to explore: http://stackoverflow.com/a/25431736

@nikhilk
Copy link
Contributor

nikhilk commented May 13, 2015

If we're guaranteed the browser doesn't execute the script, then potentially we could insert the HTML into the DOM via innerHTML, and then walk the DOM to find the script elements and execute them, rather than doing all that parsing and handling HTML pre/post script separately as outlined in the link.

@drewbryant
Copy link
Contributor Author

Seems like it could work. When I inspect the HTML being injected by the charting library within the cell output, just grabbing the script tag verbatim and evaluating it does cause the expected chart to be rendered.

Some details around innerHTML and script tag handling here: http://stackoverflow.com/a/13392818

@nikhilk
Copy link
Contributor

nikhilk commented May 13, 2015

I think that page might be getting at it, but its probably a good idea to use a document fragment (document.createDocumentFragment) to do the initial parsing and walking over the DOM to pull out content and script elements to insert into the output cell within the primary DOM.

@drewbryant
Copy link
Contributor Author

Ok, will look into this approach

drewbryant added a commit that referenced this issue May 18, 2015
drewbryant added a commit that referenced this issue May 18, 2015
drewbryant added a commit that referenced this issue May 27, 2015
drewbryant added a commit that referenced this issue May 28, 2015
rajivpb pushed a commit to rajivpb/datalab that referenced this issue Jun 16, 2017
* Set up tox, and use it for local testing and Travis.

[tox](https://tox.readthedocs.io/) is a Python test automation tool: it makes
it easy to automate running the tests for your project in several
environments. Under the hood, it just uses virtualenv to manage installed
deps.

If you have python2.7 and python3.5 installed, simply running `tox` at the
root of the repo will now run tests in both versions, as well as `flake8`.

This PR makes two changes:

* first, convert the existing `.travis.yml` into `tox.ini`, setting up three
  tox environments based on the configuration that was already
  there. (Relatedly, I added the `.tox` directory to `.gitignore`).

* Now that googledatalab#330 is fixed (wooo @brandondutra), I switched the `.travis.yml`
  over to use `tox`. Now there are separate travis environments for each tox
  environment, which has two advantages: (1) these three all run in parallel,
  speeding up builds, and (2) build logs are separated, making it faster to
  figure out what caused a given failure.

* (Code review followups, delete message on squash)

1. Note in `CONTRIBUTING.md`
2. Simplified `flake8` invocation in `tox.ini`.

* (more code review)

* (last code review round)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants