Skip to content
This repository has been archived by the owner on Oct 26, 2019. It is now read-only.

Some dashboards not working in Safari #141

Closed
3 tasks done
dalogsdon opened this issue Mar 14, 2016 · 26 comments · Fixed by #212
Closed
3 tasks done

Some dashboards not working in Safari #141

dalogsdon opened this issue Mar 14, 2016 · 26 comments · Fixed by #212
Assignees
Labels
Milestone

Comments

@dalogsdon
Copy link
Contributor

dalogsdon commented Mar 14, 2016

Simple dashboards are working fine in Safari, but the plotting demo and resizable widget demo do not.

There are no obvious relevant errors in Javascript console.

Steps to complete:

@nitind
Copy link
Member

nitind commented Apr 4, 2016

WIth a lot of help from @dalogsdon, determined that a DOMException was being silently ignored in the transformime HTML code because a Promise lacked a reject callback.

https://github.com/nteract/transformime/blob/master/src/html.transform.js#L5 encounters NotSupportedError: DOM Exception 9 in Safari.
https://github.com/nteract/transformime/blob/master/src/transformime.js#L70 catches it and calls Promise.reject(), but https://github.com/jupyter/jupyter-js-notebook/blob/7b56af00451435caf3e39681571871471fce7c7e/src/output-area/widget.ts#L254 has no "reject" callback to handle or log the exception.

A lot of this was refactored right around the time we started working on this, so it's possible that the problem has been fixed by the switch to rendermime (specifically https://github.com/jupyter/jupyter-js-ui/blob/master/src/renderers/index.ts#L31 no longer calling Range#createContextualFragment()), but moving the dashboard server to the newer version of jupyter-js-notebook appears non-trivial.

@parente
Copy link
Member

parente commented Apr 5, 2016

We need to update to the newest jupyter-js-notebook if it has moved on and is using rendermime instead. No sense in opening a defect against a lib that is no longer used.

Give the upgrade a shot and let's see how much else needs to change. Tap @jasongrout for help on the new output area usage as needed.

@jhpedemonte
Copy link
Collaborator

Worked with @nitind to update our code to use latest jupyter-js-notebook. One big issue that hit us was needing to set outputAreaModel.trusted = true, otherwise plotting libs JS (such as from matplotlib) won't run in the browser. This got us working with ipywidgets and matplotlib.

But Bokeh and Plot.ly are still not rendering. I see 2 different JS blocks added to the DOM for Bokeh. I inspected the websocket frames: the first incoming JS is large (191224) and includes bokeh.min.js. It ​does​ get executed -- I see the console logs.

Later, there is a 2nd Bokeh JS block WS message (size=86627) which ​does not​ get executed. Importantly, this 2nd block calls Bokeh.embed.embed_items(), which invokes register_target() to register the appropriate comm target. Since that comm target isn't registered, comm_msg to that target fails, resulting in:

Error: Class 690473ba-1081-4207-be37-138708b03373 not found in registry

I'm not sure why the 2nd JS block isn't getting executed (I see it in the DOM).

@nitind
Copy link
Member

nitind commented Apr 12, 2016

It seems plotting hasn't been tested against the refactored codebase (https://gitter.im/jupyter/notebook?at=570bf7972a2f4d4276138641). The lookup that fails is driven from a message in the iopub channel, but the message contents are lacking module information to go with the class name.

@parente
Copy link
Member

parente commented Apr 12, 2016

How about this for the path forward:

  1. Upgrade first to jupyter-js-notebook 0.15.x and see if bokeh / ploy.ly work in Chrome in that version so we at least know that we're closer to the latest and that it's not some other weird bug back in some older version.
  2. If it does work in 0.15, look at the delta between 0.15 and 0.16 and see if it's really just the switch to rendermime (in jupyter-js-ui) from transformime that is the likely culprit.
  3. If it is rendermime, try to construct a simple example / test-case of application/jupyterscript (or whatever the mimetype is) for rendermime to put a script on the page like we see failing for Bokeh / plot.ly.

/cc @blink1073

@parente
Copy link
Member

parente commented Apr 12, 2016

Also try 0.17.1 of jupyter-js-notebook just in case magic elves fixed it already.

@blink1073
Copy link

We explicitly do not support Safari in phosphor and jupyter-js-* because it is woefully not up to DOM specifications.

@parente
Copy link
Member

parente commented Apr 12, 2016

@blink1073 Oh, OK. That's one problem "down".

But the new one is that bokeh / plot.ly stop working in Chrome as of 0.16.x of jupyter-js-ui where rendermime is used in place of transformime.

@blink1073
Copy link

Jason is still actively working on this area, I'd say overall jupyter-js-notebook is still in flux and probably isn't ready to be used as a dependency.

See, for example, discussion here: jupyter/jupyter-js-notebook#171

@parente
Copy link
Member

parente commented Apr 12, 2016

No doubt it's under active dev. Glad to see that.

So for the time being, looks like we should get to the highest version where bokeh/plotly still work in Chrome (probably the last version that used transformime) and sit tight until things stabilize a bit.

One more question about the Safari support: do you guys plan to shim Safari support in the future once things are working well in other browser, or are you going to hold out for Safari to meet DOM spec?

@blink1073
Copy link

I'd say we're going to hold out for Safari to get its act together.

@parente
Copy link
Member

parente commented Apr 12, 2016

We explicitly do not support Safari in phosphor and jupyter-js-* because it is woefully not up to DOM specifications.

@blink1073 One more bother: is this doc'ed on the Phosphor site or a README? I couldn't find it. When folks ask what browsers phosphor/jupyterlab will work in, want to be able to point them to something concrete. (I'll open an issue if you tell me where it belongs.)

@blink1073
Copy link

All of our readmes have this section that lists the explicitly supported runtimes.

@parente
Copy link
Member

parente commented Apr 12, 2016

Doh! That's what happens when I search for the element not in the set on the page.

Thanks and sorry to bother again.

@parente
Copy link
Member

parente commented Apr 12, 2016

For our future ref:

Could be a little more testing and modification to make them match will fix things again. Or not.

@nitind
Copy link
Member

nitind commented Apr 13, 2016

Testing combinations of jupyter-js-notebook 0.15.5, 0.16.0, and 0.17.1 with jupyter-js-services 0.5.3, 0.7.1, and 0.8.0, jupyter-js-notebook 0.15.5 with jupyter-js-services 0.8.0 seems as recent as we can go. While it does generate a TypeError trying to add the RESULT_CLASS class to a document fragment at times, it doesn't encounter the class lookup problem. Nothing in the diff between jupyter-js-notebook 0.15.5 and 0.16.0 stood out as the reason for it, though.

"jupyter-js-notebook": "^0.15.5" + "jupyter-js-services": "^0.5.3" = works, console complains about  TypeError at jupyter-js-output-area.js
"jupyter-js-notebook": "^0.15.5" + "jupyter-js-services": "^0.7.1" = works, console complains about  TypeError at jupyter-js-output-area.js
"jupyter-js-notebook": "^0.15.5" + "jupyter-js-services": "^0.8.0" = works, console complains about  TypeError at jupyter-js-output-area.js
"jupyter-js-notebook": "^0.16.0" + "jupyter-js-services": "^0.7.1" = fails, guid lookup problem
"jupyter-js-notebook": "^0.16.0" + "jupyter-js-services": "^0.8.0" = fails, guid lookup problem
"jupyter-js-notebook": "^0.17.1" + "jupyter-js-services": "^0.5.3" = fails, guid lookup problem
"jupyter-js-notebook": "^0.17.1" + "jupyter-js-services": "^0.8.0" = fails, guid lookup problem

@parente
Copy link
Member

parente commented Apr 18, 2016

After switching to jupyter-js-notebook 0.17.1, putting in the rendermime transform chain logic, we discovered that the HTML renderer is the culprit preventing bokeh (and probably plot.ly) from working properly. Bokeh returns some scripts in text/html bundles. rendermime does not attempt to eval the scripts in these.

We can try to fix the problem upstream in rendermime. But we're also seeing hints that we may be targeting the wrong level of dependency using jupyter-js-notebook's OutputArea. For instance, it now renders a prompt node which is not something we'd ever want in the dashboard server. We could certainly style it away, but it's questionable if we want to keep hiding notebook-isms in jupyter-js-notebook (sounds foolhardy) rather than just starting from a lower level (rendermime? jupyter-js-services alone?)

@nitind, @jhpedemonte and I chatted about looking into creating a simple replacement for output area here. We might be wrong that something simple will do the trick, but it's worth investigation at least.

@jasongrout
Copy link
Member

jasongrout commented Apr 20, 2016

Would it be better if we made the prompt information part of the cell, and stripped the OutputArea down to just a container of outputs? That actually seems like a more natural break to me - the prompts deal with execution, which is a cell-level thing...

CC @blink1073

@blink1073
Copy link

Outputs themselves can contain prompts if they contain execute_result data: https://github.com/jupyter/jupyter-js-notebook/blob/master/src/output-area/widget.ts#L165

@blink1073
Copy link

But we could have an implementation that just renders a panel of outputs with no prompts and keep that in jupyter-js-ui.

@jasongrout
Copy link
Member

jasongrout commented Apr 20, 2016

@blink1073 - Yes, I guess we have to distinguish between output prompts that come in the output data and are rendered in its normal course, and output prompts that are set by the cell outside the output data. The cell-level output prompts, i.e., the prompts the cell is setting, are what make sense to me to move up to the cell level.

@parente
Copy link
Member

parente commented Apr 22, 2016

Outputs themselves can contain prompts if they contain execute_result data:

@blink1073 @jasongrout For my education, can you give an example of when output prompts come with the output data vs when they are set by the cell?

@blink1073
Copy link

blink1073 commented Apr 22, 2016

@parente, the prompt itself does not come with the output data, but the execution count does, and and the cell sets the prompt appropriately.

screen shot 2016-04-22 at 6 26 33 am

@parente
Copy link
Member

parente commented Apr 22, 2016

the prompt itself does not come with the output data, but the execution count does, and and the cell sets the prompt appropriately.

This one I understand. I was more wondering when the execution prompt comes "in the output data" like @jasongrout said vs in the separate execution_count field of execute_result.

@blink1073
Copy link

Yeah, I'm not sure what he meant by that either ;).

@parente parente modified the milestones: 0.5.0, 0.6.0 Apr 26, 2016
@parente parente assigned jhpedemonte and unassigned nitind May 6, 2016
@parente
Copy link
Member

parente commented May 6, 2016

Known steps to get this one finished added to the description. @jhpedemonte switched it over to you since you've been digging into it. One or many PRs, you decide.

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

Successfully merging a pull request may close this issue.

6 participants