Bug 3547 - XMLHttpRequest.statusText returns always "OK"
: XMLHttpRequest.statusText returns always "OK"
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: XML
: 412
: All All
: P2 Normal
Assigned To: Nobody
: InRadar, ReviewedForRadar
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-06-15 11:26 PDT by Peter Speck
Modified: 2009-03-13 05:58 PDT (History)
5 users (show)

See Also:


Attachments
patch (1.28 KB, patch)
2009-02-27 07:33 PST, Adam Bergkvist
no flags Details | Formatted Diff | Diff
patch (3.50 KB, patch)
2009-03-02 07:10 PST, Adam Bergkvist
no flags Details | Formatted Diff | Diff
patch (4.14 KB, patch)
2009-03-02 07:43 PST, Adam Bergkvist
ap: review-
Details | Formatted Diff | Diff
patch (2.98 KB, patch)
2009-03-03 08:59 PST, Adam Bergkvist
no flags Details | Formatted Diff | Diff
patch (5.14 KB, patch)
2009-03-04 07:13 PST, Adam Bergkvist
no flags Details | Formatted Diff | Diff
patch (3.93 KB, patch)
2009-03-05 09:01 PST, Adam Bergkvist
ap: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Speck 2005-06-15 11:26:48 PDT
When the webserver returns "500 Invalid dumpType arg = 'x'", XMLHttpRequest.statusText still returns 
"OK".

XMLHttpRequest.status returns the correct value.

tcpdump:
20:16:53.966539 IP localhost.commplex-main > localhost.55191: P 2:415(413) ack 464 win 65535 
<nop,nop,timestamp 1638750388 1638750388>
E...g.@.@...............M"..7U}v...........
a.X.a.X.TTP/1.0 500 Invalid dumpType arg = 'x'
Date: Wed, 15 Jun 2005 18:16:53 GMT
Server: DanCenterGraphicsServer/5.2.3
Connection: close
MIME-version: 1.0
X-ErrorMessage: Intern fejl i katalogserveren: Invalid dumpType arg = 'x'
Content-type: text/html
Comment 1 Peter Speck 2005-06-15 11:40:54 PDT
For testing the bug, use
http://jjj.dancenter.com/pubweb/safaritest?t=1
and something like sudo tcpdump -s0 -A -i en0 host jjj.dancenter.com
Comment 2 Anders Carlsson 2005-06-16 01:28:34 PDT
Looking at the NSHTTPURLResponse class, there's no way to get the status text in a response, it's only 
possible to get a general localized status text given a code, so this is a problem with CoreFoundation right 
now. 

Is it possible to add a getter for this to WebKitSystemInterface?
Comment 3 Mark Rowe (bdash) 2007-08-27 01:54:22 PDT
<rdar://problem/5439321>
Comment 4 Joran 2008-12-09 23:22:20 PST
Firefox is already doing it right apparently. Shockingly still not fixed for WebKit! Come on guys!
Comment 5 Adam Bergkvist 2009-02-27 07:33:52 PST
Created attachment 28071 [details]
patch

Return status text if available.
Comment 6 Alexey Proskuryakov 2009-02-27 08:20:07 PST
What platforms does this fix the bug for? Unfortunately, on Mac OS X, httpStatusText() always returns "OK".
Comment 7 Adam Bergkvist 2009-02-27 13:05:00 PST
(In reply to comment #6)
GTK WebKit (soup)
https://bugs.webkit.org/show_bug.cgi?id=24226

Comment 8 Peter Speck 2009-02-27 20:21:40 PST
Shouldn't the patch remove the FIXME as this code no longer needs to be fixed  (but only CF on OSX)?
Comment 9 Alexey Proskuryakov 2009-02-27 23:47:04 PST
Comment on attachment 28071 [details]
patch

     // FIXME: <http://bugs.webkit.org/show_bug.cgi?id=3547> XMLHttpRequest.statusText returns always "OK".

Yes, this FIXME should be removed now, and new bugs should be filed for platforms that have this problem with ResourceResponse. Note that this bug is categorized as Macintosh/10.4, which means that the reporter needed it fixed on Mac - but I think that it's OK to make a new bug tracking the Mac fix.

     if (m_response.httpStatusCode())
-        return "OK";
+        if (!m_response.httpStatusText().isEmpty())
+            return m_response.httpStatusText();
+        else
+            return "OK";

At least Mac implementation always fills the response status text with OK. It would be better to ensure that all implementations do this, and to remove the isEmpty() check.
Comment 10 Adam Bergkvist 2009-03-02 07:10:04 PST
Created attachment 28171 [details]
patch

Updated patch with test.
Comment 11 Adam Bergkvist 2009-03-02 07:43:35 PST
Created attachment 28177 [details]
patch

Sorry, forgot to add the expected results file.
Comment 12 Alexey Proskuryakov 2009-03-02 08:03:58 PST
Comment on attachment 28177 [details]
patch

This is a good test, but it is not needed. There are at least two existing tests that should change from FAIL to PASS with this test (http/tests/xmlhttprequest/web-apps/012.html and 013.html). Instead of adding a new test, please add platform-specific results for these ones, and please do run all http tests. I believe that these tests should pass at least on gtk and windows (CF) now - it may make sense to change default results to expected ones, and to add platform-specific ones for platforms that don't pass yet.

It seems that QNetworkReplyHandler needs to set a fake httpStatusText, too.
Comment 13 Adam Bergkvist 2009-03-03 08:59:50 PST
Created attachment 28222 [details]
patch

You are right. This patch contains new expected results for web-apps/012 web-apps/013 for GTK.
Comment 14 Alexey Proskuryakov 2009-03-03 09:56:36 PST
> You are right. This patch contains new expected results for web-apps/012
> web-apps/013 for GTK.

Windows/CF also has this member initialized correctly, so it should have a passing result now, too.

What about my other comment (concerning QNetworkReplyHandler)?
Comment 15 Adam Bergkvist 2009-03-04 07:13:51 PST
Created attachment 28264 [details]
patch

I have prepared a patch to correctly set the status text on QT (See https://bugs.webkit.org/show_bug.cgi?id=24349).

This updated patch includes platform specific expected results for GTK, QT, and Win. Perhaps the default expected result should be PASS instead and platform specific expected results added for platforms that set a fake status text?
Comment 16 Alexey Proskuryakov 2009-03-04 09:44:05 PST
(In reply to comment #15)
> Perhaps the default expected result should be PASS instead and platform
> specific expected results added for platforms that set a fake status text?

Yes, that makes sense.
Comment 17 Adam Bergkvist 2009-03-05 09:01:41 PST
Created attachment 28306 [details]
patch

Patch changes default expected results for tests 012 and 013 (http/tests/xmlhttprequest/web-apps/) to PASS and adds platform specific expected results for mac.

Modified XMLHttpRequest::statusText to return the status text if available regardless of the state. (Same behavior as XMLHttpRequest::status.)
Comment 18 Alexey Proskuryakov 2009-03-05 09:21:38 PST
(In reply to comment #17)
> Modified XMLHttpRequest::statusText to return the status text if available
> regardless of the state. (Same behavior as XMLHttpRequest::status.)

This will change the behavior on Mac - "OK" will be always returned, even in cases that raised an exception before. Checking by status code should be more straightforward, in my opinion.
Comment 19 Adam Bergkvist 2009-03-09 02:46:31 PDT
(In reply to comment #18)
> This will change the behavior on Mac - "OK" will be always returned, even in
> cases that raised an exception before. Checking by status code should be more
> straightforward, in my opinion.

In my opinion it is more correct to check the variable that is actually returned than a related value. If mac sets the fake status text where the real value should be set, I can not see how that would change the behavior.
Comment 20 Alexey Proskuryakov 2009-03-13 04:04:21 PDT
Comment on attachment 28306 [details]
patch

Yes, seems fine indeed. r=me
Comment 21 Alexey Proskuryakov 2009-03-13 04:11:25 PDT
Committed <http://trac.webkit.org/changeset/41665>.

Filed bug 24572 to track a Mac fix.
Comment 22 Alexey Proskuryakov 2009-03-13 05:58:11 PDT
This actually did change other results a little, see <http://trac.webkit.org/changeset/41666>. But the change was a progression.