|
|
Created:
4 years, 8 months ago by Navid Zolghadr Modified:
4 years, 7 months ago CC:
chromium-reviews, blink-reviews, dtapuska+blinkwatch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionPrevent sending click event for non primary button
Prevent sending click and dblclick events when the
button in charge is a non-primary mouse button.
As the result of this change there is no way to
prevent the behavior of opening a new tap when
middle clicking on a link. Previously, js could
listen to click event of the middle button and
call preventDefault on it.
BUG=255
Committed: https://crrev.com/76fea00a18f75886ea649414393228180306e13d
Cr-Commit-Position: refs/heads/master@{#390195}
Patch Set 1 #Patch Set 2 : Fixing tests #
Total comments: 10
Patch Set 3 : Use stopPropagation #Patch Set 4 : Remove empty line #
Total comments: 10
Patch Set 5 : Removing the test #
Total comments: 1
Patch Set 6 : Rebased #
Messages
Total messages: 38 (12 generated)
nzolghadr@chromium.org changed reviewers: + dtapuska@chromium.org, mustaq@chromium.org, rbyers@google.com
This is a possible fix for the middle click issue. Note the complication of default behavior of middle click which is opening a new tab and now transferring the preventDefault on middle click event to middle button mouse up to prevent opening a new tab. I haven't updated the tests yet. Let me know if this generally looks good and then I looked at the failed tests.
Description was changed from ========== Prevent sending click event for non primary button Prevent sending click events when the button in charge is anything but primary mouse button. Also considers preventDefault action on mouseup event for middle button to prevent default behaviour of click for middle button which is openning a new tab. BUG=255 ========== to ========== Prevent sending click event for non primary button Prevent sending click events when the button in charge is anything but primary mouse button. Also considers preventDefault action on mouseup event for middle button to prevent default behaviour of click for middle button which is openning a new tab. BUG=255 ==========
nzolghadr@chromium.org changed reviewers: - rbyers@google.com
ptal. Following our discussion from yesterday's standup I just removed the preventDefault behavior altogether.
https://codereview.chromium.org/1894253002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/events/mouse-click-events-expected.txt (right): https://codereview.chromium.org/1894253002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/events/mouse-click-events-expected.txt:9: FAIL eventLog should be mousedown(1) mouseup(1) click(1) mousedown(1) mouseup(1) click(1) dblclick(1) . Was mousedown(1) mouseup(1) mousedown(1) mouseup(1) . I think we should change the mouse-click-events.html as well as the expected outcome so it says PASS.. https://codereview.chromium.org/1894253002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/events/EventDispatcher.cpp (left): https://codereview.chromium.org/1894253002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/events/EventDispatcher.cpp:123: if (dispatchEventPreProcess(preDispatchEventHandlerResult) == ContinueDispatching) { Adding this here seems incorrect and a number of layering violations. I imagine other browsers will allow: node.dispatchEvent( new MouseEvent("click", {button: 1}); to be dispatched no? Conceptually I'd expect us to just avoid generating the click event here: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit... But I presume I am missing something. https://codereview.chromium.org/1894253002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/events/MouseEvent.cpp (right): https://codereview.chromium.org/1894253002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/events/MouseEvent.cpp:298: if (mouseEvent.button() == MouseButton::MiddleButton) I don't think this is what the spec indicates. It indicates click should only be sent for the primary button; ie; what about right click? https://codereview.chromium.org/1894253002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/input/EventHandler.cpp (left): https://codereview.chromium.org/1894253002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/input/EventHandler.cpp:1480: && !(selectionController().hasExtendedSelection() && isLinkSelection(mev)); This change just seems like formatting...
https://codereview.chromium.org/1894253002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/events/mouse-click-events-expected.txt (right): https://codereview.chromium.org/1894253002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/events/mouse-click-events-expected.txt:9: FAIL eventLog should be mousedown(1) mouseup(1) click(1) mousedown(1) mouseup(1) click(1) dblclick(1) . Was mousedown(1) mouseup(1) mousedown(1) mouseup(1) . On 2016/04/22 18:32:30, dtapuska wrote: > I think we should change the mouse-click-events.html as well as the expected > outcome so it says PASS.. Oops. I meant to do that. I missed it and mistakenly just added the output. https://codereview.chromium.org/1894253002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/events/EventDispatcher.cpp (left): https://codereview.chromium.org/1894253002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/events/EventDispatcher.cpp:123: if (dispatchEventPreProcess(preDispatchEventHandlerResult) == ContinueDispatching) { On 2016/04/22 18:32:30, dtapuska wrote: > Adding this here seems incorrect and a number of layering violations. > > I imagine other browsers will allow: > node.dispatchEvent( new MouseEvent("click", {button: 1}); to be dispatched no? > > Conceptually I'd expect us to just avoid generating the click event here: > https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit... > > But I presume I am missing something. I realized this file shouldn't be for mouse specific checks. But there was already some specific checks for mouse in this file. So I thought it shouldn't be that bad. But FireFox does let that fake event to be fired which I didn't know it would pass this part of the code and as a result we do not let them fire that after this CL which obviously is incorrect. I wanted to have this suppressing logic in EventHandler in the first place. But the problem is calling the default behavior of click in EventHandler. Basically if this event is suppressed in EventHandler layer the new tab will not open. What do you suggest about that? Here it traverses up in the tree and calls the default behavior on all the parents. I assume it does more logic anyway around that code. Do you think kind of copying that logic (say only the default behavior calling on all the parents) to EventHandler or somewhat having it exposed to EventHandler with some functions is better? https://codereview.chromium.org/1894253002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/events/MouseEvent.cpp (right): https://codereview.chromium.org/1894253002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/events/MouseEvent.cpp:298: if (mouseEvent.button() == MouseButton::MiddleButton) On 2016/04/22 18:32:30, dtapuska wrote: > I don't think this is what the spec indicates. It indicates click should only be > sent for the primary button; ie; what about right click? Right. So much focusing on middlebutton that I forgot that here. I used !=leftbutton in other places though. https://codereview.chromium.org/1894253002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/input/EventHandler.cpp (left): https://codereview.chromium.org/1894253002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/input/EventHandler.cpp:1480: && !(selectionController().hasExtendedSelection() && isLinkSelection(mev)); On 2016/04/22 18:32:30, dtapuska wrote: > This change just seems like formatting... Sure. I change it back to what it was.
https://codereview.chromium.org/1894253002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/events/EventDispatcher.cpp (left): https://codereview.chromium.org/1894253002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/events/EventDispatcher.cpp:123: if (dispatchEventPreProcess(preDispatchEventHandlerResult) == ContinueDispatching) { On 2016/04/22 19:09:58, Navid Zolghadr wrote: > On 2016/04/22 18:32:30, dtapuska wrote: > > Adding this here seems incorrect and a number of layering violations. > > > > I imagine other browsers will allow: > > node.dispatchEvent( new MouseEvent("click", {button: 1}); to be dispatched no? > > > > Conceptually I'd expect us to just avoid generating the click event here: > > > https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit... > > > > But I presume I am missing something. > > I realized this file shouldn't be for mouse specific checks. But there was > already some specific checks for mouse in this file. So I thought it shouldn't > be that bad. But FireFox does let that fake event to be fired which I didn't > know it would pass this part of the code and as a result we do not let them fire > that after this CL which obviously is incorrect. > > I wanted to have this suppressing logic in EventHandler in the first place. But > the problem is calling the default behavior of click in EventHandler. Basically > if this event is suppressed in EventHandler layer the new tab will not open. > What do you suggest about that? Here it traverses up in the tree and calls the > default behavior on all the parents. I assume it does more logic anyway around > that code. > > Do you think kind of copying that logic (say only the default behavior calling > on all the parents) to EventHandler or somewhat having it exposed to > EventHandler with some functions is better? What about removing this and adding: if (nativeEvent.button() != LeftButton) event->stopPropagation(); In Node.cpp dispatchMouseEvent... Would that get the best of both worlds?
On 2016/04/22 19:16:33, dtapuska wrote: > https://codereview.chromium.org/1894253002/diff/20001/third_party/WebKit/Sour... > File third_party/WebKit/Source/core/events/EventDispatcher.cpp (left): > > https://codereview.chromium.org/1894253002/diff/20001/third_party/WebKit/Sour... > third_party/WebKit/Source/core/events/EventDispatcher.cpp:123: if > (dispatchEventPreProcess(preDispatchEventHandlerResult) == ContinueDispatching) > { > On 2016/04/22 19:09:58, Navid Zolghadr wrote: > > On 2016/04/22 18:32:30, dtapuska wrote: > > > Adding this here seems incorrect and a number of layering violations. > > > > > > I imagine other browsers will allow: > > > node.dispatchEvent( new MouseEvent("click", {button: 1}); to be dispatched > no? > > > > > > Conceptually I'd expect us to just avoid generating the click event here: > > > > > > https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit... > > > > > > But I presume I am missing something. > > > > I realized this file shouldn't be for mouse specific checks. But there was > > already some specific checks for mouse in this file. So I thought it shouldn't > > be that bad. But FireFox does let that fake event to be fired which I didn't > > know it would pass this part of the code and as a result we do not let them > fire > > that after this CL which obviously is incorrect. > > > > I wanted to have this suppressing logic in EventHandler in the first place. > But > > the problem is calling the default behavior of click in EventHandler. > Basically > > if this event is suppressed in EventHandler layer the new tab will not open. > > What do you suggest about that? Here it traverses up in the tree and calls the > > default behavior on all the parents. I assume it does more logic anyway around > > that code. > > > > Do you think kind of copying that logic (say only the default behavior calling > > on all the parents) to EventHandler or somewhat having it exposed to > > EventHandler with some functions is better? > > What about removing this and adding: > > if (nativeEvent.button() != LeftButton) > event->stopPropagation(); > > In Node.cpp dispatchMouseEvent... > > Would that get the best of both worlds? I think stopping propagation should get the parent default behavior right. The corresponding Webkit bug (https://bugs.webkit.org/show_bug.cgi?id=22382) seems to suggest "internal-only click event" are desired for middle clicks. Does the desired "internal" include the default behavior in parents? I don't have the precise answer. My quick suggestion is: if any attempted fix in Webkit relied on stopPropagation, we should take a closer look why it failed.
ptal. I believe the stoppropagation fixed it and it is very clean. https://codereview.chromium.org/1894253002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/events/EventDispatcher.cpp (left): https://codereview.chromium.org/1894253002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/events/EventDispatcher.cpp:123: if (dispatchEventPreProcess(preDispatchEventHandlerResult) == ContinueDispatching) { On 2016/04/22 19:16:32, dtapuska wrote: > On 2016/04/22 19:09:58, Navid Zolghadr wrote: > > On 2016/04/22 18:32:30, dtapuska wrote: > > > Adding this here seems incorrect and a number of layering violations. > > > > > > I imagine other browsers will allow: > > > node.dispatchEvent( new MouseEvent("click", {button: 1}); to be dispatched > no? > > > > > > Conceptually I'd expect us to just avoid generating the click event here: > > > > > > https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit... > > > > > > But I presume I am missing something. > > > > I realized this file shouldn't be for mouse specific checks. But there was > > already some specific checks for mouse in this file. So I thought it shouldn't > > be that bad. But FireFox does let that fake event to be fired which I didn't > > know it would pass this part of the code and as a result we do not let them > fire > > that after this CL which obviously is incorrect. > > > > I wanted to have this suppressing logic in EventHandler in the first place. > But > > the problem is calling the default behavior of click in EventHandler. > Basically > > if this event is suppressed in EventHandler layer the new tab will not open. > > What do you suggest about that? Here it traverses up in the tree and calls the > > default behavior on all the parents. I assume it does more logic anyway around > > that code. > > > > Do you think kind of copying that logic (say only the default behavior calling > > on all the parents) to EventHandler or somewhat having it exposed to > > EventHandler with some functions is better? > > What about removing this and adding: > > if (nativeEvent.button() != LeftButton) > event->stopPropagation(); > > In Node.cpp dispatchMouseEvent... > > Would that get the best of both worlds? Done.
https://codereview.chromium.org/1894253002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/events/MouseEvent.cpp (right): https://codereview.chromium.org/1894253002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/events/MouseEvent.cpp:297: // Do not send dblclick event for middle click Can we update the comment so it matches the code? And add a period :-) https://codereview.chromium.org/1894253002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/input/EventHandler.cpp (right): https://codereview.chromium.org/1894253002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/input/EventHandler.cpp:1499: if (mev.event().button() != MouseButton::LeftButton) Perhaps add a comment about the bug number here? https://codereview.chromium.org/1894253002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/input/EventHandler.cpp:1501: return toWebInputEventResult(clickTargetNode->dispatchEvent(event)); This is different logic than in the past... clickEventResult was getting assigned before what was the motivation for changing that?
On 2016/04/25 19:59:37, dtapuska wrote: > https://codereview.chromium.org/1894253002/diff/60001/third_party/WebKit/Sour... > File third_party/WebKit/Source/core/events/MouseEvent.cpp (right): > > https://codereview.chromium.org/1894253002/diff/60001/third_party/WebKit/Sour... > third_party/WebKit/Source/core/events/MouseEvent.cpp:297: // Do not send > dblclick event for middle click > Can we update the comment so it matches the code? And add a period :-) > > https://codereview.chromium.org/1894253002/diff/60001/third_party/WebKit/Sour... > File third_party/WebKit/Source/core/input/EventHandler.cpp (right): > > https://codereview.chromium.org/1894253002/diff/60001/third_party/WebKit/Sour... > third_party/WebKit/Source/core/input/EventHandler.cpp:1499: if > (mev.event().button() != MouseButton::LeftButton) > Perhaps add a comment about the bug number here? > > https://codereview.chromium.org/1894253002/diff/60001/third_party/WebKit/Sour... > third_party/WebKit/Source/core/input/EventHandler.cpp:1501: return > toWebInputEventResult(clickTargetNode->dispatchEvent(event)); > This is different logic than in the past... clickEventResult was getting > assigned before what was the motivation for changing that? Test failures seem real too.
https://codereview.chromium.org/1894253002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/events/MouseEvent.cpp (right): https://codereview.chromium.org/1894253002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/events/MouseEvent.cpp:294: if (mouseEvent.type() != EventTypeNames::click || mouseEvent.detail() != 2) This |if| suggests that we would reach Line 297 only for the second click in a double-click (type==click && details==2). What am I missing here?
https://codereview.chromium.org/1894253002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/events/MouseEvent.cpp (right): https://codereview.chromium.org/1894253002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/events/MouseEvent.cpp:294: if (mouseEvent.type() != EventTypeNames::click || mouseEvent.detail() != 2) On 2016/04/25 20:12:43, mustaq wrote: > This |if| suggests that we would reach Line 297 only for the second click in a > double-click (type==click && details==2). What am I missing here? That is right. And we should suppress the dblclick of the non-primary button clicks as the result. Right?
https://codereview.chromium.org/1894253002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/events/MouseEvent.cpp (right): https://codereview.chromium.org/1894253002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/events/MouseEvent.cpp:294: if (mouseEvent.type() != EventTypeNames::click || mouseEvent.detail() != 2) On 2016/04/25 20:17:38, Navid Zolghadr wrote: > On 2016/04/25 20:12:43, mustaq wrote: > > This |if| suggests that we would reach Line 297 only for the second click in a > > double-click (type==click && details==2). What am I missing here? > > That is right. And we should suppress the dblclick of the non-primary button > clicks as the result. Right? I see. Please update the CL description accordingly.
Description was changed from ========== Prevent sending click event for non primary button Prevent sending click events when the button in charge is anything but primary mouse button. Also considers preventDefault action on mouseup event for middle button to prevent default behaviour of click for middle button which is openning a new tab. BUG=255 ========== to ========== Prevent sending click event for non primary button Prevent sending click and dblclick events when the button in charge is a non-primary mouse button. As the result of this change there is no way to prevent the behavior of opening a new tap when middle clicking on a link. Previously, js could listen to click event of the middle button and call preventDefault on it. BUG=255 ==========
ptal. It seems that the test was doing an action on click action of middle button so I had to remove it as it is not valid anymore. https://codereview.chromium.org/1894253002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/events/MouseEvent.cpp (right): https://codereview.chromium.org/1894253002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/events/MouseEvent.cpp:294: if (mouseEvent.type() != EventTypeNames::click || mouseEvent.detail() != 2) On 2016/04/25 20:47:34, mustaq wrote: > On 2016/04/25 20:17:38, Navid Zolghadr wrote: > > On 2016/04/25 20:12:43, mustaq wrote: > > > This |if| suggests that we would reach Line 297 only for the second click in > a > > > double-click (type==click && details==2). What am I missing here? > > > > That is right. And we should suppress the dblclick of the non-primary button > > clicks as the result. Right? > > I see. Please update the CL description accordingly. Done. https://codereview.chromium.org/1894253002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/events/MouseEvent.cpp:297: // Do not send dblclick event for middle click On 2016/04/25 19:59:36, dtapuska wrote: > Can we update the comment so it matches the code? And add a period :-) Done. https://codereview.chromium.org/1894253002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/input/EventHandler.cpp (right): https://codereview.chromium.org/1894253002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/input/EventHandler.cpp:1499: if (mev.event().button() != MouseButton::LeftButton) On 2016/04/25 19:59:37, dtapuska wrote: > Perhaps add a comment about the bug number here? Done. https://codereview.chromium.org/1894253002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/input/EventHandler.cpp:1501: return toWebInputEventResult(clickTargetNode->dispatchEvent(event)); On 2016/04/25 19:59:36, dtapuska wrote: > This is different logic than in the past... clickEventResult was getting > assigned before what was the motivation for changing that? My bad! Thanks for catching it.
lgtm https://codereview.chromium.org/1894253002/diff/80001/chrome/browser/ui/brows... File chrome/browser/ui/browser_browsertest.cc (left): https://codereview.chromium.org/1894253002/diff/80001/chrome/browser/ui/brows... chrome/browser/ui/browser_browsertest.cc:2639: // http://crbug.com/396347 Can you please ensure that crbug gets closed as wontfix as the test case has been removed?
On 2016/04/26 17:11:56, dtapuska wrote: > lgtm > > https://codereview.chromium.org/1894253002/diff/80001/chrome/browser/ui/brows... > File chrome/browser/ui/browser_browsertest.cc (left): > > https://codereview.chromium.org/1894253002/diff/80001/chrome/browser/ui/brows... > chrome/browser/ui/browser_browsertest.cc:2639: // http://crbug.com/396347 > Can you please ensure that crbug gets closed as wontfix as the test case has > been removed? The test is not fully removed. There is still another test called HrefShiftMiddleClickTest (which is testing middle click on a link). What I removed was WindowOpenMiddleClickTest which is also mentioned in that bug. But the original test is still there. I will put a comment in that bug regarding these removed tests but the bug is still valid for the other test I assume.
nzolghadr@chromium.org changed reviewers: + bokan@chromium.org
bokan@chromium.org: Please review changes in third_party/WebKit/Source/core/events/MouseEvent.cpp third_party/WebKit/Source/core/input/EventHandler.cpp
WebKit core lgtm
nzolghadr@chromium.org changed reviewers: + thestig@chromium.org
thestig@chromium.org: Please review changes in chrome/browser/ui/browser_browsertest.cc
lgtm
The CQ bit was checked by nzolghadr@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1894253002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1894253002/80001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...)
The CQ bit was checked by nzolghadr@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org, dtapuska@chromium.org, bokan@chromium.org Link to the patchset: https://codereview.chromium.org/1894253002/#ps100001 (title: "Rebased")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1894253002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1894253002/100001
Message was sent while issue was closed.
Description was changed from ========== Prevent sending click event for non primary button Prevent sending click and dblclick events when the button in charge is a non-primary mouse button. As the result of this change there is no way to prevent the behavior of opening a new tap when middle clicking on a link. Previously, js could listen to click event of the middle button and call preventDefault on it. BUG=255 ========== to ========== Prevent sending click event for non primary button Prevent sending click and dblclick events when the button in charge is a non-primary mouse button. As the result of this change there is no way to prevent the behavior of opening a new tap when middle clicking on a link. Previously, js could listen to click event of the middle button and call preventDefault on it. BUG=255 ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001)
Message was sent while issue was closed.
If I read the description correctly, this change means that pages / extensions no longer have a way to know if the user middle clicked? If so, this kills the "Navigate on paste" extension - on Linux middle click traditionally does a paste action. The extension catches middle click and then looks at the clipboard and does a navigate if it contains a URL. I don't know how many users rely on that extension or other JS that wants to catch middle click, but thought I'd point out that there are legitimate reasons for the behavior.
Message was sent while issue was closed.
On 2016/04/28 06:11:04, vandebo (ex-Chrome) wrote: > If I read the description correctly, this change means that pages / extensions > no longer have a way to know if the user middle clicked? If so, this kills the > "Navigate on paste" extension - on Linux middle click traditionally does a paste > action. The extension catches middle click and then looks at the clipboard and > does a navigate if it contains a URL. > > I don't know how many users rely on that extension or other JS that wants to > catch middle click, but thought I'd point out that there are legitimate reasons > for the behavior. I tried your extension against a build with Navid's change and it seems to work fine. The code you are hooking is the paste on the body via this. Are you looking for middle click somewhere? Cause I don't see it. // Copyright (c) 2013 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. if (window == top) { document.getElementsByTagName('body')[0].onpaste = pasteHandler; } function pasteHandler(e) { var t = e.target.type; if (t == 'textarea' || t == 'text' || t == 'password' || e.target.isContentEditable) { return; } var url = e.clipboardData.getData('text/plain').replace(/^\s+|\s+$/g, ''); chrome.runtime.sendMessage({url: url}) }
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/76fea00a18f75886ea649414393228180306e13d Cr-Commit-Position: refs/heads/master@{#390195}
Message was sent while issue was closed.
Description was changed from ========== Prevent sending click event for non primary button Prevent sending click and dblclick events when the button in charge is a non-primary mouse button. As the result of this change there is no way to prevent the behavior of opening a new tap when middle clicking on a link. Previously, js could listen to click event of the middle button and call preventDefault on it. BUG=255 ========== to ========== Prevent sending click event for non primary button Prevent sending click and dblclick events when the button in charge is a non-primary mouse button. As the result of this change there is no way to prevent the behavior of opening a new tap when middle clicking on a link. Previously, js could listen to click event of the middle button and call preventDefault on it. BUG=255 Committed: https://crrev.com/76fea00a18f75886ea649414393228180306e13d Cr-Commit-Position: refs/heads/master@{#390195} ========== |