Issue metadata
Sign in to add a comment
|
[User Feedback - Stable/Beta] Chrome Fails to Open Bookmarks in Bulk from Bookmarks Bar |
||||||||||||||||||||||
Issue descriptionChrome Version: 66.0.3359.139, 67.0.3396.30 OS: Mac OSX What steps will reproduce the problem? (1) Create a bookmark folder with 20+ bookmarks. (2) Hold "Command" and left-click on the folder from the bookmarks bar. (3) When the "Do you want to open all tabs" dialog appears, press Yes. (4) Observe that no tabs are opened. What is the expected result? The bookmarked items should load, each in a new tab. What happens instead? Nothing. Community Reports: - https://productforums.google.com/forum/#!topic/chrome/BQEF5mc2zPw - https://productforums.google.com/forum/#!topic/chrome/bOS0fvFZePY I can reproduce this as well on my machine (running M67 beta). Thanks!
,
May 7 2018
I can repro this as well on stable. Looks like it leaves the bookmark folder hover state on too.
,
May 7 2018
But oddly enough, selecting "Open All" from the context menu does work.
,
May 7 2018
Same scenario here. My bookmark folder has 15 bookmarks. Command/left click brings up "Do you want" dialog appears, but nothing happens when it's clicked. Right click "Open All" does work.
,
May 7 2018
I forgot to add, the same day that this started happening,tabs started requiring me to click the red x twice to close it. Same if I use command W instead, I have to press it twice in order to close the tab.
,
May 8 2018
That's really strange. I'll take a look this week.
,
May 8 2018
,
May 8 2018
I'm also having the same problem described above. If I right click on the folder > Open All.. > "Are you sure you want to open 20+ tabs?" And then click "yes", it opens the tabs just fine. However, if tap Enter on my keyboard instead of clicking "Yes" with the mouse, then everything gets very weird: the browser gets sluggish, I have to use cmd+w twice to close each tab as well as the main window when I want to close it (sometimes it even stays open and I have to force close the app).
,
May 9 2018
The browser I reproed this in above prevented shutdown today and was stuck at 100% CPU. Sample attached below. We think the root cause is the same as Issue 404385 .
,
May 9 2018
,
May 9 2018
I don't think this is the same as issue 404385 . This feature worked for me back in 2014, 2015, 2016, and most of 2017. Issue 404385 was reported in 2014. When I reported issue 794189 in December, which was duped here, it was within a few months of the regression.
,
May 9 2018
The regression is due to MacViews secondary UI being turned on, which switched us to SimpleMessageBox for this UI. The connection to issue 404385 is that we're not exiting the nested run loop.
,
May 9 2018
Can "MacViews secondary UI" be turned off until the regression can be fixed?
,
May 9 2018
If MacViews is disabled then we're back to your old bug captured in issue 794189 . You're out of luck either way. That said, don't know if you saw this, but right clicking on the folder and "Open all" apparently is supposed to work as a workaround per c#8.
,
May 9 2018
amineer@ I would think the original bug was with MacViews on. I just disabled #secondary-ui-md and #show-all-dialogs-with-views-toolkit on stable, and command+click works.
,
May 9 2018
That's been enabled since Dec 12?
,
May 9 2018
Not for 100%, but I think there was a lengthy partial rollout. ellyjones@ do you remember the timeline? If not, issue 794189 is fixed and this is something else that's very similar.
,
May 9 2018
The Dec. 12 was from M65 Canary and ellyjones@ says it was enabled by default there.
,
May 9 2018
Alex, I thought issue 794189 is a dupe of this, so disabling MacViews would actually fix that issue too? Issue 794189 is not fixed for me currently.
,
May 9 2018
That's my bad, I thought it was a relatively recently enabled experiment. Since I'm not helping here, un-CC'ing :D
,
May 9 2018
Can that be disabled until the bug is fixed?
,
May 9 2018
#22: Realistically no. There are a lot of other product considerations and moving pieces here. Unfortunately I think we're going to have to accept this at the moment and fix forward.
,
May 9 2018
Understood, thanks! Do you think a fix can be prioritized then, since the cause seems to be more isolated now?
,
May 10 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4a7db45d440a65171a620e53f6a9374e65fdd4d3 commit 4a7db45d440a65171a620e53f6a9374e65fdd4d3 Author: Leonard Grey <lgrey@chromium.org> Date: Thu May 10 21:17:04 2018 Mac: Don't trigger mouse down events on bookmark buttons if command key is down Currently, all clicks on a bookmark folder button trigger their action on mouse down, so that the folder menu can open on mouse down as expected. This also includes command click, which, rather than opening the folder menu, opens all the bookmarks in the folder as new tabs. This isn't desirable since: - The expected behavior is that button clicks trigger on mouse up. - Triggering this on mouse down interacts poorly with the nested run loop created by the confirmation dialog we display when the folder has many links. This change causes the action to no longer fire on mouse down if the user is command-clicking the folder button. Bug: 840387 Change-Id: Ic1364160cf26f2f3ea6e67d7dd0d6cf5079082eb Reviewed-on: https://chromium-review.googlesource.com/1053947 Reviewed-by: Robert Sesek <rsesek@chromium.org> Commit-Queue: Leonard Grey <lgrey@chromium.org> Cr-Commit-Position: refs/heads/master@{#557676} [modify] https://crrev.com/4a7db45d440a65171a620e53f6a9374e65fdd4d3/chrome/browser/ui/cocoa/draggable_button_mixin.mm [modify] https://crrev.com/4a7db45d440a65171a620e53f6a9374e65fdd4d3/chrome/browser/ui/cocoa/draggable_button_unittest.mm
,
May 11 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7575340a18a7acc8d5be518c80e70a0a5fce84e6 commit 7575340a18a7acc8d5be518c80e70a0a5fce84e6 Author: Timothy Loh <timloh@chromium.org> Date: Fri May 11 01:46:31 2018 Revert "Mac: Don't trigger mouse down events on bookmark buttons if command key is down" This reverts commit 4a7db45d440a65171a620e53f6a9374e65fdd4d3. Reason for revert: DraggableButtonTest.ActsOnMouseDownIgnoredForCommandClick fails on Mac10.12 bot, e.g. https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.12%20Tests/12756 (test times out) Original change's description: > Mac: Don't trigger mouse down events on bookmark buttons if command key is down > > Currently, all clicks on a bookmark folder button trigger their action on > mouse down, so that the folder menu can open on mouse down as expected. > > This also includes command click, which, rather than opening the folder menu, > opens all the bookmarks in the folder as new tabs. This isn't desirable since: > - The expected behavior is that button clicks trigger on mouse up. > - Triggering this on mouse down interacts poorly with the nested run loop > created by the confirmation dialog we display when the folder has many links. > > This change causes the action to no longer fire on mouse down if the user > is command-clicking the folder button. > > Bug: 840387 > Change-Id: Ic1364160cf26f2f3ea6e67d7dd0d6cf5079082eb > Reviewed-on: https://chromium-review.googlesource.com/1053947 > Reviewed-by: Robert Sesek <rsesek@chromium.org> > Commit-Queue: Leonard Grey <lgrey@chromium.org> > Cr-Commit-Position: refs/heads/master@{#557676} TBR=rsesek@chromium.org,lgrey@chromium.org Change-Id: Iafb3cc8af01be9ba50a5e944b44750c62ee1df84 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 840387 Reviewed-on: https://chromium-review.googlesource.com/1055027 Reviewed-by: Timothy Loh <timloh@chromium.org> Commit-Queue: Timothy Loh <timloh@chromium.org> Cr-Commit-Position: refs/heads/master@{#557760} [modify] https://crrev.com/7575340a18a7acc8d5be518c80e70a0a5fce84e6/chrome/browser/ui/cocoa/draggable_button_mixin.mm [modify] https://crrev.com/7575340a18a7acc8d5be518c80e70a0a5fce84e6/chrome/browser/ui/cocoa/draggable_button_unittest.mm
,
May 11 2018
The NextAction date has arrived: 2018-05-11
,
May 21 2018
Issue 845166 has been merged into this issue.
,
Jun 7 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b2f5b3a6c5c6cd5752c69257faa0c32e55d288f1 commit b2f5b3a6c5c6cd5752c69257faa0c32e55d288f1 Author: Trent Apted <tapted@chromium.org> Date: Thu Jun 07 23:42:10 2018 Cocoa: Mark the bookmark draggable button action as fired _before_ performing the action. Opening an entire bookmark folder with more than 20 bookmarks in it from the bookmarks bar using Cmd+Click on a Cocoa browser is currently broken. This happens in the bookmark draggable button's action. The action may run a nested loop, which may see the mouse "up" event and cause the action to be performed again, inside the nested loop. We also need to ensure the bookmark button doesn't enter its _own_ nested run loop, since the mouse up has already been eaten by the nested run loop inside the action. Failure to do so causes the action to happen twice, which results in the nested run loop of the action spawning another, nested, run loop within itself, which never exits. This puts the browser into a very weird state. TEST=On Mac (chrome://flags/#views-browser-windows DISABLED), have a folder on the bookmarks bar with >20 items and Cmd+Click it. Accept the dialog. Bookmarks should open. Bug: 849135, 840387 Change-Id: I7e1eaf57dff956647cbdcf2cb449bb971cada074 Reviewed-on: https://chromium-review.googlesource.com/1090513 Commit-Queue: Trent Apted <tapted@chromium.org> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org> Cr-Commit-Position: refs/heads/master@{#565463} [modify] https://crrev.com/b2f5b3a6c5c6cd5752c69257faa0c32e55d288f1/chrome/browser/ui/cocoa/bookmarks/bookmark_button.h [modify] https://crrev.com/b2f5b3a6c5c6cd5752c69257faa0c32e55d288f1/chrome/browser/ui/cocoa/bookmarks/bookmark_button.mm [modify] https://crrev.com/b2f5b3a6c5c6cd5752c69257faa0c32e55d288f1/chrome/browser/ui/cocoa/bookmarks/bookmark_button_unittest.mm
,
Jun 12 2018
checked in 69.0.3455.0 after ensuring chrome://flags/#views-browser-windows was Disabled. Should be fixed.
,
Jul 12
Found that even with the workaround, Chrome will crash when {Cmd}+clicking to open multiple tabs if the folder contains URLs that begin with `chrome://` e.g. `chrome://net-internals`
Tested on 67.0.3396.99
Does this patch fix that scenario as well?
,
Jul 12
yes
,
Sep 4
Just confirming that this is indeed fixed in today's 69.0.3497.81 release. Thank you. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by meh...@chromium.org
, May 7 2018