Bug 112122 – [EFL][MiniBrowser] Add a search field to the MiniBrowser

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Jinwoo Song]

Add a search field to test API ewk_view_text_find().

[Jinwoo Song]

change the component to WebKit EFL.

[Jinwoo Song]

Created attachment 192686 [details]
Patch

[Build Bot]

Comment on attachment 192686 [details]
Patch

Attachment 192686 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17155178

New failing tests:
editing/selection/selection-modify-crash.html

[Jinwoo Song]

Created attachment 192851 [details]
Patch

Add EWK_FIND_OPTIONS_WRAP_AROUND option in finding text.

[Chris Dumez]

Comment on attachment 192851 [details]
Patch

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

Patch needs to be rebased.

> Tools/MiniBrowser/efl/main.c:96
> +    Evas_Object *search_bar;

Could we group all those 4 members in a "search" sub struct?

> Tools/MiniBrowser/efl/main.c:98
> +    Evas_Object *search_up_button;

search_backward_button?

> Tools/MiniBrowser/efl/main.c:99
> +    Evas_Object *search_down_button;

search_forward_button?

> Tools/MiniBrowser/efl/main.c:280
> +    elm_object_focus_set(elm_object_top_widget_get(window->elm_window), EINA_FALSE);

Is this line really needed?

> Tools/MiniBrowser/efl/main.c:356
> +        search_box_hide(window);

I would exit fullscreen OR hide search but not both at the same time. How about the following:
1. If search box is displayed, hide it and abort.
2. If search box is hidden and window is fullscreen, exit fullscreen.

> Tools/MiniBrowser/efl/main.c:671
> +    ewk_view_text_find(window->ewk_view, text, EWK_FIND_OPTIONS_SHOW_HIGHLIGHT | EWK_FIND_OPTIONS_CASE_INSENSITIVE | EWK_FIND_OPTIONS_WRAP_AROUND, 100);

Maybe we could add a global constant for this 100 magic value.
Also maybe we could keep "EWK_FIND_OPTIONS_SHOW_HIGHLIGHT | EWK_FIND_OPTIONS_CASE_INSENSITIVE | EWK_FIND_OPTIONS_WRAP_AROUND" in a global "DEFAULT_SEARCH_FLAGS" constant to simplify the code a bit.

> Tools/MiniBrowser/efl/main.c:710
> +on_search_up_button_clicked(void *user_data, Evas_Object *search_up_button, void *event_info)

on_search_backward_button_clicked?

> Tools/MiniBrowser/efl/main.c:720
> +on_search_down_button_clicked(void *user_data, Evas_Object *search_down_button, void *event_info)

on_search_forward_button_clicked?

> Tools/MiniBrowser/efl/main.c:1160
> +on_window_resize(void *user_data, Evas *e, Evas_Object *elm_window, void *event_info)

This should really not be needed if we do the layout properly.

> Tools/MiniBrowser/efl/main.c:1203
> +    evas_object_event_callback_add(window->elm_window, EVAS_CALLBACK_RESIZE, on_window_resize, window);

Ditto.

[Jinwoo Song]

Created attachment 193095 [details]
Patch

Rebased the patch and applied Christophe's comments. Thanks for review. :)

[Kenneth Rohde Christiansen]

Comment on attachment 193095 [details]
Patch

r=me but check with chris before landing

[Chris Dumez]

Comment on attachment 193095 [details]
Patch

Looks fine, thanks.

[WebKit Review Bot]

Comment on attachment 193095 [details]
Patch

Clearing flags on attachment: 193095

Committed r145863: <http://trac.webkit.org/changeset/145863>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Mikhail Pozdnyakov]

Looks like it brings regression: a Gray stripe at the bottom of the window when launch mini browser. Will look at this to find more details..

[Sudarsana Nagineni (babu)]

(In reply to comment #11)
> Looks like it brings regression: a Gray stripe at the bottom of the window when launch mini browser. Will look at this to find more details..

I also noticed the same issue. Created a bug about it bug# 112687.