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

Add support for xmlns elements in DOMOutputSpec #898

Open
1 of 4 tasks
steveccable opened this issue Mar 4, 2019 · 4 comments
Open
1 of 4 tasks

Add support for xmlns elements in DOMOutputSpec #898

steveccable opened this issue Mar 4, 2019 · 4 comments

Comments

@steveccable
Copy link

steveccable commented Mar 4, 2019

Issue details

Currently, when a node being created via toDOM and the returned DOMOutputSpec contains namespaced xml nodes (like svg), those nodes are being created incorrectly as non-namespaced nodes. This can create broken behavior in such tags. Keeping with the svg example, it can result in a zero-width element that actually displays nothing even if you do fix the width.

Potentially related reading: #643

Steps to reproduce

// Assume that this is being called by some node's toDom in your schema
// props and structure taken from https://developer.mozilla.org/en-US/docs/Web/SVG/Element/path
function createSvgSpec() {
  var svgProps = { viewBox: '0 0 100 100', xmlns: 'http://www.w3.org/2000/svg'};
  var pathDesc = 'M 10,30
           A 20,20 0,0,1 50,30
           A 20,20 0,0,1 90,30
           Q 90,60 50,90
           Q 10,60 10,30 z';
  return [
    'svg',
    svgProps,
    ['path', {d: pathDesc}]
  ];
}

ProseMirror version

"prosemirror-model": "^1.6.3" (though the core issue looks to be still there up to time of writing. See below)

Affected platforms

  • Chrome
  • Firefox
  • Internet Explorer
  • Other

(Gonna be honest, I didn't try to check the others, since this is a pretty basic issue).

Solution

This one is pretty straightforward, and I will probably open a PR if/when I find time to just do it, but the solution to fix this locally was very simple.
Where we are creating the node in to_dom.js, check if there is an xmlns prop, and if there is then use createElementNS instead of createElement. Also be sure to pass this xmlns to child elements, as they often depend on it. Sample of what this would look like in to_dom.js (yes, I know this is basically a PR, but it's a work thing and we have some process around contributing to open source. Like I said, I'll try and throw a PR up sometime soon):

static renderSpec(doc, structure) {
    if (typeof structure == "string")
      return {dom: doc.createTextNode(structure)}
    if (structure.nodeType != null)
      return {dom: structure}
    let attrs = structure[1], start = 1
    let xmlns = attrs['xmlns'] || parentXmlns
    let dom = xmlns ? doc.createElementNS(xmlns, structure[0]) : doc.createElement(structure[0]), contentDOM = null
    if (attrs && typeof attrs == "object" && attrs.nodeType == null && !Array.isArray(attrs)) {
      start = 2
      for (let name in attrs) {
        if (attrs[name] != null) dom.setAttribute(name, attrs[name])
      }
    }
    for (let i = start; i < structure.length; i++) {
      let child = structure[i]
      if (child === 0) {
        if (i < structure.length - 1 || i > start)
          throw new RangeError("Content hole must be the only child of its parent node")
        return {dom, contentDOM: dom}
      } else {
        let {dom: inner, contentDOM: innerContent} = DOMSerializer.renderSpec(doc, child, xmlns)
        dom.appendChild(inner)
       // The rest of the code continues unmodified
@steveccable
Copy link
Author

Turns out you need to check for the validity of attrs before trying to grab its xmlns (basically the same thing as what's in the if already). I'll be sure to include that cleanup in the real PR.

@andtii
Copy link

andtii commented Apr 18, 2019

@steveccable Im having the same issue! Do you have time to create a PR?

@pocke
Copy link

pocke commented Dec 27, 2019

Hi. I found the same issue from discuss.prosemirror.net.
I wrote a patch for the problem and commented it to the issue.
https://discuss.prosemirror.net/t/how-to-render-svg-something/1737

The patch is based on @steveccable 's patch. Thanks!

@jljorgenson18
Copy link

Not sure if this helps anyone, but you can return a dom element directly in your toDOM functions. If the node has no children, you could build up the svg element yourself and return that. That would bypass having to create the DOMOuputspec Array.

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

No branches or pull requests

4 participants