Bug 17267

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.

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.

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

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

Comment on attachment 23321 [details] Primary clipboard copy support Landed in r36323. Still need paste support to close this bug.

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.

(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.

Comment on attachment 24698 [details] Copy link locations to the primary selection Alp to review..

'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.

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

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

(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.

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.

Comment on attachment 24698 [details] Copy link locations to the primary selection Landed in r42855. Style fixes landed in r42866. Clearing review flag.

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.

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.

*** This bug has been marked as a duplicate of 25685 ***