Bug 124652 – [GTK] Add API to WebKitResponsePolicyDecision to check if the MIME type can be shown

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Carlos Garcia Campos]

The decidePolicyForResponse callback in the WKPagePolicyClient now receives a boolean parameter canShowMIMEType. We could expose it in our API to make it easier and more efficient to check whether the MIME Type of the response can be shown in the WebView that triggered the policy decision.

[Carlos Garcia Campos]

Created attachment 217417 [details]
Patch

I'm not sure about the name though, I'm using webkit_response_policy_decision_is_mime_type_supported() but I'm not totally convinced. Any other suggestion for the function name?

[WebKit Commit Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Martin Robinson]

Comment on attachment 217417 [details]
Patch

Hrm. Wouldn't webkit_response_policy_decision_can_show_mime_type make sense since we already have webkit_web_view_can_show_mime_type?

[Carlos Garcia Campos]

(In reply to comment #3)
> (From update of attachment 217417 [details])
> Hrm. Wouldn't webkit_response_policy_decision_can_show_mime_type make sense since we already have webkit_web_view_can_show_mime_type?

That was my first idea, but sounded a bit weird to me, webkit_web_view_can_show_mime_type makes sense because a WebView can show content, but a policy decision doesn't. What we want to know is whether the web view that triggered this policy decision request can show the mime type of the response.

[Gustavo Noronha (kov)]

Comment on attachment 217417 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=217417&action=review

> Source/WebKit2/UIProcess/API/gtk/WebKitResponsePolicyDecision.cpp:142
> + * webkit_response_policy_decision_is_mime_type_supported:

I like the name in principle. If we want to be very accurate about it, though, we could have webkit_response_policy_decision_can_web_view_show_mime_type(). I'm OK with is_mime_type_supported(), though, if you guys don't like that one ;) I agree with Carlos that can_show_mime_type makes a bit less sense here than in WebView.

> Source/WebKit2/UIProcess/API/gtk/WebKitResponsePolicyDecision.cpp:147
> + * that triggered this policy decision request.
> + * See also webkit_web_view_can_show_mime_type().

Nit: these could be on the same line.

[Carlos Garcia Campos]

(In reply to comment #5)
> (From update of attachment 217417 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=217417&action=review
> 
> > Source/WebKit2/UIProcess/API/gtk/WebKitResponsePolicyDecision.cpp:142
> > + * webkit_response_policy_decision_is_mime_type_supported:
> 
> I like the name in principle. If we want to be very accurate about it, though, we could have webkit_response_policy_decision_can_web_view_show_mime_type(). I'm OK with is_mime_type_supported(), though, if you guys don't like that one ;) I agree with Carlos that can_show_mime_type makes a bit less sense here than in WebView.

I'm fine with is_mime_type_supported() too, Martin?
 
> > Source/WebKit2/UIProcess/API/gtk/WebKitResponsePolicyDecision.cpp:147
> > + * that triggered this policy decision request.
> > + * See also webkit_web_view_can_show_mime_type().
> 
> Nit: these could be on the same line.

Sure!

[Martin Robinson]

(In reply to comment #6)

> I'm fine with is_mime_type_supported() too, Martin?


The name seems reasonable given the constraints Carlos mentioned. It's a little sloppy, in that there's a difference between a mime type being supported and showable. What I mean is that we support non-showable mime types by downloading them. I think my point is a nitpick though, so unless anyone can think of a better one, is_mime_type_supported should work just fine. The next best thing I can think of is is_mime_type_showable, but that feels a little bit tortured.

[Carlos Garcia Campos]

so, if we agree on the name, could someone please review the patch? :-)

[Carlos Garcia Campos]

Committed r161256: <http://trac.webkit.org/changeset/161256>