Bug 29177 – [GTK] Right click does not activate text entry

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Xan Lopez]

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.

[José Millán Soto]

Created attachment 47430 [details]
Proposed patch

I think that this patch solves this problem.

[Gustavo Noronha (kov)]

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 =(.

[Eric Seidel (no email)]

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).

[José Millán Soto]

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.

[Gustavo Noronha (kov)]

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 =).

[Brian Tarricone]

*** Bug 35064 has been marked as a duplicate of this bug. ***

[José Millán Soto]

Created attachment 49096 [details]
Proposed patch

New version of the patch, this time it works well with multi-frame pages.

[Gustavo Noronha (kov)]

Comment on attachment 49096 [details]
Proposed patch

This looks better, thanks for the patch!

[WebKit Commit Bot]

Comment on attachment 49096 [details]
Proposed patch

Clearing flags on attachment: 49096

Committed r55389: <http://trac.webkit.org/changeset/55389>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Eric Seidel (no email)]

The build bots seem to think http://trac.webkit.org/changeset/55389 broke the mac builder.  But I don't believe them.