Interactive portions of plot do not render correctly in a child window

[nielsenb-jf]

When using Plotly to render in a child window, the hover functionality for plots seems to break completely. Any enabled buttons will render in the incorrect location, but seem to function correctly.

This CodePen shows the issue.

The attached screenshot demonstrates the issue as well.

I have reproduced in the latest stable Firefox and Chrome.

[etpinard]

Thanks for report.

Interesting. I'm not sure what could cause that.

[nielsenb-jf]

Everything renders correctly if you copy the injected styles from the parent window to the child. See this CodePen for a demonstration.

Things break if you try to interact with the plot by clicking on a data point or by zooming by clicking and selecting a zoom area.

[etpinard]

@nielsenb-jf Interesting. Thanks for letting us know.

[nielsenb-jf]

I have a branch in my fork that demonstrates fixing interactivity when rendering to a child window. Basically, it renders the drag cover to the document of the child window, instead of always to the parent window where Plotly was instantiated.

Looking at the code more, Plotly makes lots of assumptions about which document / window to render to. I'm not sure if it would be worth a pull request for only fixing this one issue, I'm sure there are others I haven't found yet.

Would there be interest in trying to make Plotly more window / document agnostic? It does potentially enable some interesting multi-window webapps with Plotly. This is a big enough undertaking that I'd rather not start it if there's no upstream interest in incorporating it.

If yes, what should be done about the issue of injecting stylesheets? Should they be injected to the document containing the original element given to Plotly? Is that even feasible?

What about getting the correct parent window? document.defaultView would be the 'modern' way to do it, but would limit support of IE versions before 9. Is that an issue? I've also found a polyfill of sorts on StackOverflow.

[etpinard]

@nielsenb-jf thanks so much for your efforts.

that demonstrates fixing interactivity when rendering to a child window. Basically, it renders the drag cover to the document of the child window, instead of always to the parent window where Plotly was instantiated.

Fantastic!

Would there be interest in trying to make Plotly more window / document agnostic? It does potentially enable some interesting multi-window webapps with Plotly.

Yes. There is interest. But, this is not a priority by any means (explaining why no plotly team member has looked at this issue here in details).

what should be done about the issue of injecting stylesheets? Should they be injected to the document containing the original element given to Plotly? Is that even feasible?

I'm not exactly sure how to go about this (I personally don't know much about child windows). We currently inject our stylesheets using Lib.addStyleRule here. Maybe there's a way to make it work on child windows? Alternatively, we could perhaps expose a method on Plotly design to correctly populate child windows and have users that want to use plotly.js in child windows call that method upon window.open.

but would limit support of IE versions before 9. Is that an issue?

We don't officially support IE < 9 at the moment, so no, this is not an issue.

[nielsenb-jf]

I renamed my branch to 'child_window_fixes' to better describe the goal.

I tried testing as many plot types as I could, specifically looking for ones that make assumptions about the current window or document. The only other one that showed issues that I found were ones using range slider / selectors. The issue was the same as for other plots, the component actually managing the selection events was being attached to the parent, instead of to the child.

I also corrected an issue with restyle on plots in a child window not correctly reflecting the child window size.

So the question gets to be, should we clear up as many assumptions as possible, even if they don't cause issues? Or should things that are okay with happening in the parent window stay the way they are now? For example, the 'tester' element is currently appended to the parent window, not the child. This doesn't seem to be an issue, and I could see arguments for either way being "correct".

Yes. There is interest. But, this is not a priority by any means (explaining why no plotly team member has looked at this issue here in details).

I completely understand its not a priority for the team. It doesn't seem to be a priority for other users either since I seem to be the first person to stumble on it. It just seems to me that since Plotly takes a component as an argument, it shouldn't matter where that component is. Either way, I'm willing to try to make this work on my own with guidance when necessary.

I'm not exactly sure how to go about this (I personally don't know much about child windows). We currently inject our stylesheets using Lib.addStyleRule here.

I'm not either. I definitely need to look into this more. Any further advice you have would be greatly appreciated!

We don't officially support IE < 9 at the moment, so no, this is not an issue.

Good. nielsenb-jf/plotly.js@f10706d wouldn't work with it.

[etpinard]

Thanks again for your efforts!

Or should things that are okay with happening in the parent window stay the way they are now?

I would vote 👍 for this.

Plotly takes a component as an argument, it shouldn't matter where that component is.

Yes. No doubt about it.

---

So I guess this leave us to figure out what to do with our style sheets.

Perhaps we should move the Lib.addStyleRule calls to the main Plotly.plot routine (with a check to not replicate the rules in <style>) so that every plotly.js graph is guaranteed to be accompanied with the correct style sheets.

@nielsenb-jf Would you be interested in trying that out?

[nielsenb-jf]

New branch called child_css that incorporates the previous changes, as well as injecting styles in Plotly.plot. Everything I've tested seems to work.

I changed pull_css to generate a plotcss.js file that does nothing but export the style rules. Those rules are then injected by the new plotcss_injector. The injector will not inject a rule if there is already a matching rule in one of the target document's stylesheets. This prevents redundant rules when plot is called multiple times on a document.

addStyleRule had to be modified to not keep a reference to the stylesheet being modified. Otherwise, the wrong stylesheet would be modified when plot is called first on the parent window, then on a child, or vice versa. If the document already has stylesheets, it defaults to adding the rule to the first one. I'm not sure if that's the best idea or not, but the only other idea I could come up with is to always modify the last stylesheet, assuming it was the one injected.

[etpinard]

@nielsenb-jf great! Looks like you've got all the pieces together.

Here's a few recommendations before submitting your PR:

• Make injectStyles expect the graph div gd as argument
• Move the logic in Lib.addStyleRule to injectStyle (that way we won't have to check for <style> for each style rule) and remove addStyleRule from Lib.
• We should only loop through all present style rules on the first Plotly.plot for a given gd. So after that's done we should store an _ variable (e.g. gd._styleSheetLoaded = false and then true) to bypass that loop on subsequent Plotly.plot calls.
• make sure npm run lint passes
• find a way to test that injectStyles works as expected via Plotly.plot in this suite.
• maybe add a fallback for ownerDocument? MDN has no info about it in Safari. For example, we could early on in Plotly.plot have:

gd._document = gd.ownerDocument.defaultView || window.document;

// and then use gd._document afterwards in the plotting code

[nielsenb-jf]

I've got everything but the tests put together in the 'child_window_fixes' branch. Remaining work will happen there.

[nielsenb-jf]

I've added some tests to the child_window_fixes branch.

Are we sure we want the tests in the plot_interact_test suite? It feels a little cleaner to me to have them be their own thing. I could also see them in with the plot_api_test suite since the CSS gets injected in the plot method, which is defined in plot_api. I'll defer to your judgement, it's not like we can't cut/paste the code somewhere else later.

Additionally, notice the buildFullSelector function. It appears both Firefox and Chrome do some processing to CSS selectors (and the rules themselves if you dig into it some) during an insertRule call. The result is equivalent, but it makes it difficult to test rules are inserted. I've done my best to make buildFullSelector format the selector the way Firefox and Chrome seem to, but I can't find any documentation anywhere that guarantees that will stay consistent. If I can find some documentation, I'll add some tests. Note that buildFullSelector gets used in the tests to check all selectors in plotcss were inserted properly, but due to the aforementioned processing of rules, there is no testing to guarantee the rule itself is actually correct.

[etpinard]

@nielsenb-jf Thanks so much!

Are we sure we want the tests in the plot_interact_test suite?

I agree. Your new test cases would look a lot better in a suite of their own. I'd call it plot_css_test.js, but I'll leave it up to you to decide the file name.

Additionally, notice the buildFullSelector function. It appears both Firefox and Chrome do some processing to CSS selectors (and the rules themselves if you dig into it some) during an insertRule call.

Your new css/helpers.js module looks great. Nice and DRY.

---

I think you're ready to make a PR to the main repo.

Squash those lint/formatting commits and PR away 🚀