Bug 46111 – video-served-as-text.html failing on Windows

• Component: Media
• Status: RESOLVED DUPLICATE

[Eric Carlson]

LayoutTests/http/tests/media/video-served-as-text.html is failing on Windows, eg. http://trac.webkit.org/export/67846/trunk/LayoutTests/http/tests/media/video-served-as-text.html

[Eric Carlson]

Test  skipped in http://trac.webkit.org/changeset/67883.

[Eric Carlson]

<rdar://problem/8454284>

[Jer Noble]

Created attachment 70019 [details]
Patch

[Darin Adler]

Comment on attachment 70019 [details]
Patch

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

> WebCore/platform/graphics/win/QTMovie.cpp:65
> +static bool sDisabledComponents = 0;

The “s” prefix here is not WebKit style. I think we were talking about a style like s_disabledComponents for static data members, but I don’t think we planned to use a prefix at all for file-scoped globals.

But further, this global does not have to exist outside the disableUnsupportedComponents function. You can just put the static bool in there.

> WebCore/platform/graphics/win/QTMovie.cpp:279
> +    ComponentDescription components[] = {

Code needs a why comment.

> WebCore/platform/graphics/win/QTMovie.cpp:287
> +    ComponentDescription nullDesc = {'null', 'base', kAppleManufacturer, 0, 0};
> +    Component nullComp = FindNextComponent(0, &nullDesc);

I would like the code better if there was less Desc and Comp and more Description and Component.

> WebCore/platform/graphics/win/QTMovie.cpp:294
> +        Component disabledComp = 0;
> +        while (disabledComp = FindNextComponent(disabledComp, &components[i]))

Ditto.

[Jer Noble]

Actually, I don't much like the way this code is factored either.  I'm going to submit a new patch which moves around where some of the implementation lives.

[Jer Noble]

Embarrassing.  This bug is a duplicate of https://bugs.webkit.org/show_bug.cgi?id=45603.  The Radar was never updated with the original bugzilla bug.  Even more embarrassing, my two different implementations lived in entirely different places.  

No wonder I didn't like how it was factored!

I'm going to obsolete my patch, and hopefully we can all forget this ever happened.

[Adam Roben (:aroben)]

*** This bug has been marked as a duplicate of bug 45603 ***