[webkit-reviews] review granted: [Bug 52467] REGRESSION (r72119): Audio never plays on Star Wars intro animation : [Attachment 79765] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Jer Noble <jer.noble at apple.com>'s
request for review:
Bug 52467: REGRESSION (r72119): Audio never plays on Star Wars intro animation
https://bugs.webkit.org/show_bug.cgi?id=52467

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=79765&action=review

EWS bot was unable to apply this patch.

> Source/WebCore/ChangeLog:41
> +2011-01-20  Jer Noble  <jer.noble at apple.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   REGRESSION (r72119): Audio never plays on Star Wars intro animation
> +	   https://bugs.webkit.org/show_bug.cgi?id=52467
> +	   
> +	   QuickTime's eat/m4a movie importer compontent doesn't list audio/m4a
as a mime
> +	   type which it supports, though it handles .m4a files just fine. 
Change the way
> +	   we build the list of supported MIME Types through a new
WebKitSystemInterface
> +	   function.
> +	   
> +	   Caused by r72119, which adds system-specific extension->MIME entries
to the cache
> +	   before global entries, and the system-specific entries include
QuickTime's registry 
> +	   entries which contain the audio/m4a MIME type, while its components
do not.
> +
> +	   Test: media/audio-mpeg4-supported.html
> +
> +	   * WebCore.vcproj/QTMovieWinCommon.vsprops:
> +	   * platform/graphics/win/QTMovie.cpp:
> +	   (getMIMETypeCallBack):
> +	   (initializeSupportedTypes):
> +
> +2011-01-14  Jer Noble  <jer.noble at apple.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   REGRESSION (r72119): Audio never plays on Star Wars intro animation
(52467)
> +	   https://bugs.webkit.org/show_bug.cgi?id=52467
> +
> +	   QuickTime's eat/m4a movie importer compontent doesn't list audio/m4a
as a mime
> +	   type which it supports, though it handles .m4a files just fine; so
special case
> +	   the mime types for that component.
> +	   
> +	   Caused by r72119, which adds system-specific extension->MIME entries
to the cache
> +	   before global entries, and the system-specific entries include
QuickTime's registry 
> +	   entries which contain the audio/m4a MIME type, while its components
do not.
> +
> +	   * platform/graphics/win/QTMovie.cpp:
> +	   (initializeSupportedTypes):
> +

Double change log.

> Source/WebCore/platform/graphics/win/QTMovie.cpp:753
> +    CFStringRef cfType = CFStringCreateWithCString(kCFAllocatorDefault,
type, kCFStringEncodingMacRoman);

MacRoman? Really? If it’s always going to be ASCII then can we say that? Or use
Latin1? Or UTF-8?

> WebKitLibraries/win/include/WebKitSystemInterface/WebKitSystemInterface.h:172

> +typedef void(*wkQuickTimeMIMETypeCallBack)(const char* mimeType);

I think there should be a space after void. I’m surprised that we are not
capitalizing the WK in the typedef.