RESOLVED FIXED 137110
[GTK] Implement is_selected method on WebKitHitTestResult
https://bugs.webkit.org/show_bug.cgi?id=137110
Summary [GTK] Implement is_selected method on WebKitHitTestResult
Marcos Chavarría Teijeiro (irc: chavaone)
Reported 2014-09-25 01:45:13 PDT
We need a way to check if the user is selecting something when we get an instance of WKHitTestResult. We could do this with WK1 [1]. I have check the sources and the function exists, we only have to expose some new API functions.
Attachments
Patch (7.74 KB, patch)
2014-09-26 03:50 PDT, Marcos Chavarría Teijeiro (irc: chavaone)
no flags
Patch (9.24 KB, patch)
2014-09-29 00:39 PDT, Marcos Chavarría Teijeiro (irc: chavaone)
no flags
Patch (24.93 KB, patch)
2014-10-02 01:14 PDT, Marcos Chavarría Teijeiro (irc: chavaone)
no flags
Patch (24.99 KB, patch)
2014-10-02 02:01 PDT, Marcos Chavarría Teijeiro (irc: chavaone)
no flags
Patch (24.99 KB, patch)
2014-10-02 02:38 PDT, Marcos Chavarría Teijeiro (irc: chavaone)
no flags
Marcos Chavarría Teijeiro (irc: chavaone)
Comment 1 2014-09-26 03:50:18 PDT
Marcos Chavarría Teijeiro (irc: chavaone)
Comment 2 2014-09-29 00:39:16 PDT
Created attachment 238843 [details] Patch Change variable names to make them similar to WK1 API.
Carlos Garcia Campos
Comment 3 2014-09-29 03:44:29 PDT
Comment on attachment 238843 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=238843&action=review Thanks for the patch, it looks good to me in general, but new API patches should include a unit test (see Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestUIClient.cpp and TestContextMenu.cpp). Also this patch changes cross-platform code, so it requires the approval of a WebKit2 owner. > Source/WebKit2/UIProcess/API/gtk/WebKitHitTestResult.cpp:366 > + */ Since: 2.8 > Source/WebKit2/UIProcess/API/gtk/WebKitHitTestResult.h:51 > + * @WEBKIT_HIT_TEST_RESULT_CONTEXT_SELECTION: a selected element. Since: 2.8. > Source/WebKit2/UIProcess/API/gtk/WebKitHitTestResult.h:100 > +webkit_hit_test_result_context_is_selection (WebKitHitTestResult *hit_test_result); I think there's an extra space between function name and parentheses.
Marcos Chavarría Teijeiro (irc: chavaone)
Comment 4 2014-10-02 01:14:46 PDT
Created attachment 239089 [details] Patch Add tests and fix some minor details.
Carlos Garcia Campos
Comment 5 2014-10-02 01:44:57 PDT
Comment on attachment 239089 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=239089&action=review Looks good to me, apart from the minor issues I mentioned. > Source/WebKit2/UIProcess/API/gtk/WebKitHitTestResult.cpp:367 > + * Since 2.8. Since: 2.8 > Source/WebKit2/UIProcess/API/gtk/WebKitHitTestResult.h:55 > + * Since 2.8. This here means the enum was introduced in 2.8, but it's only the value. You should add the Since tag after the flag. * @WEBKIT_HIT_TEST_RESULT_CONTEXT_SELECTION: a selected element. Since 2.8 > Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestContextMenu.cpp:408 > + // Context menu for selection. > + test->m_expectedMenuType = ContextMenuDefaultTest::Selection; > + test->showContextMenuAtPositionAndWaitUntilFinished(2, 115); I guess this should go first because otherwise any other click would remove the selection, right? I would add a comment explaining why this should always be the first check.
Marcos Chavarría Teijeiro (irc: chavaone)
Comment 6 2014-10-02 02:01:29 PDT
Created attachment 239093 [details] Patch Explain test and fix since related issues.
Carlos Garcia Campos
Comment 7 2014-10-02 02:24:14 PDT
Comment on attachment 239093 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=239093&action=review > Source/WebKit2/UIProcess/API/gtk/WebKitHitTestResult.cpp:367 > + * Since 2.8 Since: 2.8 This requires the ':', the regexp used by gtk-doc to parse this is: } elsif (m%^\s*since:%i) {
Marcos Chavarría Teijeiro (irc: chavaone)
Comment 8 2014-10-02 02:38:48 PDT
Created attachment 239097 [details] Patch Add colon to Since tag.
Tim Horton
Comment 9 2014-10-23 11:15:53 PDT
Comment on attachment 239097 [details] Patch r+ given cgarcia's LGTM+adjustments and a review of the WebKit2 bits.
Carlos Alberto Lopez Perez
Comment 10 2014-10-28 04:18:03 PDT
Seems that commit-queue forgot to update the flags or close the bug. This was committed on r175160 <http://trac.webkit.org/r175160> Closing
Note You need to log in before you can comment on or make changes to this bug.