Skip to content

dangerouslySetInnerHTML in IE11 for svg elements #10013

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
OriR opened this issue Jun 20, 2017 · 6 comments · Fixed by #11108
Closed

dangerouslySetInnerHTML in IE11 for svg elements #10013

OriR opened this issue Jun 20, 2017 · 6 comments · Fixed by #11108

Comments

@OriR
Copy link
Contributor

OriR commented Jun 20, 2017

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
When using dangerouslySetInnerHtml of svg elements, the DOM nodes from the previous render aren't being removed.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template: https://jsfiddle.net/84v837e9/).

See this fiddle

What is the expected behavior?
The DOM should only contain the last rendered value for dangerouslySetInnerHTML.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
React - 15.5.4+
Browser - IE11
Did this work in previous versions of React? Not sure

After digging around I found #6982 . Which made me think this is happening because of this line in setInnerHTML.js where it looks like this

  // IE does not have innerHTML for SVG nodes, so instead we inject the
  // new markup in a temp node and then move the child nodes across into
  // the target node
  if (node.namespaceURI === DOMNamespaces.svg && !('innerHTML' in node)) {
    reusableSVGContainer =
      reusableSVGContainer || document.createElement('div');
    reusableSVGContainer.innerHTML = '<svg>' + html + '</svg>';
    var svgNode = reusableSVGContainer.firstChild;
    while (svgNode.firstChild) {
      node.appendChild(svgNode.firstChild);
    }
  }

Because there's a call to node.appendChild(svgNode.firstChild); it will never remove all of the previous nodes but only add the new ones.
But that's just a guess...

@gaearon
Copy link
Collaborator

gaearon commented Jul 11, 2017

Would you like to create a failing unit test and maybe send a fix? Thanks!

@gaearon
Copy link
Collaborator

gaearon commented Jul 11, 2017

(You can probably force this codepath by doing some JSDOM monkeypatching)

@gaearon
Copy link
Collaborator

gaearon commented Oct 4, 2017

This seems like it would be easy to fix by clearing the container, as you suggested. Indeed we can't really unit test it because it only occurs in IE11, but a manual test will do. Want to send a PR?

@OriR
Copy link
Contributor Author

OriR commented Oct 5, 2017

Ah sorry... missed that one =/
I'd be happy to make a PR!
I'll get on it today, not sure how to test this but I'll see what I can do :)

@gaearon
Copy link
Collaborator

gaearon commented Nov 3, 2017

React 16.1.0-beta has been released. Please update react, react-dom, and react-test-renderer (if you use it) to this version and let us know if it solved the issue! We’d appreciate if you could test before Monday when we plan to get 16.1.0 out.

@OriR
Copy link
Contributor Author

OriR commented Nov 3, 2017

We haven't updated to React 16.0.0 yet, so we probably won't be able to do this by Monday.
In any case, we've found a workaround for our use case by having a conditional classname - we're not re-assigning dangerouslySetInnerHtml anymore.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants