Bug 114728

Summary: [EFL][WK2] Add sub menus to MiniBrowser
Product: WebKit Reporter: Michal Pakula vel Rutka <mpakulavelrutka>
Component: WebKit EFLAssignee: Michal Pakula vel Rutka <mpakulavelrutka>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, lucas.de.marchi
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on: 114729, 116549    
Bug Blocks:    
Attachments:
Description Flags
proposed patch
none
hiding menu on window resize added
none
hiding menu on window resize added
cdumez: review-
added nulling context_menu.ewk_menu
none
added nulling context_menu.ewk_menu none

Description Michal Pakula vel Rutka 2013-04-17 02:24:44 PDT
MiniBrowser should support sub menus.
Comment 1 Michal Pakula vel Rutka 2013-04-25 02:45:34 PDT
Created attachment 199643 [details]
proposed patch

Current implementation uses Elementary Ctxpopup widget, which does not support submenus. To implement submenus in Minibrowser, widget should be changed to Elementary Menu widget.
Comment 2 Chris Dumez 2013-05-21 02:16:15 PDT
Comment on attachment 199643 [details]
proposed patch

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

> Tools/MiniBrowser/efl/main.c:1180
> +    Evas_Coord ewk_x, ewk_y;

Why is this coordinate translation needed now?
Comment 3 Michal Pakula vel Rutka 2013-05-21 02:23:22 PDT
Comment on attachment 199643 [details]
proposed patch

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

>> Tools/MiniBrowser/efl/main.c:1180
>> +    Evas_Coord ewk_x, ewk_y;
> 
> Why is this coordinate translation needed now?

It is just something I forgot to add in earlier patch. Parent object for elm_menu is elm_window and the coordinates received from WebKit are webview coordinates, which position differs from window because of i.e. url bar.
Comment 4 Jinwoo Song 2013-05-21 04:09:27 PDT
Comment on attachment 199643 [details]
proposed patch

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

>>> Tools/MiniBrowser/efl/main.c:1180
>>> +    Evas_Coord ewk_x, ewk_y;
>> 
>> Why is this coordinate translation needed now?
> 
> It is just something I forgot to add in earlier patch. Parent object for elm_menu is elm_window and the coordinates received from WebKit are webview coordinates, which position differs from window because of i.e. url bar.

The context menu is created in wrong places when the window is resized.
Comment 5 Michal Pakula vel Rutka 2013-05-21 10:22:20 PDT
(In reply to comment #4)
> (From update of attachment 199643 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=199643&action=review
> 
> >>> Tools/MiniBrowser/efl/main.c:1180
> >>> +    Evas_Coord ewk_x, ewk_y;
> >> 
> >> Why is this coordinate translation needed now?
> > 
> > It is just something I forgot to add in earlier patch. Parent object for elm_menu is elm_window and the coordinates received from WebKit are webview coordinates, which position differs from window because of i.e. url bar.
> 
> The context menu is created in wrong places when the window is resized.

We should disable window resizing during showing context menu/select popup, as it is done in other browsers. I will try to find solution for that.
Comment 6 Chris Dumez 2013-05-22 03:02:16 PDT
Comment on attachment 199643 [details]
proposed patch

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

>>>>> Tools/MiniBrowser/efl/main.c:1180
>>>>> +    Evas_Coord ewk_x, ewk_y;
>>>> 
>>>> Why is this coordinate translation needed now?
>>> 
>>> It is just something I forgot to add in earlier patch. Parent object for elm_menu is elm_window and the coordinates received from WebKit are webview coordinates, which position differs from window because of i.e. url bar.
>> 
>> The context menu is created in wrong places when the window is resized.
> 
> We should disable window resizing during showing context menu/select popup, as it is done in other browsers. I will try to find solution for that.

I personally think it would be better to dismiss the context menu when the window gets resized.
Comment 7 Michal Pakula vel Rutka 2013-05-22 05:18:10 PDT
Created attachment 202527 [details]
hiding menu on window resize added
Comment 8 Michal Pakula vel Rutka 2013-05-22 05:19:34 PDT
Created attachment 202528 [details]
hiding menu on window resize added
Comment 9 Chris Dumez 2013-05-22 05:24:34 PDT
It crashes when I maximize the window and the context menu is shown:

Showing context menu at (457, 355).

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff78c08f0 in ewk_object_is_of_type<EwkContextMenu*> (object=0x17db8c0)
    at /home/chris/devel/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_object_private.h:40
40	    return (reinterpret_cast<T>(0)->className() == object->instanceClassName());
(gdb) bt
#0  0x00007ffff78c08f0 in ewk_object_is_of_type<EwkContextMenu*> (object=0x17db8c0)
    at /home/chris/devel/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_object_private.h:40
#1  0x00007ffff78c0717 in ewk_object_cast_check<EwkContextMenu*> (object=0x17db8c0)
    at /home/chris/devel/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_object_private.h:48
#2  0x00007ffff78c0614 in ewk_object_cast<EwkContextMenu*> (object=0x17db8c0)
    at /home/chris/devel/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_object_private.h:67
#3  0x00007ffff78bfb80 in ewk_context_menu_hide (menu=0x17db8c0)
    at /home/chris/devel/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_context_menu.cpp:128
#4  0x0000000000405602 in on_window_resize (user_data=0x47ec30, e=0x480260, elm_window=0x8006b0, event_info=0x0)
    at /home/chris/devel/WebKit/Tools/MiniBrowser/efl/main.c:275
#5  0x00007ffff7e35252 in evas_object_event_callback_call (obj=obj@entry=0x8006b0, type=type@entry=EVAS_CALLBACK_RESIZE, 
    event_info=event_info@entry=0x0, event_id=10884) at evas_callbacks.c:232
#6  0x00007ffff7e4f99e in evas_object_inform_call_resize (obj=0x8006b0) at evas_object_inform.c:38
#7  0x00007ffff7e4cfd1 in evas_object_resize (obj=<optimized out>, w=<optimized out>, h=<optimized out>) at evas_object_main.c:753
#8  0x00007ffff6dfa228 in _elm_win_resize_job (data=0x88e180) at elm_win.c:578
#9  0x00007ffff7fd56cb in _ecore_job_event_handler (data=<optimized out>, type=<optimized out>, ev=<optimized out>) at ecore_job.c:115
#10 0x00007ffff7fd210c in _ecore_call_handler_cb (event=<optimized out>, type=<optimized out>, data=<optimized out>, func=<optimized out>)
    at ecore_private.h:321
#11 _ecore_event_call () at ecore_events.c:559
#12 0x00007ffff7fd6909 in _ecore_main_loop_iterate_internal (once_only=once_only@entry=0) at ecore_main.c:1922
#13 0x00007ffff7fd6e97 in ecore_main_loop_begin () at ecore_main.c:956
#14 0x0000000000409bfa in elm_main (argc=1, argv=0x7fffffffe0d8) at /home/chris/devel/WebKit/Tools/MiniBrowser/efl/main.c:1719
#15 0x0000000000409c3c in main (argc=1, argv=0x7fffffffe0d8) at /home/chris/devel/WebKit/Tools/MiniBrowser/efl/main.c:1723
Comment 10 Chris Dumez 2013-05-22 05:26:05 PDT
Comment on attachment 202528 [details]
hiding menu on window resize added

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

r- due to crashing.

> Tools/MiniBrowser/efl/main.c:1620
>  

nit: useless blank line here.
Comment 11 Michal Pakula vel Rutka 2013-05-22 06:01:18 PDT
(In reply to comment #9)
> It crashes when I maximize the window and the context menu is shown:
> 
> Showing context menu at (457, 355).
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x00007ffff78c08f0 in ewk_object_is_of_type<EwkContextMenu*> (object=0x17db8c0)
>     at /home/chris/devel/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_object_private.h:40
> 40        return (reinterpret_cast<T>(0)->className() == object->instanceClassName());
> (gdb) bt
> #0  0x00007ffff78c08f0 in ewk_object_is_of_type<EwkContextMenu*> (object=0x17db8c0)
>     at /home/chris/devel/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_object_private.h:40
> #1  0x00007ffff78c0717 in ewk_object_cast_check<EwkContextMenu*> (object=0x17db8c0)
>     at /home/chris/devel/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_object_private.h:48
> #2  0x00007ffff78c0614 in ewk_object_cast<EwkContextMenu*> (object=0x17db8c0)
>     at /home/chris/devel/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_object_private.h:67
> #3  0x00007ffff78bfb80 in ewk_context_menu_hide (menu=0x17db8c0)
>     at /home/chris/devel/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_context_menu.cpp:128
> #4  0x0000000000405602 in on_window_resize (user_data=0x47ec30, e=0x480260, elm_window=0x8006b0, event_info=0x0)
>     at /home/chris/devel/WebKit/Tools/MiniBrowser/efl/main.c:275
> #5  0x00007ffff7e35252 in evas_object_event_callback_call (obj=obj@entry=0x8006b0, type=type@entry=EVAS_CALLBACK_RESIZE, 
>     event_info=event_info@entry=0x0, event_id=10884) at evas_callbacks.c:232
> #6  0x00007ffff7e4f99e in evas_object_inform_call_resize (obj=0x8006b0) at evas_object_inform.c:38
> #7  0x00007ffff7e4cfd1 in evas_object_resize (obj=<optimized out>, w=<optimized out>, h=<optimized out>) at evas_object_main.c:753
> #8  0x00007ffff6dfa228 in _elm_win_resize_job (data=0x88e180) at elm_win.c:578
> #9  0x00007ffff7fd56cb in _ecore_job_event_handler (data=<optimized out>, type=<optimized out>, ev=<optimized out>) at ecore_job.c:115
> #10 0x00007ffff7fd210c in _ecore_call_handler_cb (event=<optimized out>, type=<optimized out>, data=<optimized out>, func=<optimized out>)
>     at ecore_private.h:321
> #11 _ecore_event_call () at ecore_events.c:559
> #12 0x00007ffff7fd6909 in _ecore_main_loop_iterate_internal (once_only=once_only@entry=0) at ecore_main.c:1922
> #13 0x00007ffff7fd6e97 in ecore_main_loop_begin () at ecore_main.c:956
> #14 0x0000000000409bfa in elm_main (argc=1, argv=0x7fffffffe0d8) at /home/chris/devel/WebKit/Tools/MiniBrowser/efl/main.c:1719
> #15 0x0000000000409c3c in main (argc=1, argv=0x7fffffffe0d8) at /home/chris/devel/WebKit/Tools/MiniBrowser/efl/main.c:1723

window->context_menu.ewk_menu was not nulled after menu was hidden by WebKit
Comment 12 Michal Pakula vel Rutka 2013-05-22 06:05:50 PDT
Created attachment 202531 [details]
added nulling context_menu.ewk_menu
Comment 13 Chris Dumez 2013-05-22 06:15:18 PDT
wr(In reply to comment #12)
> Created an attachment (id=202531) [details]
> added nulling context_menu.ewk_menu

wrong patch
Comment 14 Michal Pakula vel Rutka 2013-05-22 06:19:40 PDT
Created attachment 202532 [details]
added nulling context_menu.ewk_menu
Comment 15 Chris Dumez 2013-05-22 06:37:42 PDT
Comment on attachment 202532 [details]
added nulling context_menu.ewk_menu

LGTM. r=me.

However, the "go back" item shows as enabled on a page where going back is impossible. This is not related to this patch as the value provided by WebKit is wrong. Could you please look into this issue?
Comment 16 WebKit Commit Bot 2013-05-22 07:16:30 PDT
Comment on attachment 202532 [details]
added nulling context_menu.ewk_menu

Clearing flags on attachment: 202532

Committed r150515: <http://trac.webkit.org/changeset/150515>
Comment 17 WebKit Commit Bot 2013-05-22 07:16:33 PDT
All reviewed patches have been landed.  Closing bug.