Bug 16311 – [Gtk] Copy rich text to clipboard as text/plain and text/html

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Luca Bruno]

We can paste text/html and text/plain to editable contents such as rich text, but we can't copy from.

[Luca Bruno]

Created attachment 17769 [details]
patch

Add targets for plain text and text/html, then complete the implementation of copying text from the page. Adding targets meant adding new API, hopefully this is not wrong.

Please consider reviewing deeply the PasteboardGtk.cpp file which has a new struct.

[Alp Toker]

Comment on attachment 17769 [details]
patch

Hey Luca,

Unfortunately this patch is going to need some re-designing.

nothing in platform/gtk should use anything from WebKit/gtk. WebKit/gtk should be built on top of platform, never the other way round.

This means that we shouldn't need to add webkit_web_view_get_target_list() as public API either. It should remain totally internal, just like GtkTextView.

(The feature itself works well and this'll be a great feature to have.)

[Luca Bruno]

Created attachment 17770 [details]
cleaner patch

Diffs:
- Use a class instead of a struct in the C++ context, which would be useful for future enhancements like URIs and GtkTextBuffer data
- Create target entries inside Pasteboard
- Do not use API except for WebKitWebViewTargetInfo enum

[Luca Bruno]

Created attachment 17773 [details]
do not use webkit enum

Diffs:
- Define webkit target info enum
- Use the right GdkAtom

Pasteboard in in mac is used for DnD not for Pasteboard, therefore they can pass their PasteboardHelper from webkit. Instead, there's no way to do this for the Pasteboard.

[Alp Toker]

GtkTextBuffer has:

GtkTargetList*      gtk_text_buffer_get_copy_target_list
                                                        (GtkTextBuffer *buffer);
GtkTargetList*      gtk_text_buffer_get_paste_target_list
                                                        (GtkTextBuffer *buffer);

Is there a reason why this patch adds only one? webkit_web_view_get_target_list()

Just wondering, have not studied the case in detail.

[Luca Bruno]

Didn't think we need two different target lists. I just thought the copy target was the same as the paste target. As i see the text buffer has two target lists with the same contents.

[Luca Bruno]

Created attachment 17972 [details]
PasteboardHelper and use the right clipboard

Well i created a PasteboardHelper (please check the license) and a target list for both copy and paste operations.

[Alp Toker]

Mitch has offered to look into this one for clipboard API correctness. There are some minor coding style issues like whitespace before braces from a quick glance.

[Luca Bruno]

Created attachment 18005 [details]
coding style issues

[Alp Toker]

Comment on attachment 18005 [details]
coding style issues

r=me (thanks to Xan and Mitch for looking into this one too)

Will land with the discussed documentation and whitespace fixes.

[Alp Toker]

Landed in r28963.