WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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-
Details
Formatted Diff
Diff
Primary clipboard copy support
(3.94 KB, patch)
2008-09-10 08:32 PDT
,
Alp Toker
no flags
Details
Formatted Diff
Diff
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
Details
Formatted Diff
Diff
Preliminary primary selection paste support
(2.25 KB, patch)
2009-01-28 15:46 PST
,
Sergio García-Cuevas González
gustavo
: review-
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
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.
Top of Page
Format For Printing
XML
Clone This Bug