Bug 49376 - [GTK] Show default context menu for the currently focused element when activated with keyboard
Summary: [GTK] Show default context menu for the currently focused element when activa...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-11-11 04:32 PST by Carlos Garcia Campos
Modified: 2010-11-16 09:31 PST (History)
2 users (show)

See Also:


Attachments
Patch to show context menu for focused items when using the keyboard (2.06 KB, patch)
2010-11-11 04:35 PST, Carlos Garcia Campos
mrobinson: review-
Details | Formatted Diff | Diff
Updated patch (2.28 KB, patch)
2010-11-12 00:01 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2010-11-11 04:32:13 PST
When context menu is activated with keyboard only selections are handled, when nothing is selected the default context menu is shown on top-left corner. If there's a focused element, like a link, the context menu for such element should be used and should be correctly positioned.
Comment 1 Carlos Garcia Campos 2010-11-11 04:35:17 PST
Created attachment 73599 [details]
Patch to show context menu for focused items when using the keyboard
Comment 2 Martin Robinson 2010-11-11 09:48:06 PST
Comment on attachment 73599 [details]
Patch to show context menu for focused items when using the keyboard

View in context: https://bugs.webkit.org/attachment.cgi?id=73599&action=review

> WebKit/gtk/webkit/webkitwebview.cpp:347
> +        || (frame->selection()->selection().isCaret() && !frame->selection()->selection().isContentEditable())) {
> +        // If there's a focused elment, use its location.
> +        bool locationSet = false;
> +        Document* doc = frame->document();
> +        if (doc) {

I think maybe we should break out this block out into a helper function...perhaps everyting from the start of this block to the FIXME. getActiveNodeLocation or something similar? This will simplify things by allowing early returns (you can do away with the locationSet variable). You can also simplify this further by doing something like:

if (Document* doc = frame->document()) {
    if (Node* focusedNode = doc->focusedNode()) {
        ...
    }
}
Comment 3 Carlos Garcia Campos 2010-11-12 00:01:23 PST
Created attachment 73707 [details]
Updated patch

I've added a method getFocusedNode() that just returns the currently focused node, because it's more generic and I'll be able to use it for bug #25525 too.
Comment 4 WebKit Commit Bot 2010-11-16 09:31:47 PST
Comment on attachment 73707 [details]
Updated patch

Clearing flags on attachment: 73707

Committed r72109: <http://trac.webkit.org/changeset/72109>
Comment 5 WebKit Commit Bot 2010-11-16 09:31:52 PST
All reviewed patches have been landed.  Closing bug.