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

When response is mime-type application/json content not captured properly. #43

Closed
jamesrusso opened this issue Jun 11, 2013 · 16 comments
Closed

Comments

@jamesrusso
Copy link

When the upload response is actually a JSON document (with mime type) in Chrome it seems to wrap the content in a <pre> tag which is messing up the content capturing.

@mjquinlan2000
Copy link

It seems that Chrome does some weird things with the iframe component of this directive. If you change line 67 of ng-upload.js to:

    var content = $.parseJSON(iframe.contents().find('body').text());

and change line 73 to:

    if (content != null) { // Fixes a bug in Google Chrome that dispose the iframe before content is ready.

Then it parses the JSON correctly. Also, I had some problems returning JSON to IE (typical) and it turns out that Microsoft has a hard time recognizing application/json as a MIME type, so rendering as text/plain works with this solution as well. Hope that helps.

@twilson63
Copy link
Owner

Hey @mjquinlan2000,

Yea, the main problem is IE, but I just tried incorporating your changes and it breaks the first example. If your changes can be applied without breaking the examples, I will be happy to incorporate them.

Thanks

Tom

yesnault added a commit to yesnault/ngUpload that referenced this issue Jun 18, 2013
@yesnault
Copy link

This is my fix (I work with JSON only) yesnault@0412f06

but why don't let the user choose between .text() and .html() ?

@mjquinlan2000
Copy link

@twilson63 Thanks for the heads up. Much like @yesnault , I deal mainly in JSON responses as well and did not consider the case where the server might actually intend to serve up a plain text response. I've modified the code to handle both scenarios. Seems to work fine in my application as well as your examples:

var content = iframe.contents().find('body').text()
// Attempt to parse JSON response, else return text response
try {
    content = $.parseJSON(iframe.contents().find('body').text());
}catch (e){
    console.log("WARN: XHR response is not valid JSON: '" + content + "'");
}

Then of course you have to check that content is neither null or an empty string after it is processed.

Hopefully that helps

@Siyfion
Copy link
Contributor

Siyfion commented Jun 20, 2013

Is this a bug that's been introduced recently into ngUpload, or in a patch to Google Chrome? Either way, getting the fix into a build & on Bower would be very much appreciated. 😄

@twilson63
Copy link
Owner

@Siyfion ,

I am not sure what could have changed to introduce this bug, but I will take a look at it.

Thanks

Tom

@Siyfion
Copy link
Contributor

Siyfion commented Jun 20, 2013

@twilson63 I wonder whether it could have been an update to Google Chrome that's caused it, still.. v. odd.

@twilson63
Copy link
Owner

Hey Guys,

Found it!

Here is the issue:

69974d4#ng-upload.js

var content = iframe.contents().find('body').text();

was changed to

var content = iframe.contents().find('body').html();

which was submitted as a PR by @cristianocd, I think he was consuming images from the response not json.

So when we changed the iframe.body to .html() it throws the <pre> tag in the returned result vs using .text() which returns the correct json string.

I can either revert it back or look at adding some sort of attribute to specify the returning type. One thing to note is that IE will not accept application/json as a return type, so an alternate solution is to ask your server to return text/html or text/plain, which will not attach the <pre>, regardless, we still need to fix this issue, any thoughts on the best approach to handle both cases?

maybe a return type attribute that specifies the mime/type, if image/* then .html(), and if type/html or type/text the .html() if application/json - then .text() and perform a json parse so you get it back as js?

Thoughts?

Thanks

Tom

@Siyfion
Copy link
Contributor

Siyfion commented Jun 20, 2013

Well I would imagine that the majority of people are probably returning JSON, so I think that would probably be the best "default" case.

However, if you're saying that IE wont work with the content header set to application/json, then perhaps we need to add to documentation to the front screen to ensure that JSON is returned as text/plain instead. Perhaps take a good look at what @mjquinlan2000 was suggesting?

@twilson63
Copy link
Owner

Ok,

I just bumped to version 0.3.8, this should fix the json problem as well as support any folks sending text or other stuff. Let me know if you continue to have issues or other suggestions.

Thanks again for using ngUpload and helping improve it!

Thanks

Tom

@Siyfion
Copy link
Contributor

Siyfion commented Jun 20, 2013

I can happily confirm that this fixes my issue. Might be worth getting everyone else to check their non-JSON stuff works before giving it the 👍 entirely.

@cristianocd
Copy link
Contributor

Working 100%. Thanks!

@mariussoutier
Copy link

With JSON it works, but a normal response is still wrapped in a "pre" tag. Is this working as intended?

@e-oz
Copy link

e-oz commented Aug 7, 2013

Bug still exists.
On pretty legal Content-Type:application/json I receive

Resource interpreted as Document but transferred with MIME type application/json: "url_here".
WARN: XHR response is not valid json

Chrome 28.0.1500.95 m

@e-oz
Copy link

e-oz commented Aug 7, 2013

As a workaround, set Content-type to text/html in response.

@adamshaylor
Copy link

I too am still seeing content wrapped in <pre> tags. @Jamm’s solution works, although the response has to be run through JSON.parse() and ngUpload logs this message:

WARN: XHR response is not valid json

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

No branches or pull requests

9 participants