Bug 15279 – Right clicking over text (or multiple spaces) auto selects the word (or multiple spaces) under it

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Rahul Kuchhal]

Due to changes made in WebCore/page/EventHandler.cpp at @24499 & @24561, when I right click on a page, the word under is auto selected and I don't see the correct context menu (Reload, View Source etc). Instead a selection based menu of Search with Google, Copy etc is shown.

[Darin Fisher (:fishd, Google)]

At least, it seems broken that this happens when right clicking on a link.

[Adam Roben (:aroben)]

As you can see in the ChangeLog comments from r24499 <http://trac.webkit.org/projects/webkit/changeset/24499>, this is actually expected behavior. If you click on something other than a block of text, no selection should be made and you should see the Reload/View Source menu as you expected.

[Darin Fisher (:fishd, Google)]

Really?  Even for hyperlinks?

[Darin Fisher (:fishd, Google)]

This can't be wontfix.  At least it is very broken behavior on Windows.  Can we at least make it possible for embedders to define their own behavior here?

[Mark Rowe (bdash)]

The behaviour when ctrl-clicking on a link in WebKit differs from that in TextEdit, so I think this is worthy of some investigation.  Ctrl-clicking on a link in TextEdit selects the entire URL, while in WebKit it only selects the word under the cursor.  There may be issues with right-clicking on other platforms at well, but at the very least this issue exists on Mac OS as well.

[Mark Rowe (bdash)]

<rdar://problem/5450655>

[Matt Lilek]

(In reply to comment #4)
> At least it is very broken behavior on Windows.

ToT on Windows behaves the same as ToT on Mac for me in regards to selection behavior and the contents of the context menu, etc.

[Robert Blaut]

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

[Robert Blaut]

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

[Luca Bruno]

On the GTK+ port there's the same problem. When i right-click it selects the words before the cursor.

[Robert Blaut]

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

[Aaron Boodman]

Created attachment 25104 [details]
Disables the word selection behavior for Chromium using an #ifdef.

[Adam Roben (:aroben)]

Comment on attachment 25104 [details]
Disables the word selection behavior for Chromium using an #ifdef. 

You should set the review flag to "r?" for new patches to indicate that your patch is ready to review.

> [phone]  aa  <set EMAIL_ADDRESS environment variable>

You should set the EMAIL_ADDRESS environment variable so that prepare-ChangeLog can fill in your email address correctly. You should also set the CHANGE_LOG_NAME environment variable to the name you want used in ChangeLogs.

> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        * page/EventHandler.cpp:
> +        (WebCore::EventHandler::sendContextMenuEvent):
> +        
> +        Disable word selection on context click for PLATFORM(CHROMIUM).

Normally we reference the Bugzilla bug we're fixing in the ChangeLog. Something like this:

Chromium fix for Bug 15279: Right clicking over text (or multiple spaces) auto selects the word (or multiple spaces) under it

<https://bugs.webkit.org/show_bug.cgi?id=15279>

> @@ -1627,8 +1627,9 @@ bool EventHandler::sendContextMenuEvent(
>      IntPoint viewportPos = v->windowToContents(event.pos());
>      MouseEventWithHitTestResults mev = doc->prepareMouseEvent(HitTestRequest(false, true), viewportPos, event);
>  
> -    // Context menu events shouldn't select text in GTK+ applications.
> -#if !PLATFORM(GTK)
> +    // Context menu events shouldn't select text in GTK+ applications or in Chromium.
> +    // TODO: This should probably be configurable by embedders. Consider making it a WebPreferences setting.

We normally use FIXME instead of TODO. If you have filed the bug for WebPreferences you could reference it here with a URL.

r=me

-Adam

[Aaron Boodman]

Note: I talked about this on IRC with aroben. For now, we're going to change this using a macro, but in the future (perhaps when the Chromium mac team gets to this area), we should change it to use a WebPreferences setting.

[Aaron Boodman]

Created attachment 25117 [details]
Disables the word selection behavior for Chromium, take 2

New patch. I believe I've addressed all your comments.

[Adam Roben (:aroben)]

Comment on attachment 25117 [details]
Disables the word selection behavior for Chromium, take 2

> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog	(revision 38339)
> +++ WebCore/ChangeLog	(working copy)
> @@ -1,3 +1,16 @@
> [phone]  aa  [e-mail]

We normally use full names in ChangeLog headers, but this isn't a requirement.

r=me

[Eric Seidel (no email)]

Shouldn't this only be disabled for chromium on windows and linux?

[Dimitri Glazkov (Google)]

Committed in https://bugs.webkit.org/show_bug.cgi?id=23351

New bug for solution with WebPreferences created: https://bugs.webkit.org/show_bug.cgi?id=23351