| Summary: | video-served-as-text.html failing on Windows | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Eric Carlson <eric.carlson> | ||||
| Component: | Media | Assignee: | Jer Noble <jer.noble> | ||||
| Status: | RESOLVED DUPLICATE | ||||||
| Severity: | Normal | CC: | aroben | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | PC | ||||||
| OS: | Windows 7 | ||||||
| Attachments: |
|
||||||
|
Description
Eric Carlson
2010-09-20 11:53:10 PDT
Test skipped in http://trac.webkit.org/changeset/67883. Created attachment 70019 [details]
Patch
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. 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. 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. |