Bug 97873 - [EFL][WK2] Add API to count the matching text
Summary: [EFL][WK2] Add API to count the matching text
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Linux
: P2 Normal
Assignee: Jinwoo Song
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-28 00:19 PDT by Jinwoo Song
Modified: 2012-10-05 03:59 PDT (History)
5 users (show)

See Also:


Attachments
Patch (6.14 KB, patch)
2012-10-03 08:50 PDT, Jinwoo Song
no flags Details | Formatted Diff | Diff
Patch (7.38 KB, patch)
2012-10-04 08:39 PDT, Jinwoo Song
gyuyoung.kim: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
patch for landing. (7.11 KB, patch)
2012-10-05 03:35 PDT, Jinwoo Song
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jinwoo Song 2012-09-28 00:19:28 PDT
We need a API to only count the matching text such as ewk_view_text_count(),
as described in https://bugs.webkit.org/show_bug.cgi?id=97431.

ewk_view_text_find() finds the matching text and but also update the UI view, 
for example, highlight the text. But some application may not want to affect the view,
and just want to know the count.
Comment 1 Jinwoo Song 2012-10-03 08:50:02 PDT
Created attachment 166905 [details]
Patch
Comment 2 Raphael Kubo da Costa (:rakuco) 2012-10-03 08:59:27 PDT
Comment on attachment 166905 [details]
Patch

Looks OK, I guess.
Comment 3 Ryuan Choi 2012-10-04 04:48:07 PDT
Comment on attachment 166905 [details]
Patch

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

Looks fine.

Below are minor which you can choose.

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:828
> +TEST_F(EWK2UnitTestBase, ewk_view_text_matches_count)

Can we add test which max_match_count is smaller than result ?

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:836
> +        "<html>"
> +        "<body>"
> +        "apple Apple apple apple banana bananaApple banana coconut"
> +        "</body>"
> +        "</html>";

For the simplity, you can omit html tag for html5.
Comment 4 Gyuyoung Kim 2012-10-04 05:06:55 PDT
Comment on attachment 166905 [details]
Patch

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

> Source/WebKit2/UIProcess/API/efl/ewk_view.h:620
> +* Searches and hightlights the given string in the document.

Missing a space in front of *

> Source/WebKit2/UIProcess/API/efl/ewk_view.h:641
> +* Counts the given string in the document.

ditto.

> Source/WebKit2/UIProcess/API/efl/ewk_view.h:657
> +* @param max count to find, unlimited if 0

max -> max_match_count
Comment 5 Chris Dumez 2012-10-04 06:56:48 PDT
Comment on attachment 166905 [details]
Patch

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

> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:1612
> +    WKPageCountStringMatches(toAPI(priv->pageProxy.get()), findText.get(), static_cast<WKFindOptions>(options), maxMatchCount);

You could use priv->pageProxy->countStringMatches() directly to avoid useless converting to WK and then back to PageProxy.
Comment 6 Jinwoo Song 2012-10-04 08:37:07 PDT
(In reply to comment #3)
> (From update of attachment 166905 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=166905&action=review
> 
> Looks fine.
> 
> Below are minor which you can choose.
> 
> > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:828
> > +TEST_F(EWK2UnitTestBase, ewk_view_text_matches_count)
> 
> Can we add test which max_match_count is smaller than result ?
> 
I added test case.

> > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:836
> > +        "<html>"
> > +        "<body>"
> > +        "apple Apple apple apple banana bananaApple banana coconut"
> > +        "</body>"
> > +        "</html>";
> 
> For the simplity, you can omit html tag for html5.
Fixed.
Comment 7 Jinwoo Song 2012-10-04 08:37:28 PDT
(In reply to comment #4)
> (From update of attachment 166905 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=166905&action=review
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_view.h:620
> > +* Searches and hightlights the given string in the document.
> 
> Missing a space in front of *
> 
Fixed.
> > Source/WebKit2/UIProcess/API/efl/ewk_view.h:641
> > +* Counts the given string in the document.
> 
> ditto.
> 
Fixed.
> > Source/WebKit2/UIProcess/API/efl/ewk_view.h:657
> > +* @param max count to find, unlimited if 0
> 
> max -> max_match_count
Fixed.
Comment 8 Jinwoo Song 2012-10-04 08:37:49 PDT
(In reply to comment #5)
> (From update of attachment 166905 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=166905&action=review
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:1612
> > +    WKPageCountStringMatches(toAPI(priv->pageProxy.get()), findText.get(), static_cast<WKFindOptions>(options), maxMatchCount);
> 
> You could use priv->pageProxy->countStringMatches() directly to avoid useless converting to WK and then back to PageProxy.

Fixed, Thanks.
Comment 9 Jinwoo Song 2012-10-04 08:39:44 PDT
Created attachment 167110 [details]
Patch
Comment 10 WebKit Review Bot 2012-10-04 20:51:24 PDT
Comment on attachment 167110 [details]
Patch

Rejecting attachment 167110 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

Last 500 characters of output:
Kit/chromium/third_party/yasm/source/patched-yasm --revision 154708 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
51>At revision 154708.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/14152794
Comment 11 Jinwoo Song 2012-10-05 03:35:05 PDT
Created attachment 167300 [details]
patch for landing.
Comment 12 WebKit Review Bot 2012-10-05 03:59:19 PDT
Comment on attachment 167300 [details]
patch for landing.

Clearing flags on attachment: 167300

Committed r130492: <http://trac.webkit.org/changeset/130492>
Comment 13 WebKit Review Bot 2012-10-05 03:59:23 PDT
All reviewed patches have been landed.  Closing bug.