WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
134486
[EFL][WK2] Minibrowser: Enhance the application to show text search count
https://bugs.webkit.org/show_bug.cgi?id=134486
Summary
[EFL][WK2] Minibrowser: Enhance the application to show text search count
Shivakumar J M
Reported
2014-06-30 22:14:08 PDT
currently in many browsers the search count will be shown in browser search bar, so Enhance the application to show text search count. These will show case use on on_text_fond() call back as well. also add search close button as well.
Attachments
Patch
(8.02 KB, patch)
2014-07-01 03:21 PDT
,
Shivakumar J M
no flags
Details
Formatted Diff
Diff
searchbar-new-ui
(164.13 KB, image/png)
2014-07-01 03:50 PDT
,
Shivakumar J M
no flags
Details
Patch-Updated-Review
(6.78 KB, patch)
2014-07-02 22:38 PDT
,
Shivakumar J M
no flags
Details
Formatted Diff
Diff
Patch for landing
(6.78 KB, patch)
2014-07-02 22:48 PDT
,
Shivakumar J M
buildbot
: commit-queue-
Details
Formatted Diff
Diff
searchbar-new-updated
(905.69 KB, image/png)
2014-07-02 22:50 PDT
,
Shivakumar J M
no flags
Details
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2
(522.48 KB, application/zip)
2014-07-02 23:54 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2
(539.12 KB, application/zip)
2014-07-03 01:01 PDT
,
Build Bot
no flags
Details
Patch-Updated-Review2
(6.65 KB, patch)
2014-07-03 01:57 PDT
,
Shivakumar J M
no flags
Details
Formatted Diff
Diff
Patch for landing
(6.60 KB, patch)
2014-07-03 21:24 PDT
,
Shivakumar J M
no flags
Details
Formatted Diff
Diff
Patch for landing
(6.67 KB, patch)
2014-07-03 21:59 PDT
,
Shivakumar J M
no flags
Details
Formatted Diff
Diff
Show Obsolete
(6)
View All
Add attachment
proposed patch, testcase, etc.
Shivakumar J M
Comment 1
2014-07-01 03:21:50 PDT
Created
attachment 234153
[details]
Patch Use 'text,found' callback to show search count.
Shivakumar J M
Comment 2
2014-07-01 03:50:25 PDT
Created
attachment 234154
[details]
searchbar-new-ui image showing the new search bar ui
Ryuan Choi
Comment 3
2014-07-02 01:42:40 PDT
Comment on
attachment 234153
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=234153&action=review
> Tools/ChangeLog:3 > + [EFL][WK2] Minibrowser: Enhance the application to show text search count
Enhance the Minibrowser to show text search count and close search bar
> Tools/MiniBrowser/efl/main.c:1871 > + window->search.search_field_count = elm_entry_add(window->elm_window);
Why do you use entry? IMO, elm_label looks enough. At least, it should be readonly, right?
> Tools/MiniBrowser/efl/main.c:1882 > + window->search.close_button = create_toolbar_button(window->elm_window, "close");
Can we locate this button at right most position?
Gyuyoung Kim
Comment 4
2014-07-02 09:24:32 PDT
Comment on
attachment 234153
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=234153&action=review
> Tools/MiniBrowser/efl/main.c:1700 > + int *matchCount = (int*)(event_info);
s/matchCount/match_count/g (int*)(event_info); -> (const int *)(event_info); ?
Shivakumar J M
Comment 5
2014-07-02 22:38:13 PDT
Created
attachment 234318
[details]
Patch-Updated-Review Updated patch with review comments, used label to show search count text, moved close button to end.
Shivakumar J M
Comment 6
2014-07-02 22:48:17 PDT
Created
attachment 234319
[details]
Patch for landing Updated patch with comments fixed.
Shivakumar J M
Comment 7
2014-07-02 22:50:27 PDT
Created
attachment 234320
[details]
searchbar-new-updated image showing the new search bar ui
Build Bot
Comment 8
2014-07-02 23:54:21 PDT
Comment on
attachment 234319
[details]
Patch for landing
Attachment 234319
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/5541842615533568
New failing tests: media/W3C/video/networkState/networkState_during_loadstart.html
Build Bot
Comment 9
2014-07-02 23:54:24 PDT
Created
attachment 234323
[details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-09 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Ryuan Choi
Comment 10
2014-07-03 00:25:46 PDT
Comment on
attachment 234319
[details]
Patch for landing View in context:
https://bugs.webkit.org/attachment.cgi?id=234319&action=review
> Tools/MiniBrowser/efl/main.c:1703 > + evas_object_focus_set(window->ewk_view, EINA_FALSE); > + elm_object_focus_set(window->search.search_field_count, EINA_TRUE);
Can I know why you touch the focus (and repeatedly)?
> Tools/MiniBrowser/efl/main.c:1715 > + evas_object_focus_set(window->ewk_view, EINA_FALSE); > + elm_object_focus_set(window->search.search_field_count, EINA_FALSE); > + elm_object_focus_set(window->search.search_field, EINA_TRUE);
Ditto.
> Tools/MiniBrowser/efl/main.c:1881 > + elm_object_text_set(window->search.search_field_count, " ");
Is space needed?
Build Bot
Comment 11
2014-07-03 01:00:57 PDT
Comment on
attachment 234319
[details]
Patch for landing
Attachment 234319
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/4876152749424640
New failing tests: media/media-fragments/TC0001.html
Build Bot
Comment 12
2014-07-03 01:01:02 PDT
Created
attachment 234324
[details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-10 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Shivakumar J M
Comment 13
2014-07-03 01:57:35 PDT
Created
attachment 234326
[details]
Patch-Updated-Review2 Removed the space at line 1881, as we do not need it. Removed the focus set at 1703, which is not needed. some time while testing i observed, search field was not getting proper focus, so i added those lines at 1715. i.e after updating search text found count on label, set focus on search field.
Gyuyoung Kim
Comment 14
2014-07-03 20:35:14 PDT
Comment on
attachment 234326
[details]
Patch-Updated-Review2 View in context:
https://bugs.webkit.org/attachment.cgi?id=234326&action=review
LGTM except for nits.
> Tools/MiniBrowser/efl/main.c:1700 > + const int *matchCount = (const int *)(event_info);
matchCount is not efl style. match_count ?
> Tools/MiniBrowser/efl/main.c:1963 > + evas_object_smart_callback_add(window->ewk_view, "text,found", on_search_text_found, window);
Personally I prefer to place it alphabetically.
Shivakumar J M
Comment 15
2014-07-03 21:24:38 PDT
Created
attachment 234389
[details]
Patch for landing
Gyuyoung Kim
Comment 16
2014-07-03 21:31:43 PDT
Comment on
attachment 234389
[details]
Patch for landing View in context:
https://bugs.webkit.org/attachment.cgi?id=234389&action=review
> Tools/ChangeLog:8 > + Use 'text,found' callback to show search count.
I think you need to mention on_search_close_button_clicked() callback and on_search_text_found().
Shivakumar J M
Comment 17
2014-07-03 21:59:35 PDT
Created
attachment 234390
[details]
Patch for landing
WebKit Commit Bot
Comment 18
2014-07-03 22:38:45 PDT
Comment on
attachment 234390
[details]
Patch for landing Clearing flags on attachment: 234390 Committed
r170786
: <
http://trac.webkit.org/changeset/170786
>
WebKit Commit Bot
Comment 19
2014-07-03 22:38:51 PDT
All reviewed patches have been landed. Closing bug.
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