[Webkit-unassigned] [Bug 135795] [EFL][WK2] Minibrowser : Enhance application to be able to support history list navigation

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=135795





--- Comment #8 from Ryuan Choi <ryuan.choi at samsung.com>  2014-08-19 22:20:09 PST ---
(From update of attachment 236752)
View in context: https://bugs.webkit.org/attachment.cgi?id=236752&action=review

> Tools/MiniBrowser/efl/main.c:1062
> +    if (!longpress_enabled) {

Nit, why don't just escape using below?

if (longpress_enabled) return;

> Tools/MiniBrowser/efl/main.c:1143
> +    ewk_view_navigate_to(window->ewk_view, eina_list_nth(history_list_get(user_data), item_index-1)); 

space between - ?

> Tools/MiniBrowser/efl/main.c:1149
> +on_navigation_button_longpress_process(void *user_data)

on_ looks not required

> Tools/MiniBrowser/efl/main.c:1151
> +    if (!longpress_enabled) {

Isn't early return better?

> Tools/MiniBrowser/efl/main.c:1182
> +        for (index = 0; index < item_count; index++) {
> +            title = ewk_back_forward_list_item_title_get(eina_list_nth(list, index));
> +            info(" title = %s", title);
> +            elm_genlist_item_append(window->history.history_list, list_item, (void*)(title), NULL, ELM_GENLIST_ITEM_NONE, on_list_item_select, user_data);
> +        }

Why don't you use EINA_LIST_FOREACH ?
It's cheaper than the loop with eina_list_nth and usual way which EFL does.

> Tools/MiniBrowser/efl/main.c:1195
> +            if (forward_navigation_enabled) {
> +                evas_object_move(window->history.history_box , x+TOOL_BAR_BUTTON_SIZE+1, y+TOOL_BAR_BUTTON_SIZE);
> +                evas_object_move(window->history.history_list , x+TOOL_BAR_BUTTON_SIZE+1, y+TOOL_BAR_BUTTON_SIZE);  
> +            } else {
> +                evas_object_move(window->history.history_box , x, y+TOOL_BAR_BUTTON_SIZE);
> +                evas_object_move(window->history.history_list , x, y+TOOL_BAR_BUTTON_SIZE);  
> +            }

space between + ?

> Tools/MiniBrowser/efl/main.c:1213
> +    forward_navigation_enabled = EINA_TRUE;

Should we keep this as static variable?

Can we pass this as the argument of on_navigation_button_longpress_process ?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.