Stop depending on "click" events for middle clicks |
|||||||||||||||||||
Issue description1. Open the Developer Tools here. 2. Go to the Sources panel. 3. Select one of the JavaScript files on the left tree (this will open a tab with the content of that file). 4. Middle click on the tab label above the content of that file. It does not close. In the stable version, it does.
,
May 11 2016
There might multiple places in the Developer Tools that look for "click" and event.button === 1 (or its equivalent event.buttons way). This is one of them. I suggest to broaden the scope of this issue.
,
May 11 2016
"dblclick" is also affected, as far as I know. So I only found two cases (but I might be too specific in my search) - https://code.google.com/p/chromium/codesearch#search/&q=file:devtools%20lang:js%20event.button%5Cb%20-file:/gen/&sq=package:chromium&type=cs
,
Jun 3 2016
,
Jun 8 2016
,
Jun 9 2016
,
Jun 14 2016
Should this be P1? It's been open since May.
,
Jun 14 2016
I would even call it Priority-0, really. Someone knowingly broke this functionality (by fixing a long standing bug) and should fix those internal cases before the release goes stable.
,
Jun 14 2016
There seems to be a few other complaints about the original change. I have a fix for this issue but I might just revert the original change. Give me some time until the end of the June so I can grab more data and reported bugs and I'll talk to the team and see what we can do here. Thanks.
,
Jun 14 2016
If you fix those cases, they will still work even if you revert your fix. But I guess you prefer not to invest the time. Can you mark this as a release blocker then? It should not slip through.
,
Jun 14 2016
No it is not about investing the time. If I revert the original change I need to revert all these fixes as well as fixing them means introducing a synthetic click event for middle button and enabling back the middle click would cause the double handling. If the decision is to revert the original change that would be a hassle to go and revert all these changes. So I thought let's not introduce more changes to revert than I already did.
,
Jun 14 2016
Blocking Chrome 52, that is.
,
Jun 14 2016
if you revert the "only primary mouse button dispatches a click" change, I think you'd only need to the dispatchEvent(click) line. note: you wrote the code to generate a synthetic "click" event as a fix for this. I also remember you saying keeping fallout fixes and a potential revert in sync wouldn't be so bad. now you're saying it's "a hassle to go and revert all these changes". we should try to find a better, future proof solution if continuing to do these fixes is too laborious (i.e. feature detect whether middle click works, and if not: THEN add our polyfill code).
,
Jun 14 2016
** only need to /comment out/ the dispatchEvent(click) line.[1] [1] https://cs.chromium.org/chromium/src/chrome/browser/resources/synthetic_middleclick.js?q=synthetic+middle+click&sq=package:chromium&dr=C&l=19
,
Jun 15 2016
Looks like any desktop platform will be affected, tagging as such, please correct if not the case. In the future please make sure release blockers have an OS tagged, as that's how release managers handle these.
,
Jun 22 2016
Regarding comment 0: What build does this happen in?
,
Jun 22 2016
Any build since this landed - https://chromium.googlesource.com/chromium/src.git/+/76fea00a18f75886ea649414393228180306e13d
,
Jun 27 2016
** IMPORTANT change in M52 merge date due to first 2 weeks of July no release weeks ** M52 Stable is launching very soon! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged ASAP. All changes MUST be merged into the release branch by 5pm on July 1 to make into the desktop Stable final build cut. Thank you!
,
Jun 29 2016
A CL has been posted for this issue: https://codereview.chromium.org/2107093003/
,
Jul 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40 commit 4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40 Author: nzolghadr <nzolghadr@chromium.org> Date: Fri Jul 01 19:02:51 2016 Revert middle click related changes This is the CL to revert the related middle button click changes. We decided to revert those changes because the issues that were caused by suppressing click event for middle button was hard to fix without having that event. Particularly the ability to prevent opening a new tab which can be done by "preventDefault"ing the click event of middle button was removed as the result of the original change. For now we revert these changes and we pursue the line of adding a new event for non-primary button click to be able to fix these problem in a more clean way. Revert "Prevent sending click event for non primary button" This reverts commit 76fea00a18f75886ea649414393228180306e13d. Revert "Dispatch middle click manually by tracking mouse" This reverts commit 88eb1110baafcba070e750866a343e81b6bcc524. Revert "Fix history page middle click action" This reverts commit a154aed1a3813cf28c6f477579ed7974a2528570. BUG= 255 , 618593 , 617444 , 611019 , 617875 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2107093003 Cr-Commit-Position: refs/heads/master@{#403496} [modify] https://crrev.com/4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40/chrome/browser/resources/history/history.js [modify] https://crrev.com/4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40/chrome/browser/resources/ntp4/new_tab.html [delete] https://crrev.com/afd007a5a072039641b7b81eddd9aa488cec4996/chrome/browser/resources/synthetic_middleclick.js [modify] https://crrev.com/4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40/chrome/browser/ui/browser_browsertest.cc [modify] https://crrev.com/4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40/testing/buildbot/filters/browser-side-navigation.linux.browser_tests.filter [modify] https://crrev.com/4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40/third_party/WebKit/LayoutTests/fast/events/mouse-click-events-expected.txt [modify] https://crrev.com/4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40/third_party/WebKit/LayoutTests/fast/events/script-tests/mouse-click-events.js [modify] https://crrev.com/4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40/third_party/WebKit/Source/core/events/MouseEvent.cpp [modify] https://crrev.com/4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40/third_party/WebKit/Source/core/input/EventHandler.cpp
,
Jul 1 2016
M52 Stable is launching very soon! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged ASAP. All changes MUST be merged into the release branch by 5pm on July 8th (in case if you missed today's 5:00 PM PST deadline) to make into the desktop Stable final build cut. Thank you!
,
Jul 4 2016
Verified the issue on Windows 7, MAC (10.11.5), Ubuntu Trusty (14.04) & CrOS [8545.0.0_Dev Blaze] for Google Chrome Canary/Dev Version - 54.0.2787.0 Screen-recording is attached. TE_Verified labels are added.
,
Jul 4 2016
,
Jul 4 2016
Your change meets the bar and is auto-approved for M52 (branch: 2743)
,
Jul 4 2016
Your change meets the bar and is auto-approved for M53 (branch: 2785)
,
Jul 4 2016
Your change meets the bar and is auto-approved for M52 (branch: 2743)
,
Jul 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f0dfcc7e4b98d8c891bb3a8bf03900403aab0637 commit f0dfcc7e4b98d8c891bb3a8bf03900403aab0637 Author: Dave Tapuska <dtapuska@chromium.org> Date: Mon Jul 04 14:25:31 2016 Revert middle click related changes This is the CL to revert the related middle button click changes. We decided to revert those changes because the issues that were caused by suppressing click event for middle button was hard to fix without having that event. Particularly the ability to prevent opening a new tab which can be done by "preventDefault"ing the click event of middle button was removed as the result of the original change. For now we revert these changes and we pursue the line of adding a new event for non-primary button click to be able to fix these problem in a more clean way. Revert "Prevent sending click event for non primary button" This reverts commit 76fea00a18f75886ea649414393228180306e13d. Revert "Dispatch middle click manually by tracking mouse" This reverts commit 88eb1110baafcba070e750866a343e81b6bcc524. Revert "Fix history page middle click action" This reverts commit a154aed1a3813cf28c6f477579ed7974a2528570. BUG= 255 , 618593 , 617444 , 611019 , 617875 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2107093003 Cr-Commit-Position: refs/heads/master@{#403496} (cherry picked from commit 4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40) Review URL: https://codereview.chromium.org/2121003002 . Cr-Commit-Position: refs/branch-heads/2785@{#12} Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382} [modify] https://crrev.com/f0dfcc7e4b98d8c891bb3a8bf03900403aab0637/chrome/browser/resources/history/history.js [modify] https://crrev.com/f0dfcc7e4b98d8c891bb3a8bf03900403aab0637/chrome/browser/resources/ntp4/new_tab.html [delete] https://crrev.com/aaad16cd6a623d566e75f072db80264e4a5836ac/chrome/browser/resources/synthetic_middleclick.js [modify] https://crrev.com/f0dfcc7e4b98d8c891bb3a8bf03900403aab0637/chrome/browser/ui/browser_browsertest.cc [modify] https://crrev.com/f0dfcc7e4b98d8c891bb3a8bf03900403aab0637/testing/buildbot/filters/browser-side-navigation.linux.browser_tests.filter [modify] https://crrev.com/f0dfcc7e4b98d8c891bb3a8bf03900403aab0637/third_party/WebKit/LayoutTests/fast/events/mouse-click-events-expected.txt [modify] https://crrev.com/f0dfcc7e4b98d8c891bb3a8bf03900403aab0637/third_party/WebKit/LayoutTests/fast/events/script-tests/mouse-click-events.js [modify] https://crrev.com/f0dfcc7e4b98d8c891bb3a8bf03900403aab0637/third_party/WebKit/Source/core/events/MouseEvent.cpp [modify] https://crrev.com/f0dfcc7e4b98d8c891bb3a8bf03900403aab0637/third_party/WebKit/Source/core/input/EventHandler.cpp
,
Jul 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/eaaefc24ebb90ea1586f1ce7aee411da3da9d16e commit eaaefc24ebb90ea1586f1ce7aee411da3da9d16e Author: Dave Tapuska <dtapuska@chromium.org> Date: Mon Jul 04 14:43:01 2016 Revert middle click related changes This is the CL to revert the related middle button click changes. We decided to revert those changes because the issues that were caused by suppressing click event for middle button was hard to fix without having that event. Particularly the ability to prevent opening a new tab which can be done by "preventDefault"ing the click event of middle button was removed as the result of the original change. For now we revert these changes and we pursue the line of adding a new event for non-primary button click to be able to fix these problem in a more clean way. Revert "Prevent sending click event for non primary button" This reverts commit 76fea00a18f75886ea649414393228180306e13d. Revert "Dispatch middle click manually by tracking mouse" This reverts commit 88eb1110baafcba070e750866a343e81b6bcc524. Revert "Fix history page middle click action" This reverts commit a154aed1a3813cf28c6f477579ed7974a2528570. BUG= 255 , 618593 , 617444 , 611019 , 617875 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2107093003 Cr-Commit-Position: refs/heads/master@{#403496} (cherry picked from commit 4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40) Review URL: https://codereview.chromium.org/2124533002 . Cr-Commit-Position: refs/branch-heads/2743@{#580} Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939} [modify] https://crrev.com/eaaefc24ebb90ea1586f1ce7aee411da3da9d16e/chrome/browser/resources/history/history.js [modify] https://crrev.com/eaaefc24ebb90ea1586f1ce7aee411da3da9d16e/chrome/browser/resources/ntp4/new_tab.html [delete] https://crrev.com/5f87571138ba5e85759984e956efcb6691cf3770/chrome/browser/resources/synthetic_middleclick.js [modify] https://crrev.com/eaaefc24ebb90ea1586f1ce7aee411da3da9d16e/chrome/browser/ui/browser_browsertest.cc [modify] https://crrev.com/eaaefc24ebb90ea1586f1ce7aee411da3da9d16e/testing/buildbot/filters/browser-side-navigation.linux.browser_tests.filter [modify] https://crrev.com/eaaefc24ebb90ea1586f1ce7aee411da3da9d16e/third_party/WebKit/LayoutTests/fast/events/mouse-click-events-expected.txt [modify] https://crrev.com/eaaefc24ebb90ea1586f1ce7aee411da3da9d16e/third_party/WebKit/LayoutTests/fast/events/script-tests/mouse-click-events.js [modify] https://crrev.com/eaaefc24ebb90ea1586f1ce7aee411da3da9d16e/third_party/WebKit/Source/core/events/MouseEvent.cpp [modify] https://crrev.com/eaaefc24ebb90ea1586f1ce7aee411da3da9d16e/third_party/WebKit/Source/core/input/EventHandler.cpp
,
Jul 5 2016
rnimmagadda@ if you are testing this please do the testing on both M52 and M53.
,
Jul 13 2016
Rechecked this on chrome version beta M52 - 52.0.2743.75 and dev M53 - 53.0.2785.14 on Windows 7, MAC 10.11.5, Ubuntu 14.04. Fix is working as intended. Using middle click closed the opened file. Adding TE-Verified labels. Thanks.!
,
Aug 2 2016
,
Sep 8 2016
The "click" event is no longer dispatched for non-primary buttons as per UI events spec. This will ensure the pages that don't check the buttons attribute on their click handlers don't show unexpected behaviors when middle clicking for example. Instead there will be new event "auxclick" event for non-primary buttons that will be useful for some apps that truly care about click action for non-primary buttons. For example in case someone wants to prevent opening a new tab when middle clicking a link or adobt any other actions on click behavior of any non-primary button. Here is the spec for the event and some examples: https://navidz.github.io/auxclick/
,
Oct 5 2016
|
|||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||
Comment 1 by phistuck@chromium.org
, May 11 2016