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

IFrame scope issues in 0.6.0 #225

Closed
rdeits opened this issue Nov 15, 2018 · 4 comments
Closed

IFrame scope issues in 0.6.0 #225

rdeits opened this issue Nov 15, 2018 · 4 comments
Labels

Comments

@rdeits
Copy link
Collaborator

rdeits commented Nov 15, 2018

Hi all! I'm having some issues with MeshCat.jl using WebIO 0.6.0, and I think the problem comes down to the new iframe behavior. As an example, try running the p5.js demo in IJulia, using an iframe to wrap its output:

using WebIO, JSExpr

w = Scope(imports=["//cdnjs.cloudflare.com/ajax/libs/p5.js/0.5.11/p5.js"])

onimport(w, @js function (p5)
    function sketch(s)
        s.setup = () -> s.createCanvas(640, 480)

        s.draw = function ()
          if s.mouseIsPressed
            s.fill(0)
          else
            s.fill(255)
          end
          s.ellipse(s.mouseX, s.mouseY, 80, 80)
        end
    end
    @new p5(sketch, this.dom.querySelector("#container"))
end)

iframe(w(dom"div#container"()))

I see no output at all from this when using WebIO 0.6.0. From poking around at the similar issues with MeshCat.jl, I think the problem is that previously we were running JS code inside the iframe's scope (with window and document set to those of the iframe), but that no longer seems to be the case.

@twavv
Copy link
Member

twavv commented Feb 23, 2019

I've been looking at this for... a while.

I think the issue boils down to this line in p5:

// this is the p5 instance
this._setup = function() {
    // ...
    var canvases = document.getElementsByTagName('canvas');
    for (var i = 0; i < canvases.length; i++) {
      // unhide canvases
    }
}

P5 hides canvases when they're created (to avoid a "flicker" while they're being setup) and then unhides them after the setup is done.

However, (and I'm about 90% sure of this), document in this context refers to the document of the parent window and not the document of the IFrame. The reason for this is because p5 is loaded in the top window context (since that's where WebIO is loaded and that's where SystemJS "lives").

To be more explicit, in a Jupyter notebook,

  • We render some WebIO output in the notebook
  • The nbextension activates and creates a new WebIO instance which is created in the notebook's Window
  • The WebIO rendering code creates an IFrame and patches in the singleton WebIO instance (i.e. const iframe = ...; iframe.contentWindow.WebIO = window.WebIO;)
  • A scope is created in the IFrame.
  • The scope requests P5.
  • WebIO loads the P5 library via SystemJS; since WebIO (and SystemJS) are running in the top window context, window and document refer to the window and document of the notebook tab, not the IFrame.
  • The onimport code executes, creating a new P5 instance.
  • P5 successfully creates the instances, but document.getElementsByTagName('canvas') doesn't return any canvases since they don't belong to document, but rather, iframe.contentDocument, and so they never get unhidden.

I'm not 100% sure that this is fixable in general. If P5 will take the merge request, it can be fixed by doing context.getElementsByTagName (where context refers to the DOM element that P5 was created on).
I think the best thing to do in general though is to avoid IFrames (they have a pretty heavy impact on memory usage in the browser - essentially equivalent to opening up a whole new tab).

The reason I don't think it's fixable in general (i.e. wherever this issue with document shows up) is because it would require loading the WebIO code as a <script /> which I also don't think is something we wanna do (it requires having a web server running which is something I'd like to get around to avoiding, as well as the performance cost of loading WebIO and all the libraries every time you have an output).

@rdeits
Copy link
Collaborator Author

rdeits commented Feb 28, 2019

Thanks for looking into this 🙂

I agree with your analysis (at least as far as I understand any of this), and I agree that the central problem is that document ends up referring to the parent document, rather than the IFrame document. I'm reasonably sure that this was not the case with previous versions of WebIO, since I know this example used to work in an iframe.

However, I should mention that my actual goal is not to make this p5.js demo work (it was just an existing example that broke at some point and made for an easy MWE of the problem). My actual goal is to get https://github.com/rdeits/MeshCat.jl working properly again in Jupyter with WebIO. The MeshCat javascript code is pretty particular about its styling, so the only way I've been able to get it to reliably render well is to use an iframe to separate it from Jupyter and its CSS. That said, I suspect I can edit it to work in a context in which document isn't the iframe, although it may require editing some of my dependencies as well.

@twavv
Copy link
Member

twavv commented Mar 3, 2019

Hmmm, well if it's strictly a CSS issue, there are some "fixes" to remove Jupyter-specific styling that you could try: https://stackoverflow.com/questions/10064172/how-to-isolate-a-div-from-public-css-styles.
I really do wish this was an easier thing to address.

@shashi shashi mentioned this issue Mar 27, 2019
7 tasks
@twavv
Copy link
Member

twavv commented Mar 28, 2019

I think one possible solution would be to have the IFrame load the generic HTTP provider bundle with a shim. That could be feasible but it definitely a very heavyweight solution; if at all possible, IFrames should be avoided, especially if there will be several. But at the very least, this might be fixable to work as it did before.

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

No branches or pull requests

3 participants