Bug 28153

Summary: [GTK] can't middle-button paste within a single window
Product: WebKit Reporter: Dan Winship <danw>
Component: WebKit GtkAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: alex, christian, commit-queue, darin, eric, evan, pachoramos1, reinouts, tony, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on: 31711    
Bug Blocks:    
Attachments:
Description Flags
Proposed patch
jmalonzo: review-
Patch proposed with test
eric: review-
New patch proposal
none
Proposed patch
none
Proposed patch none

Description Dan Winship 2009-08-10 10:35:04 PDT
bug 25685 fixed middle-button paste between apps and between windows, but you still can't middle-button paste within a single window; as soon as the textarea you're trying to paste into gets focused, the text you had previously selected loses the selection, and so there's no longer anything to paste.

In other apps, merely focusing a textarea does not affect the existing selection
Comment 1 Xan Lopez 2009-08-26 22:47:26 PDT
*** Bug 27838 has been marked as a duplicate of this bug. ***
Comment 2 Alejandro G. Castro 2009-10-22 00:52:42 PDT
Created attachment 41645 [details]
Proposed patch

This is a first proposal to fix this issue. Now we store a reference to the range instead of requesting it every time, and we do not release it until the clipboard is requested.

Consider applying this other simple patch for the .h file also: https://bugs.webkit.org/show_bug.cgi?id=30664
Comment 3 Evan Martin 2009-10-22 11:11:46 PDT
Is it not possible to write a test for this?  It would seem you could add some sort of middle-click event to layoutTestController.
Comment 4 Eric Seidel 2009-10-22 11:16:23 PDT
eventSender.mouseDown takes a button argument if that's helpful.  I think mouseDown(1) is the middle button.
Comment 5 Jan Alonzo 2009-10-24 04:08:08 PDT
Comment on attachment 41645 [details]
Proposed patch

Patch looks fine but can we please add a test?

r- for the lack of test.
Comment 6 Alejandro G. Castro 2009-10-26 06:03:25 PDT
(In reply to comment #4)
> eventSender.mouseDown takes a button argument if that's helpful.  I think
> mouseDown(1) is the middle button.

Actually we added support for that parameter some weeks ago:

    2009-10-08  Alejandro G. Castro  <alex@igalia.com>

            Reviewed by Xan Lopez.

            [GTK] Added support for a parameter setting the button that was
            pressed in the mouseDown function.
            https://bugs.webkit.org/show_bug.cgi?id=30220

            * WebKitTools/DumpRenderTree/gtk/EventSender.cpp:

I'll add that test to the patch.
Comment 7 Alejandro G. Castro 2009-11-20 03:30:25 PST
Checking the test I found another issue that we could consider before adding this patch, it is reported in bug 31711.
Comment 8 Alejandro G. Castro 2009-11-20 08:43:40 PST
Created attachment 43582 [details]
Patch proposed with test

This patch includes the test for the solution proposed, but it will not work until we solve the bug 31711. Until that moment the test requires this patch to work properly, check the other bug for more information about the issue:

index 8783973..2ff65cb 100644
--- a/LayoutTests/platform/gtk/editing/pasteboard/middle-button-paste.html
+++ b/LayoutTests/platform/gtk/editing/pasteboard/middle-button-paste.html
@@ -20,8 +20,8 @@ function runTest()
     var y = target.offsetParent.offsetTop + target.offsetTop +
         target.offsetHeight / 2;

-    eventSender.mouseMoveTo(x, y);
     eventSender.mouseDown();
+    eventSender.mouseMoveTo(x, y);
     eventSender.mouseMoveTo(x + 50, y);
     eventSender.mouseUp();
     eventSender.mouseMoveTo(x + 100, y);
Comment 9 Eric Seidel 2009-11-26 21:11:42 PST
Comment on attachment 43582 [details]
Patch proposed with test

The test should be dumpAsText().
http://trac.webkit.org/wiki/Writing%20Layout%20Tests%20for%20DumpRenderTree
Comment 10 Alejandro G. Castro 2009-12-02 05:37:47 PST
Created attachment 44143 [details]
New patch proposal

I've used dumpAsText for the test and I've added a new condition to control situations where range could be 0 in the get_contents callback because toNormalizedRange can return 0.

I'm not adding the review request until the drag bug 31711 is fixed because the test will fail if we upload it.
Comment 11 Alejandro G. Castro 2009-12-19 04:14:27 PST
Created attachment 45233 [details]
Proposed patch

I've added the workaround to the test, I do not think makes sense to wait for that fix and the test is not incorrect.
Comment 12 Xan Lopez 2009-12-19 04:46:00 PST
Comment on attachment 45233 [details]
Proposed patch

>+    EditorClient* client = reinterpret_cast<EditorClient*>(data);

Isn't static_cast enough to go from void* to EditorClient?
Comment 13 Alejandro G. Castro 2009-12-19 04:58:54 PST
(In reply to comment #12)
> (From update of attachment 45233 [details])
> >+    EditorClient* client = reinterpret_cast<EditorClient*>(data);
> 
> Isn't static_cast enough to go from void* to EditorClient?

Yep, I've checked it, reviewing the patch.
Comment 14 Alejandro G. Castro 2009-12-19 05:01:26 PST
Created attachment 45234 [details]
Proposed patch
Comment 15 Xan Lopez 2009-12-19 10:12:52 PST
Comment on attachment 45234 [details]
Proposed patch

r=me
Comment 16 WebKit Commit Bot 2009-12-19 10:38:03 PST
Comment on attachment 45234 [details]
Proposed patch

Clearing flags on attachment: 45234

Committed r52389: <http://trac.webkit.org/changeset/52389>
Comment 17 WebKit Commit Bot 2009-12-19 10:38:12 PST
All reviewed patches have been landed.  Closing bug.