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.

None yet

4 participants