Bug 24946 - right clicking on selected text de-selects the text in chromium/gtk+ ports
Summary: right clicking on selected text de-selects the text in chromium/gtk+ ports
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Tony Chang
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-03-30 14:45 PDT by Tony Chang
Modified: 2009-04-06 11:08 PDT (History)
0 users

See Also:


Attachments
[1/1] Ensure the right click doesn't deselect text under it. This was (5.81 KB, patch)
2009-03-30 14:56 PDT, Tony Chang
no flags Details | Formatted Diff | Diff
svn version (24.21 KB, patch)
2009-03-30 15:20 PDT, Tony Chang
fishd: review+
Details | Formatted Diff | Diff
v2 (24.10 KB, patch)
2009-04-01 09:51 PDT, Tony Chang
fishd: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tony Chang 2009-03-30 14:45:40 PDT
1) Select text on a page.
2) Right click on the selected text.
3) When the context menu appears, the text is deselected and the user is unable to copy to the clipboard because nothing is selected.

This regressed in r41715 https://bugs.webkit.org/show_bug.cgi?id=19737 .

Patch coming...
Comment 1 Tony Chang 2009-03-30 14:56:18 PDT
Created attachment 29089 [details]
[1/1] Ensure the right click doesn't deselect text under it.  This was

happening in the Chromium and GTK+ ports because they don't select
the text under the cursor on right click.

This was regressed in r41715, https://bugs.webkit.org/show_bug.cgi?id=19737

https://bugs.webkit.org/show_bug.cgi?id=24946
---
 LayoutTests/ChangeLog                              |   17 ++++++++++++++
 LayoutTests/fast/events/context-no-deselect.html   |   23 ++++++++++++++++++++
 .../events/context-no-deselect-expected.checksum   |    1 +
 .../fast/events/context-no-deselect-expected.png   |  Bin 0 -> 13759 bytes
 .../fast/events/context-no-deselect-expected.txt   |   16 +++++++++++++
 WebCore/ChangeLog                                  |   17 ++++++++++++++
 WebCore/page/EventHandler.cpp                      |   10 +++++---
 7 files changed, 80 insertions(+), 4 deletions(-)
Comment 2 Tony Chang 2009-03-30 14:57:59 PDT
Comment on attachment 29089 [details]
[1/1] Ensure the right click doesn't deselect text under it.  This was

Patch is missing the binary diff.  Trying to recreate the diff...
Comment 3 Tony Chang 2009-03-30 15:20:17 PDT
Created attachment 29090 [details]
svn version

Same patch as before, but made with svn so the png is in the diff.
Comment 4 Darin Fisher (:fishd, Google) 2009-03-30 23:37:43 PDT
Comment on attachment 29090 [details]
svn version

This change seems simple and correct to me.  r+


> Index: LayoutTests/fast/events/context-no-deselect.html
...
> +if (window.layoutTestController) {
> +     var x, y;
> +     x = input.offsetParent.offsetLeft + input.offsetLeft + input.offsetWidth / 2;
> +     y = input.offsetParent.offsetTop + input.offsetTop + input.offsetHeight / 2;
> +     eventSender.mouseMoveTo(x, y);
> +     eventSender.contextClick();

nit: indentation should be 4 spaces.  please fix this before landing.


why is it correct to copy the results from LayoutTests/platform/mac/fast/repaint/renderer-destruction-by-invalidateSelection-crash.html?  that test seems to have a very different rendering.
Comment 5 Tony Chang 2009-03-31 09:28:34 PDT
(In reply to comment #4)
> why is it correct to copy the results from
> LayoutTests/platform/mac/fast/repaint/renderer-destruction-by-invalidateSelection-crash.html?
>  that test seems to have a very different rendering.

Oh, it doesn't.  I had the script create a new baseline. It was probably git that thought the file was a copy of renderer-destruction-by-invalidateSelection-crash.html.
Comment 6 Tony Chang 2009-04-01 09:51:01 PDT
Created attachment 29162 [details]
v2

updated patch with indention fixed and changelog fixed.
Comment 7 Tony Chang 2009-04-02 17:54:56 PDT
Can someone land this for me?  I don't have commit access.
Comment 8 Darin Fisher (:fishd, Google) 2009-04-06 11:08:43 PDT
Landed as: http://trac.webkit.org/changeset/42154

I forgot to mark this as FIXED back on Wednesday :)