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

XML toolbox does not parse in IE11 for <hr> #50

Closed
carlosperate opened this issue Jan 23, 2015 · 10 comments
Closed

XML toolbox does not parse in IE11 for <hr> #50

carlosperate opened this issue Jan 23, 2015 · 10 comments

Comments

@carlosperate
Copy link
Contributor

I've notice that the code demo does not work in IE11 in the latest head (1c39a63 at the time of writing).

The problem arises in xml.js file when it is trying to parse the toolbox xml (https://github.com/google/blockly/blob/master/core/xml.js#L199). I'm not quite sure if i'll have the time in the inmediate future to look into it to submit a pull request, so I though I should post it here in case somebody else can do that.

@NeilFraser
Copy link
Contributor

Just a random guess, is IE11 ok with


but not
?

@NeilFraser
Copy link
Contributor

Previous comment got rendered by GitHub. Heh. Let's try that again, this time HTML encoded...

Just a random guess, is IE11 ok with <hr/> but not <hr>?

@carlosperate
Copy link
Contributor Author

That was my first guest as well, but it does not work either way. However, I just noticed that using <hr/> causes IE to also output the following:
XML5633 "End-tag name does not match the corresponding start-tag name." (https://msdn.microsoft.com/en-us/library/ie/dn423949(v=vs.85).aspx).

I started looking back the git history up to when the separators were added (5aed33b) and parseFromString() it's been problematic in IE11 from the beginning.

What I find interesting is that in my fork I am loading an XML file with <hr/> tags for the toolbox using XMLHttpRequest and it does seem to work fine in IE.

@NeilFraser
Copy link
Contributor

Does it want <hr><hr/>?

Or maybe it is allergic to an XML tag than looks like HTML? Does switching from <hr> to <ie_is_trash> or <ie_is_trash/> work?

@carlosperate
Copy link
Contributor Author

<hr></hr> doesn't work.
<ie_is_trash> and <ie_is_trash/> don't work either.
<ie_is_trash></ie_is_trash> does work.
br behaves the same way hr does, so it seems to be specific to html tags.

@NeilFraser
Copy link
Contributor

Ok, that's really helpful, thank you.
I propose changing Blockly to use <sep></sep> instead of <hr>. Normally I'd push for allowing <hr> to live on as a deprecated alternative, but it looks like we need to actively eliminate it. I'll have Blockly (in non-IE browsers) print an error to the console if it sees a <hr> tag, ignore the tag, and continue parsing.

@NeilFraser
Copy link
Contributor

Change committed, documentation updated.

@carlosperate
Copy link
Contributor Author

I was just about to suggest a slightly different approach. It's not an elegant fix, but it would maintain the hr tag usable in IE. I can prepare a demo in a little bit of time.

@NeilFraser
Copy link
Contributor

The feature is only two months old, so I think it's fine to make a breaking change to it. There's probably only one developer using <hr> right now (the one who asked for it originally).

@carlosperate
Copy link
Contributor Author

Nevermind then. In case you are curios, IE was removing the self closing slash from the HTML DOM, and that is way it did not parse as valid XML (carlosperate@e990eb4). That is also why it worked in IE when I was loading the XML data from a file.

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

2 participants