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

Issue 1235173002: Revert of Chrome user menu shouldn't close if a tab steals focus (Closed)

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

Description

Revert of Chrome user menu shouldn't close if a tab steals focus (patchset #6 id:120001 of https://codereview.chromium.org/1136693002/) Reason for revert: This CL revealed issues 507686, 506158, 504772, 503938, 503932. Issue 504772 and 503938 can not be solved immediately, so we revert this cl to allow time to solve them or find alternative solutions. Original issue's description: > Chrome user menu shouldn't close if a tab steals focus. > > This CL is dedicated to fix above issue and achieve below behaviors: > 1. User Menu does not close during sign-in flow: Things can't steal focus; the user menu stays open while you're typing / in the signin flow. > 2. Sign-in confirmation is modal: The user menu is not closeable until the user interacts with the signin confirmation window. Sync should be delayed until the user interacts with the dialogue. > 3. Blue information box: This shouldn't go away until the user explicitly interacts with it (not you, what's new, "x"). Doesn't affect modality/close-ability of the user menu. > > BUG=341196 > > Committed: https://crrev.com/131615b8cfd0e9b3045f3384e06ec3d8eb4f272a > Cr-Commit-Position: refs/heads/master@{#330789} TBR=rogerta@chromium.org,msw@chromium.org,groby@chromium.org,jcivelli@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=341196 Committed: https://crrev.com/15589a459c096ea61d0228b594e4d549e463e94d Cr-Commit-Position: refs/heads/master@{#338927}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -84 lines) Patch
M chrome/browser/ui/browser_window.h View 1 chunk +0 lines, -3 lines 0 comments Download
M chrome/browser/ui/cocoa/browser_window_cocoa.h View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/ui/cocoa/browser_window_cocoa.mm View 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/ui/cocoa/profiles/avatar_base_controller.h View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/ui/cocoa/profiles/avatar_base_controller.mm View 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm View 1 chunk +0 lines, -9 lines 0 comments Download
M chrome/browser/ui/views/frame/browser_view.h View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/ui/views/frame/browser_view.cc View 1 chunk +0 lines, -7 lines 0 comments Download
M chrome/browser/ui/views/profiles/new_avatar_button.cc View 1 chunk +2 lines, -8 lines 0 comments Download
M chrome/browser/ui/views/profiles/profile_chooser_view.h View 1 chunk +0 lines, -9 lines 0 comments Download
M chrome/browser/ui/views/profiles/profile_chooser_view.cc View 2 chunks +0 lines, -21 lines 0 comments Download
M chrome/browser/ui/webui/signin/inline_login_handler.cc View 2 chunks +0 lines, -15 lines 0 comments Download
M chrome/test/base/test_browser_window.h View 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 27 (11 generated)
gogerald1
Created Revert of Chrome user menu shouldn't close if a tab steals focus
5 years, 5 months ago (2015-07-14 14:15:50 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1235173002/1
5 years, 5 months ago (2015-07-14 14:16:31 UTC) #2
commit-bot: I haz the power
No L-G-T-M from a valid reviewer yet. Only full committers are accepted. Even if an ...
5 years, 5 months ago (2015-07-14 14:16:35 UTC) #4
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1235173002/1
5 years, 5 months ago (2015-07-14 16:01:01 UTC) #6
commit-bot: I haz the power
Dry run: No L-G-T-M from a valid reviewer yet. Only full committers are accepted. Even ...
5 years, 5 months ago (2015-07-14 16:01:05 UTC) #8
Roger Tawa OOO till Jul 10th
lgtm
5 years, 5 months ago (2015-07-14 16:31:38 UTC) #9
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1235173002/1
5 years, 5 months ago (2015-07-14 16:33:20 UTC) #12
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 5 months ago (2015-07-14 16:34:19 UTC) #14
gogerald1
Revert: @avi@chromium.org Could you help look the revert changes in ui/cocoa/* since groby is OOO.
5 years, 5 months ago (2015-07-14 17:20:40 UTC) #16
gogerald1
phajdan.jr@chromium.org: Please review revert changes in chrome/test/base/test_browser_window.h
5 years, 5 months ago (2015-07-14 17:21:46 UTC) #18
Avi (use Gerrit)
If this is just a revert, lgtm
5 years, 5 months ago (2015-07-14 17:22:59 UTC) #19
Paweł Hajdan Jr.
chrome/test LGTM
5 years, 5 months ago (2015-07-14 23:33:18 UTC) #20
msw
Thanks for explaining the reason for reverting better, lgtm.
5 years, 5 months ago (2015-07-15 21:28:34 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1235173002/1
5 years, 5 months ago (2015-07-15 22:17:04 UTC) #25
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 5 months ago (2015-07-15 22:21:11 UTC) #26
commit-bot: I haz the power
5 years, 5 months ago (2015-07-15 22:23:14 UTC) #27
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/15589a459c096ea61d0228b594e4d549e463e94d
Cr-Commit-Position: refs/heads/master@{#338927}

Powered by Google App Engine
This is Rietveld 408576698