Bug 136736 - [EFL][WK2] Minibrowser : Add support for mouse middle button to open links in new window
Summary: [EFL][WK2] Minibrowser : Add support for mouse middle button to open links in...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-09-10 23:26 PDT by Rohit
Modified: 2014-10-22 08:09 PDT (History)
6 users (show)

See Also:


Attachments
Patch (2.80 KB, patch)
2014-09-10 23:39 PDT, Rohit
no flags Details | Formatted Diff | Diff
Patch (2.74 KB, patch)
2014-09-11 02:04 PDT, Rohit
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (555.21 KB, application/zip)
2014-09-11 03:08 PDT, Build Bot
no flags Details
Patch (2.65 KB, patch)
2014-10-06 03:58 PDT, Rohit
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rohit 2014-09-10 23:26:44 PDT
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.
Comment 1 Rohit 2014-09-10 23:39:44 PDT
Created attachment 237937 [details]
Patch
Comment 2 Gyuyoung Kim 2014-09-10 23:44:44 PDT
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 ?
Comment 3 Rohit 2014-09-11 02:04:39 PDT
Created attachment 237944 [details]
Patch
Comment 4 Rohit 2014-09-11 02:05:54 PDT
(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.
Comment 5 Build Bot 2014-09-11 03:08:17 PDT
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
Comment 6 Build Bot 2014-09-11 03:08:21 PDT
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
Comment 7 Gyuyoung Kim 2014-10-06 01:34:41 PDT
(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 ?
Comment 8 Gyuyoung Kim 2014-10-06 01:35:36 PDT
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.
Comment 9 Rohit 2014-10-06 02:04:50 PDT
(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.
Comment 10 Rohit 2014-10-06 03:58:14 PDT
Created attachment 239326 [details]
Patch
Comment 11 Gyuyoung Kim 2014-10-21 17:33:52 PDT
Comment on attachment 239326 [details]
Patch

LGTM.
Comment 12 WebKit Commit Bot 2014-10-22 08:09:46 PDT
Comment on attachment 239326 [details]
Patch

Clearing flags on attachment: 239326

Committed r175049: <http://trac.webkit.org/changeset/175049>
Comment 13 WebKit Commit Bot 2014-10-22 08:09:55 PDT
All reviewed patches have been landed.  Closing bug.