RESOLVED DUPLICATE of bug 25685 17267
[GTK] Primary selection/clipboard support
https://bugs.webkit.org/show_bug.cgi?id=17267
Summary [GTK] Primary selection/clipboard support
Alp Toker
Reported 2008-02-09 19:21:28 PST
We already have clipboard support, but it doesn't work with the primary clipboard on X. We need to support (a) selections for pasting from WebKit into other applications (b) middle-click for pasting from the primary clipboard into editable WebKit areas On platforms without a primary clipboard, GTK+ automatically makes this a no-op, so we don't need to add any conditional/X-specific code to support this.
Attachments
Attempt at primary clipboard support (1.08 KB, patch)
2008-06-22 05:09 PDT, Christian Dywan
oliver: review-
Primary clipboard copy support (3.94 KB, patch)
2008-09-10 08:32 PDT, Alp Toker
no flags
Copy link locations to the primary selection (4.33 KB, patch)
2008-10-27 15:47 PDT, Sergio García-Cuevas González
no flags
Preliminary primary selection paste support (2.25 KB, patch)
2009-01-28 15:46 PST, Sergio García-Cuevas González
gustavo: review-
Christian Dywan
Comment 1 2008-06-22 05:09:52 PDT
Created attachment 21872 [details] Attempt at primary clipboard support I do not know too well about WebCore's clipboard support, but at some point I really needed primary clipboard support. So here is my attempt. It works for all I want.
Oliver Hunt
Comment 2 2008-06-22 05:35:05 PDT
Comment on attachment 21872 [details] Attempt at primary clipboard support r-, the shouldBlah methods are callbacks made to the api to query whether something is allowed, not that is has already happened. EditorClient methods are (by and large) just present to allow WebCore's behaviour to be controlled through the platforms API, so all these methods should just map through to your API implementation. The WebKit/GTK API's default implementation for didChangeSelectedRange (or whatever, definitely not shouldChangeSelectedRange) would probably be something along these lines.
Alp Toker
Comment 3 2008-08-11 16:26:50 PDT
The performance hit/memory fragmentation of converting HTML to text every time a large selection is changed would be too great, and would rule out the possibility of content negotiation (copy and paste as HTML, text, rich text, Pango markup etc.) I think we need to avoid copying the inner text of the selection every time it changes, instead responding to a primary selection request when it happens (Qt does the former and I don't like it.)
Alp Toker
Comment 4 2008-09-10 08:32:50 PDT
Created attachment 23321 [details] Primary clipboard copy support The behaviour of this patch is intended to match GtkTextView (which is slightly different to the selection support in Firefox and OpenOffice and fits in better with GTK+ applications).
Alp Toker
Comment 5 2008-09-10 13:32:09 PDT
Comment on attachment 23321 [details] Primary clipboard copy support Landed in r36323. Still need paste support to close this bug.
Sergio García-Cuevas González
Comment 6 2008-10-27 15:47:21 PDT
Created attachment 24698 [details] Copy link locations to the primary selection This patch makes WebCore::Pasteboard::writeURL use the primary selection as well as the clipboard selection.
Alp Toker
Comment 7 2008-10-30 13:41:09 PDT
(In reply to comment #6) > Created an attachment (id=24698) [edit] > Copy link locations to the primary selection > > This patch makes WebCore::Pasteboard::writeURL use the primary selection as > well as the clipboard selection. > Thanks, this is a reasonable feature to add. I'll review this one in a bit. There are a few interactions that need to be checked before getting this in.
Jan Alonzo
Comment 8 2008-11-29 23:52:52 PST
Comment on attachment 24698 [details] Copy link locations to the primary selection Alp to review..
Alexander Butenko
Comment 9 2009-01-12 21:36:56 PST
'Primary clipboard copy support' is already landed. i applied 'Copy link locations to the primary selection' patch but still cant paste nothing to editable areas in webkit.
Sergio García-Cuevas González
Comment 10 2009-01-28 15:46:33 PST
Created attachment 27129 [details] Preliminary primary selection paste support Here is my attempt to support middle-click pasting. The target editable area must be selected, so it only works when the selection comes from an external application or when a clipboard manager is running (and then the selection comes from an external application, too).
Sergio García-Cuevas González
Comment 11 2009-01-28 15:56:00 PST
Comment on attachment 27129 [details] Preliminary primary selection paste support >Index: WebKit/gtk/ChangeLog >=================================================================== >--- WebKit/gtk/ChangeLog (revision 40327) >+++ WebKit/gtk/ChangeLog (working copy) >@@ -1,3 +1,13 @@ >+2009-01-28 Sergio García-Cuevas <sergio_gcg@hotmail.com> >+ >+ https://bugs.webkit.org/show_bug.cgi?id=17267 >+ >+ Implement primary selection support (pasting into editable regions). >+ >+ * WebCoreSupport/EditorClientGtk.cpp: >+ (WebKit::EditorClient::buttonPressed): >+ (WebKit::EditorClient::EditorClient): >+ > 2009-01-27 Brady Eidson <beidson@apple.com> > > Reviewed by Dan Bernstein >Index: WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp >=================================================================== >--- WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp (revision 40327) >+++ WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp (working copy) >@@ -37,6 +37,23 @@ using namespace WebCore; > > namespace WebKit { > >+static void buttonPressed(GtkWidget* widget, GdkEventButton* event, EditorClient* client) >+{ >+ if (event->button == 2 && event->type == GDK_BUTTON_PRESS) { >+ Frame* frame = core(client->m_webView)->focusController()->focusedOrMainFrame(); >+ Editor* editor = frame->editor(); >+ if (editor->canPaste()) { >+ GtkClipboard* clipboard = gtk_widget_get_clipboard(GTK_WIDGET(client->m_webView), GDK_SELECTION_PRIMARY); >+ gchar* utf8 = gtk_clipboard_wait_for_text(clipboard); >+ if (utf8) { >+ String text = String::fromUTF8(utf8); >+ g_free(utf8); >+ editor->insertText(text, 0); >+ } >+ } >+ } >+} >+ > static void imContextCommitted(GtkIMContext* context, const gchar* str, EditorClient* client) > { > Frame* targetFrame = core(client->m_webView)->focusController()->focusedOrMainFrame(); >@@ -470,6 +490,7 @@ EditorClient::EditorClient(WebKitWebView > WebKitWebViewPrivate* priv = m_webView->priv; > g_signal_connect(priv->imContext, "commit", G_CALLBACK(imContextCommitted), this); > g_signal_connect(priv->imContext, "preedit-changed", G_CALLBACK(imContextPreeditChanged), this); >+ g_signal_connect_after(m_webView, "button-press-event", G_CALLBACK(buttonPressed), this); > } > > EditorClient::~EditorClient()
Sergio García-Cuevas González
Comment 12 2009-01-28 16:09:09 PST
(In reply to comment #11) I misplaced the braces in my latest patch. Then I tried to correct it in an obviously wrong way. I think I should have some rest.
Gustavo Noronha (kov)
Comment 13 2009-04-22 19:38:07 PDT
Comment on attachment 24698 [details] Copy link locations to the primary selection > + WARNING: NO TEST CASES ADDED OR CHANGED This warning should be removed. There is no behavior change for webcore; we could probably add GTK+ unit tests for this, though, in WebKit/gtk. > +GtkClipboard* PasteboardHelperGtk::getPrimary(Frame* frame) const { > + WebKitWebView* webView = webkit_web_frame_get_web_view(kit(frame)); > + return gtk_widget_get_clipboard(GTK_WIDGET (webView), > + GDK_SELECTION_PRIMARY); > +} > + I believe the only problem with this patch is the incorrect placement of braces here. Whoever lands, please fix this, and rs=me on a second commit to fix the other braces in this file. I wonder if we should review the PasteBoard files placements as a whole, sometime.
Jan Alonzo
Comment 14 2009-04-24 17:07:30 PDT
Comment on attachment 24698 [details] Copy link locations to the primary selection Landed in r42855. Style fixes landed in r42866. Clearing review flag.
Gustavo Noronha (kov)
Comment 15 2009-05-06 14:54:48 PDT
Comment on attachment 27129 [details] Preliminary primary selection paste support > +static void buttonPressed(GtkWidget* widget, GdkEventButton* event, EditorClient* client) > +{ > + if (event->button == 2 && event->type == GDK_BUTTON_PRESS) > + { > + Frame* frame = core(client->m_webView)->focusController()->focusedOrMainFrame(); > + Editor* editor = frame->editor(); > + if (editor->canPaste()) > + { The patch looks right, but the if braces are bad. They should look like this: if (a) { blahblah(); } Also, we should return early, where possible, so I would change that if to: if (!editor->canPaste()) return; I thought about suggesting making only 1 early-return if, but I believe we may want to use this callback for other features in the future, so it makes sense this way. r=me with those changes.
Gustavo Noronha (kov)
Comment 16 2009-05-06 16:49:04 PDT
Comment on attachment 27129 [details] Preliminary primary selection paste support Actually, this does not seem to work. The entries do not generate a button-press-event for the view.
Gustavo Noronha (kov)
Comment 17 2009-05-12 10:14:27 PDT
*** This bug has been marked as a duplicate of 25685 ***
Note You need to log in before you can comment on or make changes to this bug.