Summary: | [EFL][WK2] Minibrowser : Add support for mouse middle button to open links in new window | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Rohit <kumar.rohit> | ||||||||||
Component: | WebKit EFL | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | buildbot, commit-queue, gyuyoung.kim, lucas.de.marchi, rniwa, ryuan.choi | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Rohit
2014-09-10 23:26:44 PDT
Created attachment 237937 [details]
Patch
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 ? Created attachment 237944 [details]
Patch
(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 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 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
(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 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. (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. Created attachment 239326 [details]
Patch
Comment on attachment 239326 [details]
Patch
LGTM.
Comment on attachment 239326 [details] Patch Clearing flags on attachment: 239326 Committed r175049: <http://trac.webkit.org/changeset/175049> All reviewed patches have been landed. Closing bug. |