Check if data is an object in loadRemote. #206
Conversation
|
Do you have an example of a jsFiddle that demonstrates this problem? There's no parsing happening in the code you're referencing, just a simple property loop. |
|
@rubenv I've actually come here to report the same problem. Seeing this thread, I have tried to produce a jsfiddle example but in the contrived setting of jsFiddle, I failed to do so. However, I can confirm that I have experienced problems with the same portion of code that @pokono has patched. The loadRemote function parameter is an url, but it can be an url that returns something else than JSON - as pokono wrote, it could be an index.html file. In that case the loop doesn't actually loop over object properties, but over indices of a (potentially very long) string. That has exactly happened to me because of a misconfigured proxy, resulting in strangely displayed text captions (concatenated multiple times), eventually leading to a browser crash. As soon as the url was properly configured to serve a json file with translations, the problem disappeared. It wasn't so easy to figure out what's wrong, though. Anyway, sorry for not being able to reproduce it in a test environment, but I hope you'll find this relevant anyway. |
|
@rubenv I'll provide a test case, so you can check it out. I just haven't had the time yet! |
|
@WWuzzy @rubenv I created a JSFiddle to show the issue here: https://jsfiddle.net/pokono/zj8v65f3/17/. Just run it and look at the console, I'm printing everything that is parsed there. I hope this helps. EDIT: I updated the last example with the latest code. |
| @@ -286,8 +286,10 @@ angular.module('gettext').factory('gettextCatalog', function (gettextPlurals, ge | |||
| cache: catalog.cache | |||
| }).then(function (response) { | |||
| var data = response.data; | |||
| for (var lang in data) { | |||
| catalog.setStrings(lang, data[lang]); | |||
| if (typeof data === 'object') { | |||
There was a problem hiding this comment.
I would check the response type tbh
There was a problem hiding this comment.
I hear what you are saying, but in this case the response would be an object and the status would be a 200 too. See the screenshot below.
I am willing to put in more work into this but I need some guidance from you guys.
|
Can you squash the commits? A bit too verbose for a ~1 line change |
|
I will squash the commits when we're getting to the final result that we want. Do you have any idea of why is failing the tests? I was trying to understand but I didn't yet. |
…the content. This avoid parsing data that is not useful, like in situations where pinging a file that doesn’t exist return index.html.
|
Not sure to be honest, but it doesn't look related to your change. o: |
Added data === 'object' to check if data is an object before parsing the content. This avoid parsing data that is not useful, like in situations where pinging a file that doesn’t exist return a index.html file.
If this is not present may happen that the code parse data for minutes. At least this was happening to me.
Any suggestions welcome ;)