Bug 29177 - [GTK] Right click does not activate text entry
Summary: [GTK] Right click does not activate text entry
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:
: 35064 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-09-11 04:32 PDT by Xan Lopez
Modified: 2010-03-01 20:12 PST (History)
5 users (show)

See Also:


Attachments
Proposed patch (2.50 KB, patch)
2010-01-26 11:49 PST, José Millán Soto
eric: review-
Details | Formatted Diff | Diff
Proposed patch (9.23 KB, patch)
2010-02-02 12:12 PST, José Millán Soto
no flags Details | Formatted Diff | Diff
Proposed patch (8.73 KB, patch)
2010-02-19 11:03 PST, José Millán Soto
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Xan Lopez 2009-09-11 04:32:16 PDT
From epiphany bug https://bugzilla.gnome.org/show_bug.cgi?id=575502#add_comment:

> Also clicking with right mouse button on the text box does not make it active
> (might be a heritage from the mac-land :P). When you are typing in one textbox
> and then do a right mouse click on another text box and choose paste from the
> context menu, text ends up in previously active (aka wrong) textbox.
Comment 1 José Millán Soto 2010-01-26 11:49:11 PST
Created attachment 47430 [details]
Proposed patch

I think that this patch solves this problem.
Comment 2 Gustavo Noronha (kov) 2010-01-26 12:09:16 PST
Comment on attachment 47430 [details]
Proposed patch

> +        Makes the frame to handle the mouse click event before sending the

You can remove that 'to' =)

I can say 'go' to this patch if it keeps working to bring up the context menu in maps.google.com, and to remove talent points here: http://www.wowhead.com/?talent#I (click the squares to add points, then right-click to remove points). Does it? Would be good to have some layout tests for this =(.
Comment 3 Eric Seidel (no email) 2010-01-26 14:28:28 PST
Comment on attachment 47430 [details]
Proposed patch

OK.  But how do we test this?  r- for lack of tests (or lack of explanation as to why testing is impossible here).
Comment 4 José Millán Soto 2010-02-02 12:12:30 PST
Created attachment 47962 [details]
Proposed patch

This patch works in the pages commented in comment 2.

There were already layout tests for this, but they were skipped in GTK. This patch removes them from the Skipped list.
Also, popup menu position when invoked via keyboard is improved (popup menu appeared where the mouse cursor was, which could led to confusion if it was not above the selected node).

Click event handling using javascript is tested in fast/events/mouse-click-events.html; WebKitGTK fails that text, but in the results page it can be seen that the right button is correctly handled.
Comment 5 Gustavo Noronha (kov) 2010-02-14 08:21:45 PST
Comment on attachment 47962 [details]
Proposed patch

I very much like this patch, sorry for the delay in looking at it. I have a few questions:

 235     if (mainFrame->view() && mainFrame->eventHandler()->handleMousePressEvent(event))
 236         mousePressEventResult = TRUE;

Why do we use the main frame here, instead of the focusedOrMainFrame?

285      if (!start.node() || !end.node())
 303     if (!start.node() || !end.node()
 304         || (page->mainFrame()->selection()->selection().isCaret() && !page->mainFrame()->selection()->selection().isContentEditable()))

Same here, will this still work for multi-frame pages, such as, say, the patch review interface in this bugzilla? Thanks for digging the tests, that helps a lot in keeping the sanity =).
Comment 6 Brian Tarricone 2010-02-18 12:20:33 PST
*** Bug 35064 has been marked as a duplicate of this bug. ***
Comment 7 José Millán Soto 2010-02-19 11:03:33 PST
Created attachment 49096 [details]
Proposed patch

New version of the patch, this time it works well with multi-frame pages.
Comment 8 Gustavo Noronha (kov) 2010-03-01 10:53:09 PST
Comment on attachment 49096 [details]
Proposed patch

This looks better, thanks for the patch!
Comment 9 WebKit Commit Bot 2010-03-01 17:12:23 PST
Comment on attachment 49096 [details]
Proposed patch

Clearing flags on attachment: 49096

Committed r55389: <http://trac.webkit.org/changeset/55389>
Comment 10 WebKit Commit Bot 2010-03-01 17:12:28 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 Eric Seidel (no email) 2010-03-01 20:12:51 PST
The build bots seem to think http://trac.webkit.org/changeset/55389 broke the mac builder.  But I don't believe them.