RESOLVED FIXED 23653
PopupMenuChromium.h could be cleaned-up 
https://bugs.webkit.org/show_bug.cgi?id=23653
Summary PopupMenuChromium.h could be cleaned-up
Jay Campan
Reported 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.
Attachments
Patch for PopuMenuChromium.[h,cpp] (12.74 KB, patch)
2009-01-30 11:49 PST, Jay Campan 		eric: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Jay Campan
Comment 1 2009-01-30 11:49:26 PST
Created attachment 27190 [details] Patch for PopuMenuChromium.[h,cpp]
Eric Seidel (no email)
Comment 2 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.
Eric Seidel (no email)
Comment 3 2009-02-02 15:08:08 PST
Grats on your first patch to the webkit.org tree Jay. :)
Eric Seidel (no email)
Comment 4 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
Note You need to log in before you can comment on or make changes to this bug.