Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(675)

Issue 7528007: [Mac] Delete more bookmark bar folder code. This removes things that were missed last time. (Closed)

Created:
9 years, 4 months ago by Robert Sesek
Modified:
9 years, 4 months ago
Reviewers:
mrossetti
CC:
chromium-reviews, Paweł Hajdan Jr.
Visibility:
Public.

Description

[Mac] Delete more bookmark bar folder code. This removes things that were missed last time. One of the issues is that a nested event loop in |-mouseDown:| was causing the thread to block after the menu closes, because the opening of the menu pumps the mouseUp event. BUG=91850 TEST=Open a bookmark from the folder and the spinner spins. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=95901

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 2

Messages

Total messages: 3 (0 generated)
Robert Sesek
9 years, 4 months ago (2011-08-08 21:12:48 UTC) #1
mrossetti
LGTM http://codereview.chromium.org/7528007/diff/2001/chrome/browser/ui/cocoa/bookmarks/bookmark_button.mm File chrome/browser/ui/cocoa/bookmarks/bookmark_button.mm (right): http://codereview.chromium.org/7528007/diff/2001/chrome/browser/ui/cocoa/bookmarks/bookmark_button.mm#newcode289 chrome/browser/ui/cocoa/bookmarks/bookmark_button.mm:289: - (BOOL)canBecomeKeyView { Does the default implementation already ...
9 years, 4 months ago (2011-08-08 21:42:24 UTC) #2
Robert Sesek
9 years, 4 months ago (2011-08-08 21:42:48 UTC) #3
http://codereview.chromium.org/7528007/diff/2001/chrome/browser/ui/cocoa/book...
File chrome/browser/ui/cocoa/bookmarks/bookmark_button.mm (right):

http://codereview.chromium.org/7528007/diff/2001/chrome/browser/ui/cocoa/book...
chrome/browser/ui/cocoa/bookmarks/bookmark_button.mm:289: -
(BOOL)canBecomeKeyView {
On 2011/08/08 21:42:24, mrossetti wrote:
> Does the default implementation already return NO? If so, this function could
> just be removed.

I *think* it does, but it's not documented as such.

Powered by Google App Engine
This is Rietveld 408576698