RESOLVED FIXED Bug 39492
QtWebkit fails to load http://upg.de, because MIME type is TEXT/HTML
https://bugs.webkit.org/show_bug.cgi?id=39492
Summary QtWebkit fails to load http://upg.de, because MIME type is TEXT/HTML
Andrea Diamantini
Reported 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 :)
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-
patch_v2 (4.40 KB, patch)
2010-06-07 23:21 PDT, Charles Wei
eric: review-
removing the redundent test code (3.88 KB, patch)
2010-06-13 02:28 PDT, Charles Wei
no flags
Charles Wei
Comment 1 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.
Charles Wei
Comment 2 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.
Charles Wei
Comment 3 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.
George Staikos
Comment 4 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.
Charles Wei
Comment 5 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.
Eric Seidel (no email)
Comment 6 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.
Charles Wei
Comment 7 2010-06-13 02:28:38 PDT
Created attachment 58585 [details] removing the redundent test code removing the duplicated test code.
George Staikos
Comment 8 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.
WebKit Commit Bot
Comment 9 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>
WebKit Commit Bot
Comment 10 2010-06-13 23:32:18 PDT
All reviewed patches have been landed. Closing bug.
Darin Adler
Comment 11 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.
George Staikos
Comment 12 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.
Darin Adler
Comment 13 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.
Note You need to log in before you can comment on or make changes to this bug.