[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 #14 from Tanay <tanay.c at samsung.com>  2014-08-20 06:35:17 PST ---
(In reply to comment #11)
> (From update of attachment 236858 [details])
> 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)
> 
Done.

> > 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.
> 
Done.

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

> > 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.
> 

Modified. More details 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
>
Using EINA_LIST_FREE to free the list and data.

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

> > 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()

Appending the Ewk_Back_Forward_List_Item instead of user_data. Passing the window pointer using evas_object_data_set. Now ewk_view_navigate_to() in on_list_item_select uses these values instead of index.

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