[webkit-reviews] review granted: [Bug 24750] [GTK] requests download instead of displaying page : [Attachment 28840] proposed fix

[bugzilla-daemon at webkit.org]

Holger Freyther <zecke at selfish.org> has granted Gustavo Noronha (kov)
<gns at gnome.org>'s request for review:
Bug 24750: [GTK] requests download instead of displaying page
https://bugs.webkit.org/show_bug.cgi?id=24750

Attachment 28840: proposed fix
https://bugs.webkit.org/attachment.cgi?id=28840&action=review

------- Additional Comments from Holger Freyther <zecke at selfish.org>
> -    String contentType = soup_message_headers_get(msg->response_headers,
"Content-Type");
> +    GHashTable *contentTypeParameters = 0;

style, '*' pleaced wrongly.



> +    if (!contentType.isEmpty() && contentType.find(",", 0)) {
> +	   Vector<String> contentTypes;
> +	   contentType.split(',', contentTypes);
> +	   contentType = contentTypes[0];
> +    }

Interesting question. What is faster... searching the string twice or creating
the Vector?


> +
> +    if (contentTypeParameters) {
> +	   GHashTableIter hashTableIter;
> +	   gpointer hashKey;
> +	   gpointer hashValue;
> +
> +	   g_hash_table_iter_init(&hashTableIter, contentTypeParameters);
> +	   while (g_hash_table_iter_next(&hashTableIter, &hashKey, &hashValue))
{
> +	       contentType += "; ";
> +	       contentType += static_cast<char*>(hashKey);
> +	       contentType += "=";
> +	       contentType += static_cast<char*>(hashValue);
> +	   }

You could consider using StringBuilder here. And another thing. With HTTP
headers are like ASCII which makes your use of String(char*) okay, but maybe
write String() around it as well to highlight that we really want this.