Bug 24349

Summary: [Qt] HTTP status text is never set
Product: WebKit Reporter: Adam Bergkvist <adam.bergkvist>
Component: WebKit QtAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, zecke
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
patch ap: review+

Description Adam Bergkvist 2009-03-04 06:11:44 PST
See related bug: https://bugs.webkit.org/show_bug.cgi?id=3547
Comment 1 Adam Bergkvist 2009-03-04 06:24:17 PST
Created attachment 28263 [details]
patch
Comment 2 Alexey Proskuryakov 2009-03-05 04:54:15 PST
Comment on attachment 28263 [details]
patch

r=me
Comment 3 Holger Freyther 2009-03-07 05:08:50 PST
Comment on attachment 28263 [details]
patch


> +        response.setHTTPStatusText(m_reply->attribute(QNetworkRequest::HttpReasonPhraseAttribute).toByteArray().constData());

this is weird...
QString -> QByteArray -> constData -> String when we have a QString -> String already...
Comment 4 Alexey Proskuryakov 2009-03-09 01:16:12 PDT
I'm reluctant to make any changes, as I don't have a Qt build, but please feel free to tweak and land.
Comment 5 Adam Bergkvist 2009-03-09 02:31:05 PDT
(In reply to comment #3)
> (From update of attachment 28263 [details] [review])
> 
> > +        response.setHTTPStatusText(m_reply->attribute(QNetworkRequest::HttpReasonPhraseAttribute).toByteArray().constData());
> 
> this is weird...
> QString -> QByteArray -> constData -> String when we have a QString -> String
> already...
> 

m_reply->attribute(QNetworkRequest::HttpReasonPhraseAttribute) is a QVariant::ByteArray. We could do a toString() to construct a QString, but that would cause an extra copy of the character data.
Comment 6 Holger Freyther 2009-03-09 02:41:07 PDT
I only looked at the Qt site of things and:

grep -rn HttpReasonPhraseAttribute *

qnetworkaccesscachebackend.cpp:81:    setAttribute(QNetworkRequest::HttpReasonPhraseAttribute, attributes.value(QNetworkRequest::HttpReasonPhraseAttribute));
qnetworkaccesshttpbackend.cpp:772:    setAttribute(QNetworkRequest::HttpReasonPhraseAttribute, httpReply->reasonPhrase());
qnetworkaccesshttpbackend.cpp:890:    setAttribute(QNetworkRequest::HttpReasonPhraseAttribute, attributes.value(QNetworkRequest::HttpReasonPhraseAttribute));
qnetworkaccesshttpbackend.cpp:1039:        attributes.insert(QNetworkRequest::HttpReasonPhraseAttribute, httpReply->reasonPhrase());
qnetworkrequest.cpp:116:    \value HttpReasonPhraseAttribute
qnetworkrequest.h:67:        HttpReasonPhraseAttribute,


and reasonPhrase() is a QString coming from:

qhttp.h:    QString reasonPhrase() const;
qhttpnetworkconnection_p.h:    QString reasonPhrase() const;


so what am I missing?
Comment 7 Adam Bergkvist 2009-03-09 03:24:30 PDT
Strange... considering that the documentation states that it is a ByteArray. A ByteArray would make sense since the reason phrase can only contain ascii characters.

http://doc.trolltech.com/main-snapshot/qnetworkrequest.html#Attribute-enum
Comment 8 Holger Freyther 2009-03-13 03:04:50 PDT
(In reply to comment #7)

> http://doc.trolltech.com/main-snapshot/qnetworkrequest.html#Attribute-enum

Okay, we should stick with the documentation here.

Comment 9 Alexey Proskuryakov 2009-03-13 03:06:26 PDT
Committed revision 41664.