[webkit-reviews] review granted: [Bug 15279] Right clicking over text (or multiple spaces) auto selects the word (or multiple spaces) under it : [Attachment 25104] Disables the word selection behavior for Chromium using an #ifdef.

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has granted 's request for review:
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

Attachment 25104: Disables the word selection behavior for Chromium using an
#ifdef. 
https://bugs.webkit.org/attachment.cgi?id=25104&action=review

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
You should set the review flag to "r?" for new patches to indicate that your
patch is ready to review.

> +2008-11-12  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