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

Issue 1359483002: Revert of BrowserWindowCocoa::Show should activate the window (Closed)

Created:
5 years, 3 months ago by Andre
Modified:
5 years, 3 months ago
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of BrowserWindowCocoa::Show should activate the window (patchset #1 id:1 of https://codereview.chromium.org/1166643003/ ) Reason for revert: Caused regression: http://crbug.com/500038, and http://crbug.com/496786 Original issue's description: > BrowserWindowCocoa::Show should activate the window > > When a new tab/window is opened on Mac, the window is currently brought > to the front relative to other Chrome windows, but not relative to other > apps. This often goes unnoticed, since Chrome is often already the > active app, but it's particularly noticeable when clicking on a push > notification opens a new tab/window, as this window will remain > underneath other apps if Chrome wasn't already in the foreground. > > This patch makes Show always Activate the window as well. > > BUG=470830 > TEST=manual, follow steps in https://crbug.com/470830 > > Committed: https://crrev.com/b13b6d7b2c7ad67c1b1937897fe3c26527eeb943 > Cr-Commit-Position: refs/heads/master@{#332615} R=avi@chromium.org,johnme@chromium.org BUG=470830 Committed: https://crrev.com/bfacce0691d0ffe7551e2b35a816c1503a9d489a Cr-Commit-Position: refs/heads/master@{#349884}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -2 lines) Patch
M chrome/browser/ui/cocoa/browser_window_cocoa.mm View 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 11 (3 generated)
Andre
Created Revert of BrowserWindowCocoa::Show should activate the window
5 years, 3 months ago (2015-09-18 21:53:40 UTC) #1
Andre
PTAL
5 years, 3 months ago (2015-09-20 00:44:04 UTC) #2
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1359483002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1359483002/1
5 years, 3 months ago (2015-09-20 00:44:19 UTC) #4
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 3 months ago (2015-09-20 01:08:44 UTC) #6
Avi (use Gerrit)
lgtm
5 years, 3 months ago (2015-09-20 16:03:30 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1359483002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1359483002/1
5 years, 3 months ago (2015-09-20 16:03:39 UTC) #9
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 3 months ago (2015-09-20 22:38:00 UTC) #10
commit-bot: I haz the power
5 years, 3 months ago (2015-09-20 22:38:52 UTC) #11
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/bfacce0691d0ffe7551e2b35a816c1503a9d489a
Cr-Commit-Position: refs/heads/master@{#349884}

Powered by Google App Engine
This is Rietveld 408576698