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

Some Browsers Don't Properly Import XML with Sub-Block Nodes #2499

Closed
BeksOmega opened this issue May 25, 2019 · 5 comments
Closed

Some Browsers Don't Properly Import XML with Sub-Block Nodes #2499

BeksOmega opened this issue May 25, 2019 · 5 comments
Assignees

Comments

@BeksOmega
Copy link
Collaborator

Problem statement

If the XML parser tries to deserialize some xml that has nodes inside block tags (i.e. shadow blocks, values, etc) it will sometimes fail, depending on the browser.

This can be seen when opening certain toolbox categories, as well as when importing straight XML through the playground.

Steps to Reproduce

  1. Try to import the below code in the playground.
<xml xmlns="http://www.w3.org/1999/xhtml">
  <block type="controls_if" id="L:ym~c^IZzI,G#/PaR(^" x="38" y="88">
    <value name="IF0">
      <block type="logic_boolean" id="JQdsdXtYxW}j~W8k%ke`">
        <field name="BOOL">TRUE</field>
      </block>
    </value>
  </block>
</xml>
  1. Observe the error.

Stack Traces

0: Object doesn't support property or method 'getAttribute'
xml.js (630,6)

Operating System and Browser

  • Windows Internet Explorer 11
  • Windows Edge

Additional Information

I think the problem originates with #2468 If you try and log the value of Element.TEXT_NODE inside of the domToBlockHeadless_ function when you're running on internet explorer or edge, it will return undefined. If you log it when on chrome or firefox it returns 3 (correct).

@NeilFraser
Copy link
Contributor

Good grief. Thanks for this.

Can you check to see if Microsoft browsers know about Node.TEXT_NODE?

@NeilFraser NeilFraser self-assigned this May 26, 2019
@NeilFraser
Copy link
Contributor

I've prepared this commit: 2c09839 However, I'll wait until we hear whether Microsoft browsers support Node constants before creating a PR.

What's odd is that Element is a child of Node. Which means that if a property is on Node, then it should also be on Element.

@BeksOmega
Copy link
Collaborator Author

Can you check to see if Microsoft browsers know about Node.TEXT_NODE?

Screen Cap of Edge Console:
TEXTNODE_Edge

Screen Cap of IE 11 Console:
TEXTNODE_IE

So it looks like they do!

@NeilFraser
Copy link
Contributor

Thanks! PR #2503 created.

@RoboErikG
Copy link
Contributor

#2503 has been merged.

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

3 participants