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

plots in plotly.js tutorial docs not displaying in Internet Explorer 11 #3186

Closed
priyatharsan opened this issue Oct 30, 2018 · 16 comments · Fixed by #3187
Closed

plots in plotly.js tutorial docs not displaying in Internet Explorer 11 #3186

priyatharsan opened this issue Oct 30, 2018 · 16 comments · Fixed by #3187

Comments

@priyatharsan
Copy link

priyatharsan commented Oct 30, 2018

This issue is specific only to the plots in CodePen.

codepen

@etpinard
Copy link
Contributor

Can you try opening https://codepen.io/ and any other example codepen in IE11 to help us out? Thanks.

@Braintelligence
Copy link

@etpinard Current release https://github.com/plotly/plotly.js/releases/tag/v1.42.0 doesn't work for me in IE 11 anymore. Something about The object doesn't support the attribute or method "remove"
The release before works fine...

@priyatharsan
Copy link
Author

priyatharsan commented Oct 30, 2018

Yeah I get this message when opening https://codepen.io/
CodePen probably won't work great in this browser. We generally only support the major desktop browsers like Chrome, Firefox, Safari, and Edge. Use this one at your own risk! If you're looking to test things, try looking at Pens/Projects in Debug View.

capture

like I doubted it could be related to codepen and not plotly but maybe I am wrong.
Also, the full page view is working fine in IE11

capture1

But not the pen view:

capture3

@Braintelligence
Copy link

Braintelligence commented Oct 30, 2018

IE 11 doesn't support ChildNode.remove() https://caniuse.com/#search=remove

@Braintelligence
Copy link

Providing this polyfill before triggering Plotly works: https://developer.mozilla.org/en-US/docs/Web/API/ChildNode/remove#Polyfill

@etpinard
Copy link
Contributor

@Braintelligence which example are you looking at?

@alexcjohnson
Copy link
Collaborator

This seems to be the offender:

if(style) style.remove();

@Braintelligence
Copy link

@etpinard I hope alex hit the spot. Sadly I'm not looking at any example, I'm looking at a website in the work that is still under NDA 🙈.

@priyatharsan
Copy link
Author

I also figured that some plots are displaying. For instance, these two examples https://plot.ly/javascript/line-charts/#connect-gaps-between-data and https://plot.ly/javascript/line-charts/#labelling-lines-with-annotations are displaying but not the rest of the plots in the doc. The only common difference I can find is these two plots has showlegend: false

@alexcjohnson
Copy link
Collaborator

(BTW thanks @Braintelligence for tracking down the broken feature)
Modify that polyfill to always overwrite, and trace its calls, and you can see whenever node.remove is called in any browser:

(function (arr) {
  arr.forEach(function (item) {
    Object.defineProperty(item, 'remove', {
      configurable: true,
      enumerable: true,
      writable: true,
      value: function remove() {
        console.trace(this);
        if (this.parentNode !== null)
          this.parentNode.removeChild(this);
      }
    });
  });
})([Element.prototype, CharacterData.prototype, DocumentType.prototype]);

Perhaps we can build that into our tests, with a throw instead of a console.trace, to ensure we never use node.remove?

@Braintelligence
Copy link

Braintelligence commented Oct 30, 2018

@alexcjohnson Good catch. I combined it with a bowser.msie check to not call the polyfill for anything but IE.

@alexcjohnson
Copy link
Collaborator

@Braintelligence my comment was just for our own debugging. Hopefully we'll have a patch release out shortly that fixes this so you won't need to polyfill at all, but as written in that MDN link it's great even without an IE check, since it'll only fill if the method is initially absent.

@Braintelligence
Copy link

Braintelligence commented Oct 30, 2018

@alexcjohnson Yeah, your comment still makes sense for me if you check that you use IE 11 beforehand 😸. The if-block was for skipping every element that has .remove() naturally, right? So if I just check once if I'm looking at IE 11 or not I can skip the whole iteration beforehand for everything but IE 11. 😼

@antoinerg
Copy link
Contributor

@Braintelligence Can you try the latest patched plotly.js (https://17351-45646037-gh.circle-artifacts.com/0/plotly.min.js) built from PR #3187 and confirm over there that it works!

@Braintelligence
Copy link

@antoinerg Sorry I'm off work now. Will try tomorrow :(

@Braintelligence
Copy link

@antoinerg I can confirm that your linked plotly.min.js works without providing the polyfill yourself.

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 a pull request may close this issue.

5 participants