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

Store html content in a data-html attribute. #66

Merged
merged 9 commits into from
Jan 26, 2020
Merged

Store html content in a data-html attribute. #66

merged 9 commits into from
Jan 26, 2020

Conversation

dstein64
Copy link
Contributor

@dstein64 dstein64 commented Nov 6, 2019

This is a workaround for python-visualization/folium#812.

The prior approach of using a data URI for the src attribute can encounter a 2MB limit in Chrome, which is problematic for displaying folium maps with a lot of data (examples given in the aforementioned issue).

The approach proposed here is to use a data-html attribute containing the underlying html that will be used to populate the iframe. This requires javascript. It's not particularly elegant in my opinion, but it was the best I could think of given the trade offs.

An alternative approach, which I utilized to monkey patch folium/branca temporarily for a dashboard I created, is to use a srcdoc attribute in addition to the data URI in the src. srcdoc is not supported in IE/Edge, but utilizing the data URI in src combined with srcdoc works (the srcdoc is utilized by browsers that recognize it). However, this approach stores redundant data in src and srcdoc, and also requires special handling to escape the HTML stored under srcdoc (e.g., for quotes and ampersands).

The prior approach of using a data URI for the src attribute can
encounter a 2MB limit in Chrome.
Tests are failing with a WebDriverException.
The call to 'firefox --version' returns:
  libgtk-3.so.0: cannot open shared object file: No such file or
  directory
@dstein64
Copy link
Contributor Author

dstein64 commented Nov 6, 2019

The test_rendering_utf8_iframe test failed after I opened this PR. However, that test is seemingly unrelated to my update.

Upon further inspection, it appeared the issue was related to firefox. The travis logs reveal the following error when the latest firefox is installed.

Installing Firefox latest
$ firefox --version
XPCOMGlueLoad error for file /home/travis/firefox-latest/firefox/libmozgtk.so:
libgtk-3.so.0: cannot open shared object file: No such file or directory
Couldn't load XPCOM.

I suppose this did not occur for whatever version of firefox installed when these tests were last run four months ago.

Adding libgtk-3-0 as a dependency to .travis.yml fixes the issue.

@dstein64
Copy link
Contributor Author

dstein64 commented Nov 6, 2019

Adding libgtk-3-0 dependency fixes the issue mentioned above, but travis_apt_get_update sometimes hangs, causing the tests to fail. There is no way to disable the apt updating, since it happens whenever there are apt packages listed:
https://github.com/travis-ci/travis-build/blob/master/lib/travis/build/addons/apt.rb#L85

@Conengmo
Copy link
Member

I’m ready to merge this PR. I think it’s a good way to solve our issues with Jupyter on Chrome now. One question though, can you think of scenarios where this might break? You mentioned requiring JavaScript, but that’s required for Leaflet anyway. It seems according to specifications there’s no limit on data attributes. Anything else that might go wrong here?

@dstein64
Copy link
Contributor Author

dstein64 commented Jan 26, 2020

Hi @Conengmo,

Besides the Javascript scenario, I'm unable to think of any additional scenarios where this approach might break.

I've tested the approach on Chrome and Firefox. Additionally, I have not received any errors reported from the 10-20 users that are using an interface I built using this approach.

@Conengmo Conengmo merged commit f0dfe2e into python-visualization:master Jan 26, 2020
@Conengmo
Copy link
Member

Thanks so much for your work on this @dstein64, and your useful comments. It’s appreciated. We’ll make sure to get this released soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants