Summary: | Multiselect Popup - PopupMenuClient extension | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Luiz Agostini <luiz> | ||||||||||
Component: | WebCore Misc. | Assignee: | Luiz Agostini <luiz> | ||||||||||
Status: | CLOSED FIXED | ||||||||||||
Severity: | Normal | CC: | commit-queue, hausmann, kenneth, koivisto, tonikitoo | ||||||||||
Priority: | P2 | Keywords: | Qt | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Bug Depends on: | 36177 | ||||||||||||
Bug Blocks: | 36186 | ||||||||||||
Attachments: |
|
Description
Luiz Agostini
2010-03-16 11:54:00 PDT
Created attachment 50833 [details]
patch 1
+ Document* doc = static_cast<Element*>(node())->document(); no need to cast here, luiz: node()->document() +if (!doc || doc != doc->frame()->document()) that looks odd :) 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 Created attachment 51046 [details]
patch 2
Created attachment 51055 [details]
patch 3
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.
Created attachment 51105 [details]
patch 4
Comment on attachment 51105 [details]
patch 4
Looks good, r=me
Comment on attachment 51105 [details] patch 4 Clearing flags on attachment: 51105 Committed r56285: <http://trac.webkit.org/changeset/56285> All reviewed patches have been landed. Closing bug. |