[webkit-reviews] review granted: [Bug 70110] [Qt] fast/events/media-focus-in-standalone-media-document.html fails : [Attachment 111954] patch

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has granted Deepak Sherveghar
<bpwv64 at motorola.com>'s request for review:
Bug 70110: [Qt] fast/events/media-focus-in-standalone-media-document.html fails
https://bugs.webkit.org/show_bug.cgi?id=70110

Attachment 111954: patch
https://bugs.webkit.org/attachment.cgi?id=111954&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=111954&action=review


> Source/WebCore/platform/qt/MIMETypeRegistryQt.cpp:64
> +    { "mp4", "video/mp4" },
> +    { "m4v", "video/mp4" },
> +    { "m4a", "audio/x-m4a" },
> +    { "mp3", "audio/mp3" },
> +    { "ogv", "video/ogg" },
> +    { "oga", "audio/ogg" },
> +    { "ogm", "audio/ogg" },
> +    { "ogg", "audio/ogg" },
> +    { "webm", "video/webm" },
> +    { "webm", "audio/webm" },
> +    { "wav", "audio/wav" },
> +    { "mov", "video/quicktime" },

I think that this part is correct and badly needed :)

> Source/WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:666
> +    if (MIMETypeRegistry::isSupportedMediaMIMEType(type))
> +	   return true;
> +

canShowMIMEType is called _after_ dispatchDecidePolicyForResponse. If
DecidePolicyForResponse returns PolicyUse but canShowMIMEType suddenly would
return false, then that
would sound like a bug to me. I think that is why for example the WK2
implementation always returns true and we probably should do the same in WK1.
Your patch "improves" this
by increasing the probability of returning true ;-), but in the long run I
wonder if canShowMIMEType should be removed from the FrameLoaderClient
altogether?