Bug 36178 - Multiselect Popup - PopupMenuClient extension
Summary: Multiselect Popup - PopupMenuClient extension
Status: CLOSED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Luiz Agostini
URL:
Keywords: Qt
Depends on: 36177
Blocks: 36186
  Show dependency treegraph
 
Reported: 2010-03-16 11:54 PDT by Luiz Agostini
Modified: 2010-03-25 03:01 PDT (History)
5 users (show)

See Also:


Attachments
patch 1 (5.82 KB, patch)
2010-03-16 14:01 PDT, Luiz Agostini
no flags Details | Formatted Diff | Diff
patch 2 (5.91 KB, patch)
2010-03-18 10:39 PDT, Luiz Agostini
no flags Details | Formatted Diff | Diff
patch 3 (5.77 KB, patch)
2010-03-18 11:02 PDT, Luiz Agostini
no flags Details | Formatted Diff | Diff
patch 4 (5.96 KB, patch)
2010-03-18 15:42 PDT, Luiz Agostini
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Luiz Agostini 2010-03-16 11:54:00 PDT
PopupMenuClient class is the interface used by combobox popup implementations. It needs to be extended to handle <select multiple> needs.
Comment 1 Luiz Agostini 2010-03-16 14:01:34 PDT
Created attachment 50833 [details]
patch 1
Comment 2 Antonio Gomes 2010-03-16 14:15:07 PDT
+ Document* doc = static_cast<Element*>(node())->document();

no need to cast here, luiz: node()->document()

+if (!doc || doc != doc->frame()->document())

that looks odd :)
Comment 3 Kenneth Rohde Christiansen 2010-03-18 08:15:15 PDT
Remove [Qt] from title as this patch doesn't even touch Qt code. Add Qt as keyword so that it shows up in our filters
Comment 4 Luiz Agostini 2010-03-18 10:39:43 PDT
Created attachment 51046 [details]
patch 2
Comment 5 Luiz Agostini 2010-03-18 11:02:57 PDT
Created attachment 51055 [details]
patch 3
Comment 6 Dave Hyatt 2010-03-18 12:27:43 PDT
Comment on attachment 51055 [details]
patch 3

Let's call it ListPopupMenuClient instead of just ListPopupClient.

I think you should wrap all the code in this patch inside #if ENABLE(NO_LISTBOX_RENDERING).  That will help people understand that this is unique to that feature.

Everything else looks fine.
Comment 7 Luiz Agostini 2010-03-18 15:42:30 PDT
Created attachment 51105 [details]
patch 4
Comment 8 Antti Koivisto 2010-03-19 09:10:04 PDT
Comment on attachment 51105 [details]
patch 4

Looks good, r=me
Comment 9 WebKit Commit Bot 2010-03-19 17:08:39 PDT
Comment on attachment 51105 [details]
patch 4

Clearing flags on attachment: 51105

Committed r56285: <http://trac.webkit.org/changeset/56285>
Comment 10 WebKit Commit Bot 2010-03-19 17:08:44 PDT
All reviewed patches have been landed.  Closing bug.