Bug 136736 – [EFL][WK2] Minibrowser : Add support for mouse middle button to open links in new window

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Rohit]

Currently EFL minibrowser supports opening new links in new window using context menu. Add support for opening links in new window (tab in case of tabbed browser) using mouse wheel like all other browsers.

[Rohit]

Created attachment 237937 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 237937 [details]
Patch

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

> Tools/MiniBrowser/efl/main.c:1891
> +    Ewk_Navigation_Policy_Decision* decision = (Ewk_Navigation_Policy_Decision*)event_info;

Wrong * place. Missing a space between "(Ewk_Navigation_Policy_Decision" and "*)"

> Tools/MiniBrowser/efl/main.c:1900
> +        ewk_navigation_policy_decision_reject(decision); 

Doesn't we call ewk_navigation_policy_decision_accept() ? And It looks we need to call this reject function when ewk_navigation_policy_mouse_button_get(decision) is not EWK_EVENT_MOUSE_BUTTON_MIDDLE, isn't it ?

[Rohit]

Created attachment 237944 [details]
Patch

[Rohit]

(In reply to comment #2)
> (From update of attachment 237937 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=237937&action=review
> 
> > Tools/MiniBrowser/efl/main.c:1891
> > +    Ewk_Navigation_Policy_Decision* decision = (Ewk_Navigation_Policy_Decision*)event_info;
> 
> Wrong * place. Missing a space between "(Ewk_Navigation_Policy_Decision" and "*)"
> 

Done.

> > Tools/MiniBrowser/efl/main.c:1900
> > +        ewk_navigation_policy_decision_reject(decision); 
> 
> Doesn't we call ewk_navigation_policy_decision_accept() ? And It looks we need to call this reject function when ewk_navigation_policy_mouse_button_get(decision) is not EWK_EVENT_MOUSE_BUTTON_MIDDLE, isn't it ?

As per the bug https://bugs.webkit.org/show_bug.cgi?id=90953
by default the navigation request is accepted. So I think there is no need to call ewk_navigation_policy_decision_accept().
Also if we call reject function when ewk_navigation_policy_mouse_button_get(decision) is not EWK_EVENT_MOUSE_BUTTON_MIDDLE, then we will not be able to navigate to any page for all cases other than mouse middle button case. And adding reject at the end makes sure that we do not open the same page in parent and child window.

[Build Bot]

Comment on attachment 237944 [details]
Patch

Attachment 237944 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5960791274553344

New failing tests:
transitions/color-transition-rounding.html
transitions/border-radius-transition.html
transitions/change-values-during-transition.html
transitions/clip-transition.html
transitions/background-transitions.html
transitions/created-while-suspended.html

[Build Bot]

Created attachment 237950 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Gyuyoung Kim]

(In reply to comment #4)

> > > Tools/MiniBrowser/efl/main.c:1900
> > > +        ewk_navigation_policy_decision_reject(decision); 
> > 
> > Doesn't we call ewk_navigation_policy_decision_accept() ? And It looks we need to call this reject function when ewk_navigation_policy_mouse_button_get(decision) is not EWK_EVENT_MOUSE_BUTTON_MIDDLE, isn't it ?
> 
> As per the bug https://bugs.webkit.org/show_bug.cgi?id=90953
> by default the navigation request is accepted. So I think there is no need to call ewk_navigation_policy_decision_accept().
> Also if we call reject function when ewk_navigation_policy_mouse_button_get(decision) is not EWK_EVENT_MOUSE_BUTTON_MIDDLE, then we will not be able to navigate to any page for all cases other than mouse middle button case. And adding reject at the end makes sure that we do not open the same page in parent and child window.

I don't understand why we should call reject() yet. Could you explain it again ?

[Gyuyoung Kim]

Comment on attachment 237944 [details]
Patch

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

> Tools/MiniBrowser/efl/main.c:1891
> +    Ewk_Navigation_Policy_Decision *decision = (Ewk_Navigation_Policy_Decision*)event_info;

nit: (Ewk_Navigation_Policy_Decision*) -> (Ewk_Navigation_Policy_Decision *)

> Tools/MiniBrowser/efl/main.c:1895
> +

Unneeded line.

[Rohit]

(In reply to comment #7)
> (In reply to comment #4)
> 
> > > > Tools/MiniBrowser/efl/main.c:1900
> > > > +        ewk_navigation_policy_decision_reject(decision); 
> > > 
> > > Doesn't we call ewk_navigation_policy_decision_accept() ? And It looks we need to call this reject function when ewk_navigation_policy_mouse_button_get(decision) is not EWK_EVENT_MOUSE_BUTTON_MIDDLE, isn't it ?
> > 
> > As per the bug https://bugs.webkit.org/show_bug.cgi?id=90953
> > by default the navigation request is accepted. So I think there is no need to call ewk_navigation_policy_decision_accept().
> > Also if we call reject function when ewk_navigation_policy_mouse_button_get(decision) is not EWK_EVENT_MOUSE_BUTTON_MIDDLE, then we will not be able to navigate to any page for all cases other than mouse middle button case. And adding reject at the end makes sure that we do not open the same page in parent and child window.
> 
> I don't understand why we should call reject() yet. Could you explain it again ?

After creating new window, we need to reject the decision for the parent window so that it doesn't navigate to the same page as the child window. Calling reject() makes sure that original behaviour of "mouse middle button click on links -> naviage to new page" is taken care of.

[Rohit]

Created attachment 239326 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 239326 [details]
Patch

LGTM.

[WebKit Commit Bot]

Comment on attachment 239326 [details]
Patch

Clearing flags on attachment: 239326

Committed r175049: <http://trac.webkit.org/changeset/175049>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.