Bug 23653 - PopupMenuChromium.h could be cleaned-up
Summary: PopupMenuChromium.h could be cleaned-up
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All OS X 10.5
: P3 Minor
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-01-30 11:47 PST by Jay Campan
Modified: 2009-02-02 15:08 PST (History)
0 users

See Also:

Attachments
Patch for PopuMenuChromium.[h,cpp] (12.74 KB, patch)
2009-01-30 11:49 PST, Jay Campan 		eric: review+
Details | Formatted Diff | Diff
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 Jay Campan 2009-01-30 11:47:34 PST 
In WebCore/platform/chromium/PopupMenuChromium.h many states are passed around with setters (setTextOnIndexChange, setAcceptOnAbandon, setLoopSelectionNavigation...). As these settings are only required when initializing and as new ones may be added, it would be better to have a settings struct instead of setters.
Comment 1 Jay Campan 2009-01-30 11:49:26 PST 
Created attachment 27190 [details]
Patch for PopuMenuChromium.[h,cpp]
Comment 2 Eric Seidel (no email) 2009-02-02 14:36:28 PST 
Comment on attachment 27190 [details]
Patch for PopuMenuChromium.[h,cpp]

This looks fine.

 1 2009-01-29  Jay Campan  <set EMAIL_ADDRESS environment variable>

Your EMAIL_ADDRESS is missing.

Wrong spacing:
+static const PopupContainerSettings dropDownSettings = {
+  true, // focusOnShow
+  true, // setTextOnIndexChange
+  true, // acceptOnAbandon
+  false // loopSelectionNavigation
+};

This looks fine.  Can be fixed on commit.
Comment 3 Eric Seidel (no email) 2009-02-02 15:08:08 PST 
Grats on your first patch to the webkit.org tree Jay. :)
Comment 4 Eric Seidel (no email) 2009-02-02 15:08:20 PST 
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/platform/chromium/PopupMenuChromium.cpp
	M	WebCore/platform/chromium/PopupMenuChromium.h
Committed r40494