goat1000 / SVGGraph

PHP SVG graph library
http://www.goat1000.com/svggraph.php
GNU Lesser General Public License v3.0
120 stars 30 forks source link

Preserve JS when SVG inserted via document.getElementById("id").innerHTML = result; #26

Open amessina opened 5 years ago

amessina commented 5 years ago

I had been inserting SVG graphs into my HTML5 page dynamicall via Javascript's Fetch API. After 21555c74fee7c0a8c4e98e8a6956727a29c4a346, this no longer works, with error Uncaught TypeError: window[initfns[f]] is not a function at init (eval at <anonymous> (my.js:401), <anonymous>:114:23)

The graph is created with $this->graph->Fetch(<graphtype>, FALSE, FALSE);

Admittedly, the simplified code below was a quick workaround when I switched from jQuery to pure JS and learned that innerHTML wouldn't automatically run the <script></script> contained wtihin the SVG.

    fetch("/", {
        method: "POST",
        headers: {"Content-Type": "application/json"},
        body: JSON.stringify(data),
        mode: "same-origin",
        credentials: "same-origin"
    })
    .then(response => response.text())
    .then(result => {
        document.getElementById("graph").innerHTML = result;
        // innerHTML doesn't load/execute embedded Javascript,
        // so run eval() for SVG tooltips to work
        const svg = document.getElementsByTagName("svg")[0];
        const script = svg.getElementsByTagName("script")[0];
        eval(script.textContent);
    })
    .catch(error => {
        console.log("Request failed", error);
    });

I'm not sure this is a proper bug but am seeking some guidance on the best way to insert SVG directly into HTML5 and run the script associated with the SVG.

goat1000 commented 5 years ago

This looks like a change I made in the init.txt template has broken things - try editing templates/init.txt and change the

    window[initfns[f]]();

line back to how it was before:

    eval(initfns[f] + '()');

I'll try to take a proper look at it tomorrow and see if I can come up with a better fix.

amessina commented 5 years ago

Thanks, @goat1000. Your suggestion above restores the previous functionality. Though, again, there is probably a better way for me to be doing this in the first place.

goat1000 commented 5 years ago

I'm going to simplify how the init function works and it will fix this. The reason the current code fails is that the eval inserts the script functions and variables into a closure instead of the global scope - as an alternative you could copy the script to a new element and add it to the document:

// eval(script.textContent);
const s = document.createElement('script');
s.textContent = script.textContent;
document.getElementsByTagName('head')[0].appendChild(s);
amessina commented 5 years ago

Thank you. Your suggestion above does work. I'm hoping to add multiple SVGs (all with their own tool tips) to the same page in multiple different Fetch API queries, so each SVG/Javascript will be independent of the others as far as SVGGraph PHP processing is concerned.

goat1000 commented 5 years ago

In that case you probably will need to keep each graph's script inside its own closure, since they would all be assigning different values to the same variables.

amessina commented 5 years ago

In that case you probably will need to keep each graph's script inside its own closure, since they would all be assigning different values to the same variables.

I'm sure you're right, though I'm not sure how to do that yet.

goat1000 commented 5 years ago

I've played about with this a bit and have got it working with two graphs on a page, each with different variables and functions. You must set the SVGGraph id_prefix option for your graphs so they don't use the same element ids, but apart from that it just works.

To put the code in separate closures I used this:

  const fn = new Function("(function(){\n" + script.textContent + "\n})();");
  fn();

If SVGGraph used its own closure this would be simpler, so I'll add it to my to-do list.

amessina commented 5 years ago

The enclosure combined with setting id_prefix works. Thank you for pointing me in the right direction. I look forward to your updates.