Bug 23653

Summary: PopupMenuChromium.h could be cleaned-up
Product: WebKit Reporter: Jay Campan <jcampan>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Severity: Minor    
Priority: P3    
Version: 528+ (Nightly build)   
Hardware: All   
OS: OS X 10.5   
Description Flags
Patch for PopuMenuChromium.[h,cpp] eric: review+

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