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

Issue 37253004: Mac: delay window raising until space transitions. (Closed)

Created:
7 years, 2 months ago by davidben
Modified:
7 years, 1 month ago
Reviewers:
jackhou1, Nico
CC:
chromium-reviews
Visibility:
Public.

Description

Mac: delay window raising until space transitions. When manually raising browser windows in response to a dock activation that triggered a space transition, isOnActiveSpace is wrong and we focus the wrong space's windows. Detect this by seeing if the key window is off-screen. If so, delay the raise until the space change notification. BUG=309656 TEST=see bug Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=231647

Patch Set 1 #

Patch Set 2 : Comment grammar (try jobs in previous patchset) #

Total comments: 1

Patch Set 3 : Clean up event handler #

Patch Set 4 : Expand on comment. #

Patch Set 5 : Missing comma #

Unified diffs Side-by-side diffs Delta from patch set Stats (+74 lines, -10 lines) Patch
M chrome/browser/app_controller_mac.h View 2 chunks +5 lines, -0 lines 0 comments Download
M chrome/browser/app_controller_mac.mm View 1 2 3 4 6 chunks +56 lines, -3 lines 0 comments Download
M ui/base/cocoa/focus_window_set.mm View 1 1 chunk +13 lines, -7 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
davidben
More spaces woes. https://codereview.chromium.org/37253004/diff/40001/chrome/browser/app_controller_mac.mm File chrome/browser/app_controller_mac.mm (right): https://codereview.chromium.org/37253004/diff/40001/chrome/browser/app_controller_mac.mm#newcode1126 chrome/browser/app_controller_mac.mm:1126: reopenTime_ = base::TimeTicks::Now(); Not calling FocusWindowSet ...
7 years, 2 months ago (2013-10-23 16:04:45 UTC) #1
Nico
lgtm Maybe a comment in app_controller_mac.mm should contain parts of the CL description and maybe ...
7 years, 1 month ago (2013-10-29 17:52:22 UTC) #2
davidben
Expanded on the comment and referenced the bug.
7 years, 1 month ago (2013-10-29 18:11:40 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/davidben@chromium.org/37253004/220001
7 years, 1 month ago (2013-10-29 18:41:00 UTC) #4
commit-bot: I haz the power
7 years, 1 month ago (2013-10-29 21:57:57 UTC) #5
Message was sent while issue was closed.
Change committed as 231647

Powered by Google App Engine
This is Rietveld 408576698