Bug 97460 - [GTK] Add Select All method to WebKit2 GTK+ API
Summary: [GTK] Add Select All method to WebKit2 GTK+ API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Pena
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-24 09:30 PDT by Simon Pena
Modified: 2012-09-24 12:53 PDT (History)
6 users (show)

See Also:


Attachments
Patch (6.42 KB, patch)
2012-09-24 09:45 PDT, Simon Pena
no flags Details | Formatted Diff | Diff
Patch (6.17 KB, patch)
2012-09-24 11:07 PDT, Simon Pena
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Pena 2012-09-24 09:30:43 PDT
[GTK] Add Select All method to WebKit2 GTK+ API
Comment 1 Simon Pena 2012-09-24 09:45:07 PDT
Created attachment 165400 [details]
Patch
Comment 2 Simon Pena 2012-09-24 09:48:06 PDT
Updating component and adding Carlos to CC.

Carlos, I'm not sure if the unit test is a bit redundat right now, checking the same both when it's editable or not. What do you think? Do you think I should be testing anything else?
Comment 3 WebKit Review Bot 2012-09-24 09:50:05 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 4 Martin Robinson 2012-09-24 10:36:42 PDT
Comment on attachment 165400 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=165400&action=review

Nice work. The tests you have written are more than adequate, in my opinion. I just have a few minor nits. Do you mind just uploading a new patch and then I can commit it for you.

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebViewEditor.cpp:59
> +    void executeEditingCommand(const char* command)
> +    {
> +        webkit_web_view_execute_editing_command(m_webView, command);
> +    }
> +

Since this method just does one thing, it probably makes a bit more sense just do call webkit_web_view_execute_editing_command from the test. It makes it a bit more clear what API you are actually testing there.

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebViewEditor.cpp:157
> +    // Initially only the subspan is selected

Just a minor nit here. Comments in WebKit should have a period at the end.
Comment 5 Simon Pena 2012-09-24 11:07:08 PDT
Created attachment 165414 [details]
Patch
Comment 6 Simon Pena 2012-09-24 11:07:52 PDT
(In reply to comment #5)
> Created an attachment (id=165414) [details]
> Patch

Addressing Martin's comments here
Comment 7 WebKit Review Bot 2012-09-24 12:53:22 PDT
Comment on attachment 165414 [details]
Patch

Clearing flags on attachment: 165414

Committed r129399: <http://trac.webkit.org/changeset/129399>
Comment 8 WebKit Review Bot 2012-09-24 12:53:26 PDT
All reviewed patches have been landed.  Closing bug.