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

toJson() update #82

Closed
wants to merge 4 commits into from
Closed

Conversation

vojtajina
Copy link
Contributor

@vojtajina
Copy link
Contributor Author

@igor:
I have applied all your comments and rebased into related commits, except the ones about checking window/document constructor.

Your suggestion does not work, as window.constructor.name is undefined.
So I leave my old code: instanceof Window, instance of Document respectivelly...
This works on my FF, Safari, even in iframes...
All tests pass....

@IgorMinar
Copy link
Contributor

sigh.. the issue is that what you implemented doesn't work in Chrome and likely also not in IE. I'm just going to do a simple comparison with document and object, which will work for everything but iframes, but we'll figure that out when we actually have a need to deal with that.

thanks for all the other changes.

@IgorMinar
Copy link
Contributor

committed as:
5be325a
b7027b9
fe8353b
c780030

@vojtajina
Copy link
Contributor Author

Sorry, I did not tested Chrome and IE, next time I will definitely do so !
I should fix that really soon, as this does not work in IE, Chrome...

This is a big pain, as I can't find a way, how to determine whether a object is document or window object, that would work across the all browsers...

So I can see two solutions at this moment:
1/ different code for different browsers... It's ugly, but when you touch the DOM, bad days come :-D

2/ I have looked into JSON.stringify() code (http://www.json.org/json2.js)
There is one thing I like. When angular's forEach() iterates, it checks whether the object has it's own forEach() method like array does and if so, call it... JSON.stringify() uses the same approach...
When object has a method toJson() it uses this method... I think this is a good feature for our toJson() method...
Then we could add this method to window, document objects... Probably when defining $window service...
angularServiceInject('$window', function() {
window.toJson = function() {
return 'WINDOW';
};
return window;
});

@IgorMinar
Copy link
Contributor

I think this is resolved. I amended your commit, aren't you happy with my hanges?

@vojtajina
Copy link
Contributor Author

Sorry, I did not check your changes before. Now I see, good solution, like it...

This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants