New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 840387 link

Starred by 18 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-05-11
OS: Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment

[User Feedback - Stable/Beta] Chrome Fails to Open Bookmarks in Bulk from Bookmarks Bar

Project Member Reported by craigtumblison@chromium.org, May 7 2018

Issue description

Chrome 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!
 
Labels: -Type-Bug -Pri-3 Pri-1 Type-Bug-Regression

Comment 2 by lgrey@chromium.org, May 7 2018

Labels: Hotlist-CocoaBrowser
Status: Available (was: Untriaged)
I can repro this as well on stable. Looks like it leaves the bookmark folder hover state on too.

Comment 3 by lgrey@chromium.org, May 7 2018

But oddly enough, selecting "Open All" from the context menu does work.
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.
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.
Labels: -Pri-1 M-68 Target-68 Pri-2
NextAction: 2018-05-11
Owner: ellyjo...@chromium.org
Status: Assigned (was: Available)
That's really strange. I'll take a look this week.
Summary: [User Feedback - Stable/Beta] Chrome Fails to Open Bookmarks in Bulk from Bookmarks Bar (was: Chrome Fails to Open Bookmarks in Bulk from Bookmarks Bar)
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).

Comment 9 Deleted

Cc: ellyjo...@chromium.org
Owner: lgrey@chromium.org
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 .
hang_sample.txt
1.3 MB View Download
Cc: amin...@google.com cmasso@google.com
 Issue 794189  has been merged into this issue.
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.
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.
Can "MacViews secondary UI" be turned off until the regression can be fixed?
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.
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.

That's been enabled since Dec 12?
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.
The Dec. 12 was from M65 Canary and ellyjones@ says it was enabled by default there.
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.
Cc: -amin...@google.com
That's my bad, I thought it was a relatively recently enabled experiment.  Since I'm not helping here, un-CC'ing :D
Can that be disabled until the bug is fixed?
#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.
Understood, thanks! Do you think a fix can be prioritized then, since the cause seems to be more isolated now?
Project Member

Comment 25 by bugdroid1@chromium.org, 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

Project Member

Comment 26 by bugdroid1@chromium.org, 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

The NextAction date has arrived: 2018-05-11
 Issue 845166  has been merged into this issue.
Project Member

Comment 29 by bugdroid1@chromium.org, 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

Status: Fixed (was: Assigned)
checked in 69.0.3455.0 after ensuring chrome://flags/#views-browser-windows was Disabled. Should be fixed.

Comment 31 Deleted

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?
yes
Just confirming that this is indeed fixed in today's 69.0.3497.81 release. Thank you.

Sign in to add a comment