Bug 24946

Summary: right clicking on selected text de-selects the text in chromium/gtk+ ports
Product: WebKit Reporter: Tony Chang <tony>
Component: WebCore Misc.Assignee: Tony Chang <tony>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows XP   
Attachments:
Description Flags
[1/1] Ensure the right click doesn't deselect text under it. This was
none
svn version
fishd: review+
v2 fishd: review+

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