Bug 53576 - Improve readability of updateWidget by converting bool parameter to an enum
Summary: Improve readability of updateWidget by converting bool parameter to an enum
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-01 22:11 PST by Adam Barth
Modified: 2011-02-02 00:00 PST (History)
2 users (show)

See Also:

Attachments
Patch (8.04 KB, patch)
2011-02-01 22:12 PST, Adam Barth 		no flags Details | Formatted Diff | Diff
Patch for landing (8.23 KB, patch)
2011-02-01 22:36 PST, Adam Barth 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2011-02-01 22:11:39 PST 
Improve readability of updateWidget by converting bool parameter to an enum
Comment 1 Adam Barth 2011-02-01 22:12:06 PST 
Created attachment 80891 [details]
Patch
Comment 2 Alexey Proskuryakov 2011-02-01 22:16:39 PST 
Comment on attachment 80891 [details]
Patch

OK. I hope that someone actually refactors this code one day, so that we don't pass plugin creation option in HTMLMediaElement::updateWidget(), and that HTMLPlugInImageElement::updateWidgetIfNecessary() doesn't know about types of plug-ins!
Comment 3 Adam Barth 2011-02-01 22:17:45 PST 
Comment on attachment 80891 [details]
Patch

Yes.  This code is pretty goofy.
Comment 4 Simon Fraser (smfr) 2011-02-01 22:18:24 PST 
Comment on attachment 80891 [details]
Patch

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

> Source/WebCore/html/HTMLPlugInImageElement.h:33
> +    CreateAllWidgetTypes,

CreateAnyPluginType is better, I think.
Comment 5 Adam Barth 2011-02-01 22:23:35 PST 
> CreateAnyPluginType is better, I think.

Maybe "CreateAnyWidgeType" ?  This function can create more than just plugins.
Comment 6 Alexey Proskuryakov 2011-02-01 22:30:16 PST 
I think that to make these two changes a definitive win, you could also add a proper FIXME comment to the original spot, perhaps something like "FIXME: Why do we have a special case for some plug-in types here?"
Comment 7 Adam Barth 2011-02-01 22:36:40 PST 
Created attachment 80892 [details]
Patch for landing
Comment 8 Alexey Proskuryakov 2011-02-01 23:06:18 PST 
Thanks Adam!
Comment 9 WebKit Commit Bot 2011-02-02 00:00:31 PST 
Comment on attachment 80892 [details]
Patch for landing

Clearing flags on attachment: 80892

Committed r77366: <http://trac.webkit.org/changeset/77366>
Comment 10 WebKit Commit Bot 2011-02-02 00:00:37 PST 
All reviewed patches have been landed.  Closing bug.