Bug Tracker

Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#8999 closed bug (wontfix)

ajax bug in Opera 11

Reported by: Gorets Owned by:
Priority: low Milestone: 1.next
Component: ajax Version: 1.6rc1
Keywords: Cc: jaubourg
Blocked by: Blocking:

Description

http://katalog.lg.ua/jquery_bug/index.html

$.post("2.php", { notice: notice } , function( data ){ $('#send_result').append(' ' + data ); });

2.php <?php

@header("Content-type: text/plain; charset=windows-1251");

$buffer = "<font color=\"green\">THIS HTML TEXT</font>";

echo $buffer;

?>

If the response is HTML tag and set header Opera 11 returns [object XMLDocument]

Change History (12)

comment:1 Changed 8 years ago by Rick Waldron

Cc: jaubourg added
Component: unfiledajax

On initial review, it appears that the response is being "detected" as xml. I'm going to cc @jaubourg on this so he can weigh in.

comment:2 Changed 8 years ago by miketaylr

Funky, going to have one of our engineers check it out.

comment:3 Changed 8 years ago by timmywil

Priority: undecidedlow
Status: newopen

comment:4 Changed 8 years ago by hallvord@…

Opera applies some content sniffing for text/plain responses, thinks this is somewhat parse-able, and since the XML parser can successfully parse it, we expose the resulting document in xhr.responseXML

jQuery contains some rather convoluted logic for determining how to process XHR responses (see method ajaxHandleResponses()). There is for example this:

 if ( ct ) { 
 for ( type in contents ) { 
 if ( contents[ type ] && contents[ type ].test( ct ) ) { 
 dataTypes.unshift( type ); 
 break; 
 } 
 } 
 }

which seems to handle four types jQuery knows by default - html, json, JS and xml (notably not plain text). The script has not given jQuery any "expected" data type or defined any conversion functions. Hence, in this:

if ( !dataTypes[ 0 ] || s.converters[ type + " " + dataTypes[0] ] )

the !dataTypes[0] part will always be true, and jQuery will randomly default to the first type the "for ( type in responses ) {" enumeration returns, which happens to be 'xml' in our case. This is predictable because of the order in the callback method:

 // Construct response list 
 if ( xml && xml.documentElement /* #4958 */ ) { 
 responses.xml = xml; 
 } 
 responses.text = xhr.responseText; 

Moving the responses.text line above the xml stuff fixes this bug.

(Alternative fixes: simplify jQuery's overcomplicated and non-jQueryish AJAX, or Opera avoiding content sniffing for text/plain XHR responses.)

comment:5 in reply to:  4 ; Changed 8 years ago by jaubourg

Replying to hallvord@…:

Opera applies some content sniffing for text/plain responses, thinks this is somewhat parse-able, and since the XML parser can successfully parse it, we expose the resulting document in xhr.responseXML

Thank you for stating the obvious. What is the purpose of this, knowing it's dead-easy for JavaScript code to parse the document itself if needs be? Also, I guess checking for "<?xml" was too much of a hassle?

jQuery contains some rather convoluted logic for determining how to process XHR responses (see method ajaxHandleResponses()). There is for example this:

 if ( ct ) { 
 for ( type in contents ) { 
 if ( contents[ type ] && contents[ type ].test( ct ) ) { 
 dataTypes.unshift( type ); 
 break; 
 } 
 } 
 }

which seems to handle four types jQuery knows by default - html, json, JS and xml (notably not plain text).

Yes, plain text is the default. If you wanted to add another dataType (like css), in ajaxSettings.contents, you'd be glad not to have to deal with a text dataType tested BEFORE it. Never used a switch statement or heard of the "default" keyword?

The script has not given jQuery any "expected" data type or defined any conversion functions. Hence, in this:

if ( !dataTypes[ 0 ] || s.converters[ type + " " + dataTypes[0] ] )

the !dataTypes[0] part will always be true, and jQuery will randomly default to the first type the "for ( type in responses ) {" enumeration returns, which happens to be 'xml' in our case. This is predictable because of the order in the callback method:

 // Construct response list 
 if ( xml && xml.documentElement /* #4958 */ ) { 
 responses.xml = xml; 
 } 
 responses.text = xhr.responseText; 

Moving the responses.text line above the xml stuff fixes this bug.

Gotta love the use of "randomly" and "predictable" for the same piece of code.

You apparently decided to ignore that, sometimes, a Transport cannot provide a Content-Type. This "random", yet "predicatble", behaviour allows a Transport to specify the most "advanced" data structure it can first as a default without specifying a Content-Type (when not able: think local files which are probably the reason why Opera is "sniffing" in the first place... and that would be fine if limited to them).

So Transports do send back the most "advanced" format available first which is then chosen by default if no dataType is specified upfront in the ajax options and when no recognized Content-Type is provided with the response.

Interestingly, that's somehow what Opera is trying to do itself by "sniffing" XML... problem is it does so ignoring Content-Type entirely if provided... does Opera at least check the Accept request header? I seriously doubt it.

Well, I guess headers are irrelevant to Opera now.

Alternative fixes: simplify jQuery's overcomplicated and non-jQueryish AJAX

Yeah, jQuery's AJAX is complex because, you know, we love complexity and pointless features. That's what we do all day long: plot the most insane API possible to confuse the hell out of our users.

In fact, our primary goal is to make jQuery as non-jQueryish as possible!

Are you on drugs?

or Opera avoiding content sniffing for text/plain XHR responses.

At last! Thank you!

Key here is that Opera's xhr trying too hard will break any library relying on this very simple principle: text/plain is plain text and MUST NOT be sniffed/parsed.

This "feature" will break 1.4.x which doesn't contain any of the code you so valiantly "reviewed" here... strange isn't it? I'll be so bold as to guess it'll break any library relying on proper xhr implementation.

Since you used "we" in your first paragraph, I did some digging and I assume you're Hallvord R. M. Steen from Opera's customer service department: http://my.opera.com/hallvors/blog/

Way to enhance relationships between browser and JavaScript devs, pal! If I ever had any confidence we could have some kind of decent dialog going on, rest assure I now stand corrected.

Apparently, you decide to ignore the real issue on Opera's end in favour of a quick and dirty patch in jQuery (which won't solve the issue in earlier versions). Why do I have the feeling this will turn into some nonsensical arm-wrestle?

So be it and let's not mince words like you failed to do in your comment: this IS a regression in Opera, plain and simple. It breaks jQuery 1.4.x (most probably earlier versions too, not to mention other libs). Blaming jQuery 1.5.x for this is laughable at best, completely misled and rather pointless for sure... but, since it has been rewritten recently (and not to your liking it seems), let's beat the dead horse, shall we?

Please, review the points made in this post. No, really, READ them, UNDERSTAND them and cut the preconceptions. Thank you.

comment:6 in reply to:  5 Changed 8 years ago by miketaylr

Replying to jaubourg:

Replying to hallvord@…: Since you used "we" in your first paragraph, I did some digging and I assume you're Hallvord R. M. Steen from Opera's customer service department: http://my.opera.com/hallvors/blog/

Way to enhance relationships between browser and JavaScript devs, pal! If I ever had any confidence we could have some kind of decent dialog going on, rest assure I now stand corrected.

Jaubourg, I think if you cut the sarcasm a decent dialog _will_ be possible.

comment:7 Changed 8 years ago by anonymous

This problem only in jQuery >= 1.5

comment:8 in reply to:  5 ; Changed 8 years ago by hallvord@…

Replying to jaubourg:

Hi jaubourg,

it seems some apologies are due. I didn't realise it would come across as such an offensive rant because I stated that I consider this particular method a complicated and non-jQuery-ish part of jQuery. (I believe that the rest of my comment was mostly technical and useful analysis, and that if I had omitted the "non-jQuery-ish" part you might have read it as such - what do you think?)

So, I'm honestly sorry for having offended you. I still think this part of the jQuery API is a bit complicated to understand, but I'll try to get my head around it so we can get that dialogue going ;-) I've read http://api.jquery.com/extending-ajax/ but it's a bit too high-level, so please bear with me for a couple of silly questions below.

By the way, while posting the above comment yesterday I also confirmed the corresponding Opera bug report, argued that it should get some priority because we break jQuery, and added tests to check that we drop content-sniffing for text/plain when it is requested by XHR. (That we do sniff is probably merely an accidental side effect of the fact that browsers need to do content-sniffing for text/plain when you try to load a regular page). I shold have made it a lot clearer yesterday that we intend to fix this.

Some questions in response to your comment - I hope you won't mind:

1) You say "plain text is the default", but also "Transports do send back the most 'advanced' format available". We're looking at a case where the server sent text/plain, but jQuery doesn't acknowledge that text/plain header (because you actively look for four other types but not text/plain), and prefers returning responseXML if the browser makes it available. I understand by the above statement about 'most advanced format' that this is intentional. Right? Then I don't quite understand what you mean by plain text being the default, but it might not matter.

2) Explaining why my suggested fix isn't usable, you say "sometimes, a Transport cannot provide a Content-Type". While I'm afraid I still don't fully understand jQuery's "Transport" API and terminology, I assume you are talking about the case where the server returns no Content-Type header at all, and the problem is that in this case jQuery still wants to return an XML document (being the 'most advanced format') if the response is, in fact, XML. So in this case you want to rely on the UA's content sniffing. Correct?

3) Given that you don't want to reverse the .text and .xml assignments, in order to use UA content sniffing for non-labelled XML, is there a simple way you can recognise the text/plain header and return just responseText? If not, I suggest you close this bug and wait for the fix from our side.

Last edited 8 years ago by hallvord@… (previous) (diff)

comment:9 in reply to:  8 ; Changed 8 years ago by jaubourg

Hey Hallvord,

Well, I think I over-heated a little. It's quite clear you had the best intentions in mind :(

The fault is definitely mine and mike tried to explain it to me on IRC though, at that time, I wasn't receptive enough. Thing is, I'm getting a bit jaded due to some other browser bugs "discussions" that invariably ended up with devs getting on their high horses (which is what the arm-wrestle reference was all about)... sadly, that's the posture *I* took here, ironically, and I'm sorry about that.

The non-jQuery-ish reference, together with the random/predictable thingy had me completely ignore the rest of the post. So you're right my own preconceptions derailed this discussion.

So let's get to your questions. I just hope I'll be clear enough in my answers because, yes, Transports can get a bit involved at times.

I'll try to give some general context regarding dataType auto-detection in jQuery:

So, a Transport will return a dataType to data map back to ajax. That way, a Transport can provide several alternative "views" of the response, which is exactly what an XHR implements (text and xml). If no expected dataType was given, ajax will then look at the Content-Type header to try and determine what the actual dataType is, failing that it will rely on the Transport giving the best possible dataType (first one in the response map).

You're rightfully puzzled as to why ajax does not try and recognize text. So, let's consider if we added { text: /text/ } at the end of the contents option in ajaxSettings (as a catchall "default"): the problem is solved but at the price of extensibility. I'll explain why.

Let's say you wanna handle css automagically in ajax. You provide the following (untested and simplified) converter:

$.ajaxSetup({
    converters: {
	   "text css": function( text ) {
           $( "<style/>" ).text( text ).appendTo( $("head") );
           return text;
       }
    }
});

You can already do requests as follows:

$.ajax( url, { dataType: "css" } );

and ajax will create style elements for you under the hood.

If you want to make it complete, you'll provide a regexp for css in the contents option so that ajax can auto-detect css:

$.ajaxSetup({
    contents: {
        css: /css/
    }
});

Problem is, if you have { text: /text/ } already in there, the css will never be auto-detected... and as far-fetched as the example may seem to a fresh pair of eyes, this is exactly how the script dataType works ( https://github.com/jquery/jquery/blob/master/src/ajax/script.js#L4 ) and is how ajax will handle dataType auto-detection.

In all honesty, I would have preferred a tree-like structure to handle dataType detection (to handle sub-dialects like rss feeds and such) but, even if I haven't given up on the idea yet, I thought the overall system was already complex enough as it was. Thing is, auto-detection is made in order and it means a "catch-all" text detection is simply not an option. Note that the auto-detection system is very lightly documented because I do intend to get back to it (but mime-types as consistent as text/javascript and application/javascript make for quite a challenging "tree" structure, especially from an extension point of view).

So, to answer question 1, if the xhr does not try and parse content when the Content-Type is not XML, the map only contains a text response, ajax does not recognize the Content-Type as XML either, so it will rely on the transport to provide the best possible dataType (the first in the map): here "text". That's why I use the term default for text. If and when an XHR implementation ignores a valid Content-Type and tries and parse it as XML anyway, then the only option for jQuery would be to list all known Content-Type values as a mean to circumvent the problem, with the "non-extensible" caveat above. A tree structure for auto-detection would mitigate the issue, that's for sure.

As for 2, you're perfectly correct. AFAIK, no existing XHR implementation tries and emulates HTTP over file://, which means you always have a status 0 and no headers (so no Content-Type). I find it puzzling and even a bit counter-intuitive. I'm still under the impression it would be quite easy for a browser to emulate HTTP over file:// with status 200, 404, 50x (for security errors) and 304 (files do have a Last Modified date). The OS is potentially able to "guess" the mimetype of a file so the Content-Type header could be provided too. But anyway, we deal with this (status 0 being the worse of it seeing as, in HTTP requests it means a network error or an abort) but fact is we don't have a Content-Type and we have to (as opposed to want to, really ;)) rely on some XHR content sniffing in that particular instance.

The rule of thumb would go like this: if a Content-Type header is provided, rely on it and don't sniff, if there is no Content-Type header at all, sniff.

Why I talk about Transports in general is because you could have, as an example, an image Transport (that uses an Image object to request the image) which would give ajax a response map akin to { img: ImageObject } with no headers. In that instance, it's quite obvious the default is "img", not "text". The Transport could fake a Content-Type header but the Image object doesn't differentiate between gif, jpeg, png, whatever, so it would be a bit pointless in the end.

Now to question 3, we could hardcode a check for text/plain after the loop on options.contents. The real issue here is to know if this would be enough. Should we check for /text/? What if the Content-Type is something different entirely like "my/content"? Will Opera try and parse it as xml and what decision should jQuery take if this was the case?

Anyway, I hope this (long) answer will make things clearer and thank you for the interest you take in this issue.

comment:10 in reply to:  9 ; Changed 8 years ago by hallvord@…

Replying to jaubourg:

Hi jauborg,

thanks for a long and detailed reply. No further apologies needed. To be honest, it's not my first brush with debugging issues with Transports and websites that perhaps don't configure their Ajax calls perfectly, so perhaps I had some built-up frustrations there.. Thanks to this discussion, I'll probably spend significantly less time debugging this from now ;-).

Internally, we've decided to fast-track a fix even though we know no real life site being broken at the moment - if we break jQuery's assumptions it's just going to be a matter of time before a major site chokes and goes belly-up in Opera. I don't have a real ETA (as it's not even fixed yet, and discussions surrounding an older bug in the same area indicates it might be harder than you think) but I would be slightly surprised if it didn't make 11.50.

Because this report is filed for a simple demo and we don't know a broken site, it's not super high pri for us to make you change anything either ;-). Worst case, we could always use browser.js to add a custom hack nullifying responseXML if it becomes an issue on a big site.

Now to question 3, we could hardcode a check for text/plain after the loop on options.contents. The real issue here is to know if this would be enough. Should we check for /text/? What if the Content-Type is something different entirely like "my/content"? Will Opera try and parse it as xml and what decision should jQuery take if this was the case?

Seems Opera tries if it parses as XML no matter what. I would hope that people specifying entirely custom content-types would also specify what they want in the ajax call, and special-casing text/plain would make sense to me personally, but I understand that's it's complicated and you're reluctant to change it.

comment:11 in reply to:  10 Changed 8 years ago by jaubourg

Resolution: wontfix
Status: openclosed

To be honest, it's not my first brush with debugging issues with Transports and websites that perhaps don't configure their Ajax calls perfectly, so perhaps I had some built-up frustrations there..

Drop me a mail. I'd love to hear about these problems you encountered and see if there are stuff to be ironed out jQuery side.

I'll close this as won't fix for now, waiting for the fix your side and thanks again for having looked deeply and quickly into this.

Last edited 8 years ago by jaubourg (previous) (diff)

comment:12 Changed 7 years ago by miketaylr

This is fixed internally in Opera now.

Note: See TracTickets for help on using tickets.