WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(9.24 KB, patch)
2014-09-29 00:39 PDT
,
Marcos Chavarría Teijeiro (irc: chavaone)
no flags
Details
Formatted Diff
Diff
Patch
(24.93 KB, patch)
2014-10-02 01:14 PDT
,
Marcos Chavarría Teijeiro (irc: chavaone)
no flags
Details
Formatted Diff
Diff
Patch
(24.99 KB, patch)
2014-10-02 02:01 PDT
,
Marcos Chavarría Teijeiro (irc: chavaone)
no flags
Details
Formatted Diff
Diff
Patch
(24.99 KB, patch)
2014-10-02 02:38 PDT
,
Marcos Chavarría Teijeiro (irc: chavaone)
no flags
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Marcos Chavarría Teijeiro (irc: chavaone)
Comment 1
2014-09-26 03:50:18 PDT
Created
attachment 238707
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug