Bug 39492 – QtWebkit fails to load http://upg.de, because MIME type is TEXT/HTML

• Component: WebKit Qt
• Status: RESOLVED FIXED

[Andrea Diamantini]

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 :)

[Charles Wei]

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]

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]

I am raising the review flag to get the attention of the reviewer,  and please comment and suggest if denied.

[George Staikos]

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]

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 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]

Created attachment 58585 [details]
removing the redundent test code

removing the duplicated test code.

[George Staikos]

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 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]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

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]

I agree the fix is suboptimal, but I think it's still correct.  Let's fix the bug, optimize after.

[Darin Adler]

My putting makeLower here we fix one case, but leave broken any other code paths using these functions.