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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 20 01:23:10 PDT 2014


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





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

> Tools/MiniBrowser/efl/main.c:992
> +    if (forward_navigation_enabled)
> +        history_items = ewk_back_forward_list_n_forward_items_copy(list, count); 
> +    else
> +        history_items = ewk_back_forward_list_n_back_items_copy(list, count);

ewk_back_forward_list_{back|forward}_items_copy looks better. (without _n_ and count)

> Tools/MiniBrowser/efl/main.c:1133
> +list_item_label_get(void *data, Evas_Object *obj, const char* part)
> +{
> +    int len = strlen((char*)data);
> +    char* buf = (char*)malloc(sizeof(char)*(len+1));

The position of * is different.

> Tools/MiniBrowser/efl/main.c:1134
> +    snprintf(buf, len+1, "%s", (char*)data);

space between +

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

With you comment, I understood that forward_navigation_enabled looks required because of this line.

I think that we should not do this.
While showing history box, ewk_view can navigate back or forward so history can be changed.

So, item_index might not be same with newly allocated history_items.

Instead, I think that we should pass the Ewk_Back_Forward_List_Item as user_data.

I'll leave the comment more about it below.

> Tools/MiniBrowser/efl/main.c:1159
> +    Eina_List* list = history_list_get(user_data);

Leak. ( and check the position of *)

Below is doxygen of ewk_back_forward_list_n_back_items_copy.

 * @return @c Eina_List containing @c Ewk_Back_Forward_List_Item elements or @c NULL in case of error,
 *            the Eina_List and its items should be freed after use. Use ewk_object_unref()
 *            to free the items

> Tools/MiniBrowser/efl/main.c:1161
> +    const Eina_List* l;
> +    void* data;

The position of *

> Tools/MiniBrowser/efl/main.c:1186
> +        elm_genlist_item_append(window->history.history_list, list_item, (void*)(title), NULL, ELM_GENLIST_ITEM_NONE, on_list_item_select, user_data);

Instead of passing user_data(which is Browser_Window), we should pass Ewk_Back_Forward_List_Item after called evas_object_ref().

I think that user_data can be passed to the callbacks using evas_object_data_set()

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



More information about the webkit-unassigned mailing list