Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

fix(angular.copy): support copying XML nodes #12786

Merged
merged 1 commit into from
Sep 9, 2015

Conversation

petebacondarwin
Copy link
Member

Closes #5429

@petebacondarwin
Copy link
Member Author

Good old Internet Explorer:

IE 11.0.0 (Windows 8.1) angular copy should support XML nodes FAILED
    Expected '<?XML:NAMESPACE PREFIX = "PUBLIC" NS = "URN:COMPONENT" /><foo></foo>' to equal '<foo></foo>'.

@gkalpak
Copy link
Member

gkalpak commented Sep 8, 2015

Did you force-push a fix for the IE issue ?

@@ -386,6 +386,13 @@ describe('angular', function() {
expect(aCopy).toBe(aCopy.self);
});

it("should support XML nodes", function() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

" --> ' ? 😊

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:-) cut and paste from the original commit

@petebacondarwin
Copy link
Member Author

@gkalpak - yes I hid my shame with a forced push. It was not really IE's fault (well yes it probably was, but it was mine too). Can you spot the mistake in my test:

    it("should support XML nodes", function() {
      var anElement = document.createElement("foo");
      var theCopy = anElement.cloneNode();
      expect(copy(anElement.outerHTML)).toEqual(theCopy.outerHTML);
      expect(copy(anElement)).not.toBe(anElement);
    });

@petebacondarwin
Copy link
Member Author

@gkalpak and @jbedard I added a commit with your suggested changes. Thanks

@gkalpak
Copy link
Member

gkalpak commented Sep 9, 2015

@petebacondarwin

Can you spot the mistake in my test

You mean besides the "..." ? 😄
Oh...and the misplaced parenthesis :)

LGTM

@petebacondarwin
Copy link
Member Author

Thanks. I'll merge

@petebacondarwin petebacondarwin merged commit 122ab07 into angular:master Sep 9, 2015
petebacondarwin added a commit that referenced this pull request Nov 19, 2015
@petebacondarwin petebacondarwin deleted the issue-5429 branch December 17, 2015 12:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants