Bug 11139 – Content types other than HTML are not handled

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Dex Deacon]

Specifically, WebView::setPolicyDelegate is not implemented.

Any page that isn't HTML is displayed as text by the renderer.  For example, navigating to an image URL will display as garbage.

[Dex Deacon]

Created attachment 10934 [details]
Work in progress for handling mime types

Here's what I have so far.

[Dex Deacon]

Created attachment 11000 [details]
Review candidate

This patch adds support for determining the mime type of content, and passing that to various places.  Navigating to images will display the image rather than garbage.  There is also partial support for policy delegates and IWebPolicyDelegate::decidePolicyForMIMEType().

This also fixes a crash bug in WebFrame::loadDataSource().

[Dex Deacon]

Created attachment 11002 [details]
Proposed patch

Same as before, but without my irrelevant change to TemporaryLinkStubs.cpp.

Also, the crash this fixes is the one mentioned in http://bugs.webkit.org/show_bug.cgi?id=11190 .

[Dex Deacon]

Created attachment 11003 [details]
Proposed patch (fixed)

Resubmitting previous patch, since I had accidentally changed tons of lines in the ChangeLog.

[Dex Deacon]

Created attachment 11026 [details]
Proposed patch

Small tweak to previous patch that fixes behavior if the WebPolicyDelegate chooses to ignore a mime type.

[Maciej Stachowiak]

I will give more detailed feedback later, but my first impression is that this is moving things a bit in the wrong direction. What we're trying to do right now is get the existing mac WebKit loader code to be almost entirely cross-platform, and then it would make upcalls which issue the actual delegate calls in a platform specific way. The code is already moved down to WebCore (it's in loader/mac now) and is now getting turned into C++.

This approach may be ok, but it builds a lot more on the current temporary approach, and once the final code is in place the specific order of the delegate calls and what happens when may change.

[Dave Hyatt]

Careful with the Image::supportsType method... you pick up SVG (which for now at least is incorrect) by just checking for image/*.

[Dex Deacon]

Created attachment 11051 [details]
Proposed patch

Another small tweak to get it to compile with latest changes.

[Dex Deacon]

Created attachment 11074 [details]
Download support added

I added download support to the previous patch (though I didn't obsolete it because this is much bigger, and it may be better to review in 2 chunks).

[Adam Roben (:aroben)]

Comment on attachment 11051 [details]
Proposed patch

Marking obsolete because attachment 11074 [details] is a superset of this patch.

[Oliver Hunt]

Comment on attachment 11074 [details]
Download support added

+bool Image::supportsType(const String& mimeType)
 {
     // FIXME: Implement.
+    if (mimeType.startsWith("image/"))
+        return true;
+    
     return false;
 }

Shouldn't link as Image::supportsType is defined in Image.cpp -- you should be using the mimetyperegistry

which may or may not be implemented in windows -- i wrote it a while back, i can't remember how much (if any) effort i put into making it crossplatform

[Eric Seidel (no email)]

*** Bug 11190 has been marked as a duplicate of this bug. ***