Summary: | [Qt] [WK2] create an initial implementation of the context menu handling for WebKit 2 | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Benjamin Poulain <benjamin> | ||||||
Component: | WebKit2 | Assignee: | Benjamin Poulain <benjamin> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Minor | CC: | commit-queue | ||||||
Priority: | P3 | Keywords: | Qt, QtTriaged | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 51902 | ||||||||
Attachments: |
|
Description
Benjamin Poulain
2011-01-03 09:57:03 PST
Created attachment 77814 [details]
Patch
Comment on attachment 77814 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=77814&action=review > WebCore/platform/qt/ContextMenuQt.cpp:76 > +Vector<ContextMenuItem> contextMenuItemVector(PlatformMenuDescription platformDescription) I would just write "description", but as this is already paltform code, I would replace PlatformMenuDescription with what it is defined to, ie QList<ContextMenuItem>. > WebKit2/UIProcess/API/qt/qgraphicswkview.cpp:330 > + // Remove the active menu in case this function is called twice. I think it is called "in the case". You can replace twice with repeatedly. > WebKit2/UIProcess/API/qt/qgraphicswkview.cpp:341 > + // FIXME: try to determine the view from the position of the incoming events? > + QWidget* view = 0; > + if (QGraphicsScene* myScene = scene()) { > + const QList<QGraphicsView*> views = myScene->views(); > + view = views.value(0, 0); > + } Did you see how I did this with the vkb? I think that I tried handling this. > WebKit2/UIProcess/qt/WebContextMenuProxyQt.cpp:49 > + default: break; I think break should go to the next line > WebKit2/UIProcess/qt/WebContextMenuProxyQt.cpp:109 > + > + // don't show sub-menus with just disabled actions With capital and ending with a dot > WebKit2/UIProcess/qt/WebContextMenuProxyQt.cpp:114 > + bool anyEnabledAction = false; anyActionEnabled, anyActionWasEnabled? depending on what it really represents. Created attachment 77876 [details]
Kenneth, is that what you had in mind to find the correct view?
(And thanks for reviewing the patch by the way) Comment on attachment 77876 [details] Kenneth, is that what you had in mind to find the correct view? Clearing flags on attachment: 77876 Committed r74964: <http://trac.webkit.org/changeset/74964> All reviewed patches have been landed. Closing bug. |