Summary: | [Windows] Loader is not properly determining supported MIME Types | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brent Fulgham <bfulgham> | ||||||
Component: | WebCore Misc. | Assignee: | Brent Fulgham <bfulgham> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | bfulgham, eric.carlson, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Brent Fulgham
2013-08-27 16:27:45 PDT
This bug is visible when requesting a WebView display a media type (e.g., http://test-safari.apple.com/captions/media/AVSCCsync720P_AW_122.mov). This patch was put together with Eric Carlson, though the refactoring to avoid the String->BSTR->String round trip was added later. Created attachment 209819 [details]
Patch
Created attachment 209820 [details]
Patch
Comment on attachment 209820 [details] Patch Attachment 209820 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/1625077 Comment on attachment 209820 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=209820&action=review > Source/WebKit/win/WebView.cpp:2579 > + notImplemented(); > + return true; Sorry, I don’t get why this isn’t implemented. Comment on attachment 209820 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=209820&action=review >> Source/WebKit/win/WebView.cpp:2579 >> + return true; > > Sorry, I don’t get why this isn’t implemented. It wasn't implemented before. I started hooking it up on the client side and realized the WebView wasn't implemented either. I figured it was better to have a "not implemented" state in a single place, rather than having it not implemented in the WebFrameLoaderClient *and* not implemented in the WebView. Now at least there is only one thing to implement. Comment on attachment 209820 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=209820&action=review >> Source/WebKit/win/WebView.cpp:2579 >> + return true; > > Sorry, I don’t get why this isn’t implemented. It wasn't implemented before. I started hooking it up on the client side and realized the WebView wasn't implemented either. I figured it was better to have a "not implemented" state in a single place, rather than having it not implemented in the WebFrameLoaderClient *and* not implemented in the WebView. Now at least there is only one thing to implement. (In reply to comment #6) > (From update of attachment 209820 [details]) > Attachment 209820 [details] did not pass win-ews (win): > Output: http://webkit-queues.appspot.com/results/1625077 This output seems wrong. All individual projects build, but the overall state is marked 'Failed'. I also do not see any build errors when performing a local build. Comment on attachment 209820 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=209820&action=review >>>> Source/WebKit/win/WebView.cpp:2579 >>>> + return true; >>> >>> Sorry, I don’t get why this isn’t implemented. >> >> It wasn't implemented before. I started hooking it up on the client side and realized the WebView wasn't implemented either. I figured it was better to have a "not implemented" state in a single place, rather than having it not implemented in the WebFrameLoaderClient *and* not implemented in the WebView. Now at least there is only one thing to implement. > > It wasn't implemented before. I started hooking it up on the client side and realized the WebView wasn't implemented either. I figured it was better to have a "not implemented" state in a single place, rather than having it not implemented in the WebFrameLoaderClient *and* not implemented in the WebView. Now at least there is only one thing to implement. It might be worth filing about about implementing all of the missing parts of WebView. > Source/WebKit/win/WebCoreSupport/WebFrameLoaderClient.cpp:926 > +bool WebFrameLoaderClient::canShowMIMEType(const String& MIMEType) const Although we are inconsistent about whether a "MIME type" variable should be "MIMEType" or "mimeType", the later is more than 8x more common so I would suggest using that. If you prefer the former, you should at least make all uses in this patch consistent. Committed r154759: <http://trac.webkit.org/changeset/154759> (In reply to comment #11) > (From update of attachment 209820 [details]) > It might be worth filing about about implementing all of the missing parts of WebView. Done. Filed as https://bugs.webkit.org/show_bug.cgi?id=120426. |