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

kinda clone attrs obj argument #50

Merged
merged 1 commit into from
Dec 10, 2014
Merged

kinda clone attrs obj argument #50

merged 1 commit into from
Dec 10, 2014

Conversation

sonnyp
Copy link
Member

@sonnyp sonnyp commented Dec 3, 2014

Address #47

Ideally el.attrs would be an object with no prototype (Object.create(null)) so we wouldn't need hasOwnProperty and iterating would be faster but that would break on IE<9.

Also it's not a deep clone but we do not expect the object to be nested anyway.

So it's not perfect but I think it's better that just referencing the passed object. We can improve later.

@@ -808,11 +813,12 @@ SaxLtx.prototype.end = function(data) {

function unescapeXml(s) {
return s.
replace(/\&amp;/g, '&').
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure what happened here, maybe my browserify version?

Copy link
Member

Choose a reason for hiding this comment

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

Yes that will be it.

@sonnyp
Copy link
Member Author

sonnyp commented Dec 3, 2014

@lloydwatkin added test and reverted unexpected browserify changes

@sonnyp
Copy link
Member Author

sonnyp commented Dec 10, 2014

ping

@lloydwatkin lloydwatkin merged commit e67e7ef into xmppjs:master Dec 10, 2014
lloydwatkin pushed a commit that referenced this pull request Dec 10, 2014
@lloydwatkin
Copy link
Member

Made some small changes, pushed as 0.8.0. Thanks!

Mostly used Object.keys instead.

@sonnyp
Copy link
Member Author

sonnyp commented Dec 10, 2014

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants