Bug 39492 - QtWebkit fails to load http://upg.de, because MIME type is TEXT/HTML
Summary: QtWebkit fails to load http://upg.de, because MIME type is TEXT/HTML
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL: http://upg.de
Keywords: Qt
Depends on:
Blocks:
 
Reported: 2010-05-21 08:38 PDT by Andrea Diamantini
Modified: 2010-06-14 10:48 PDT (History)
5 users (show)

See Also:


Attachments
Make the MIME type to lower before check for policy (3.34 KB, patch)
2010-05-24 21:28 PDT, Charles Wei
staikos: review-
Details | Formatted Diff | Diff
patch_v2 (4.40 KB, patch)
2010-06-07 23:21 PDT, Charles Wei
eric: review-
Details | Formatted Diff | Diff
removing the redundent test code (3.88 KB, patch)
2010-06-13 02:28 PDT, Charles Wei
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrea Diamantini 2010-05-21 08:38:06 PDT
I tried loading this page with rekonq & arora. They basically have the same behavior trying to download the page instead of showing it. Further investigation shows that the site comes with an upper case mimetype ( "TEXT/HTML; CHARSET=ISO-8859-1" ) that is unknown to QtWebkit :)
Comment 1 Charles Wei 2010-05-24 21:28:09 PDT
Created attachment 56969 [details]
Make the MIME type to lower before check for policy

with the Qt porting,  make the MIME type lower before checking the policy for it , this fixes the problem. Another way to fix it is to add the MIME type in upper case to the MIME type repository , but that's much more work than this.
Comment 2 Charles Wei 2010-06-07 18:09:11 PDT
As a patch submitter,  I am expecting an explanation from the reviewer to deny the patch. I am frustrated the reviewer denied it without a reason.
Comment 3 Charles Wei 2010-06-07 19:00:59 PDT
I am raising the review flag to get the attention of the reviewer,  and please comment and suggest if denied.
Comment 4 George Staikos 2010-06-07 19:51:15 PDT
Comment on attachment 56969 [details]
Make the MIME type to lower before check for policy

You should make a copy of the string, if this happens to be correct.  const_cast is not correct.
Comment 5 Charles Wei 2010-06-07 23:21:15 PDT
Created attachment 58113 [details]
patch_v2

Addressing the comments by the reviewer,  to make a local copy of the MIME type string instead of const_cast the copy of the input MIMEType parameter.
Comment 6 Eric Seidel (no email) 2010-06-12 20:52:47 PDT
Comment on attachment 58113 [details]
patch_v2

MIMEType as an argument name violates the style guidelines.  We shoudl fix that to mimeType while here.  Otherwise looks fine.

I'm slightly surprised this isn't already tested, but I'm glad you added a test.

The tests appear to be copied twice.  r- for that at least.
Comment 7 Charles Wei 2010-06-13 02:28:38 PDT
Created attachment 58585 [details]
removing the redundent test code

removing the duplicated test code.
Comment 8 George Staikos 2010-06-13 20:44:50 PDT
Comment on attachment 58585 [details]
removing the redundent test code

Looks corrected to me.  The argument name can be corrected as a separate style-fixing patch.  This one doesn't add invalid style.
Comment 9 WebKit Commit Bot 2010-06-13 23:32:12 PDT
Comment on attachment 58585 [details]
removing the redundent test code

Clearing flags on attachment: 58585

Committed r61110: <http://trac.webkit.org/changeset/61110>
Comment 10 WebKit Commit Bot 2010-06-13 23:32:18 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Darin Adler 2010-06-13 23:35:04 PDT
Does not seem right to call makeLower here. The functions should be checking without being case sensitive. The test is great, but the fix seems wrong.
Comment 12 George Staikos 2010-06-14 06:16:01 PDT
I agree the fix is suboptimal, but I think it's still correct.  Let's fix the bug, optimize after.
Comment 13 Darin Adler 2010-06-14 10:48:20 PDT
My putting makeLower here we fix one case, but leave broken any other code paths using these functions.