Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2409623002:
[Mac] Pass YES to activateIgnoringOtherApps: to actually activate Chrome. (Closed)
|
Created:
4 years, 2 months ago by Sidney San Martín Modified:
4 years, 2 months ago CC:
chromium-reviews, Robert Sesek Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[Mac] Pass YES to activateIgnoringOtherApps: to actually activate Chrome.
Passing NO doesn't activate our app in most cases; it seems to be mainly
useful when an app is launched (so there may be a long delay before it's
ready to become active) but the user might have switched to another app
in the mean time. In that case, the activation isn't the immediate
result of a user gesture, so it shouldn't steal focus.
This is a recent regression from crrev.com/2386343004.
BUG=654441, 653483, 650845
Committed: https://crrev.com/d694190ff0e1e1d594ccd644f807a1309ee61d5e
Cr-Commit-Position: refs/heads/master@{#424182}
Patch Set 1 #
Dependent Patchsets: Messages
Total messages: 18 (8 generated)
Description was changed from ========== [Mac] Pass YES to activateIgnoringOtherApps: to actually activate Chrome. Passing NO doesn't activate our app in most cases; it seems to be mainly useful when an app is launched (so there may be a long delay before it's ready to become active) but the user might have switched to another app in the mean time. In that case, the activation isn't the immediate result of a user gesture, so it shouldn't steal focus. This is a recent regression from crrev.com/2395083003. BUG= ========== to ========== [Mac] Pass YES to activateIgnoringOtherApps: to actually activate Chrome. Passing NO doesn't activate our app in most cases; it seems to be mainly useful when an app is launched (so there may be a long delay before it's ready to become active) but the user might have switched to another app in the mean time. In that case, the activation isn't the immediate result of a user gesture, so it shouldn't steal focus. This is a recent regression from crrev.com/2395083003. BUG=654441 ==========
sdy@chromium.org changed reviewers: + avi@chromium.org
rsesek@chromium.org changed reviewers: + rsesek@chromium.org
Add BUG=653483, 650845 as well LGTM
Description was changed from ========== [Mac] Pass YES to activateIgnoringOtherApps: to actually activate Chrome. Passing NO doesn't activate our app in most cases; it seems to be mainly useful when an app is launched (so there may be a long delay before it's ready to become active) but the user might have switched to another app in the mean time. In that case, the activation isn't the immediate result of a user gesture, so it shouldn't steal focus. This is a recent regression from crrev.com/2395083003. BUG=654441 ========== to ========== [Mac] Pass YES to activateIgnoringOtherApps: to actually activate Chrome. Passing NO doesn't activate our app in most cases; it seems to be mainly useful when an app is launched (so there may be a long delay before it's ready to become active) but the user might have switched to another app in the mean time. In that case, the activation isn't the immediate result of a user gesture, so it shouldn't steal focus. This is a recent regression from crrev.com/2395083003. BUG=654441,653483,650845 ==========
On 2016/10/10 16:14:48, Robert Sesek wrote: > Add BUG=653483, 650845 as well > > LGTM Done, thanks.
On 2016/10/10 16:13:10, Sidney San Martín wrote: Who's calling -[BWC activate] (the only caller of this)? If it's only extensions or other times where we need to force things to the front, ok, but the regression is from code that didn't call with NSApplicationActivateIgnoringOtherApps so it seems like different behavior.
On 2016/10/10 16:17:37, Avi wrote: > On 2016/10/10 16:13:10, Sidney San Martín wrote: > > Who's calling -[BWC activate] (the only caller of this)? If it's only extensions > or other times where we need to force things to the front, ok, but the > regression is from code that didn't call with > NSApplicationActivateIgnoringOtherApps so it seems like different behavior. That's correct, it is only things that actually want to bring it to the front. Regarding behavior, good point! I just actually bisected this, and it was crrev.com/2386343004 which lost the old behavior.
Description was changed from ========== [Mac] Pass YES to activateIgnoringOtherApps: to actually activate Chrome. Passing NO doesn't activate our app in most cases; it seems to be mainly useful when an app is launched (so there may be a long delay before it's ready to become active) but the user might have switched to another app in the mean time. In that case, the activation isn't the immediate result of a user gesture, so it shouldn't steal focus. This is a recent regression from crrev.com/2395083003. BUG=654441,653483,650845 ========== to ========== [Mac] Pass YES to activateIgnoringOtherApps: to actually activate Chrome. Passing NO doesn't activate our app in most cases; it seems to be mainly useful when an app is launched (so there may be a long delay before it's ready to become active) but the user might have switched to another app in the mean time. In that case, the activation isn't the immediate result of a user gesture, so it shouldn't steal focus. This is a recent regression from crrev.com/2386343004. BUG=654441,653483,650845 ==========
On 2016/10/10 16:39:52, Sidney San Martín wrote: > On 2016/10/10 16:17:37, Avi wrote: > > On 2016/10/10 16:13:10, Sidney San Martín wrote: > > > > Who's calling -[BWC activate] (the only caller of this)? If it's only > extensions > > or other times where we need to force things to the front, ok, but the > > regression is from code that didn't call with > > NSApplicationActivateIgnoringOtherApps so it seems like different behavior. > > That's correct, it is only things that actually want to bring it to the front. …I should say "as far as I can tell", based on this being the old behavior and never noticing any unwanted activations. It looks like it mainly gets called via NativeAppWindowCocoa::Activate().
lgtm
The CQ bit was checked by sdy@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [Mac] Pass YES to activateIgnoringOtherApps: to actually activate Chrome. Passing NO doesn't activate our app in most cases; it seems to be mainly useful when an app is launched (so there may be a long delay before it's ready to become active) but the user might have switched to another app in the mean time. In that case, the activation isn't the immediate result of a user gesture, so it shouldn't steal focus. This is a recent regression from crrev.com/2386343004. BUG=654441,653483,650845 ========== to ========== [Mac] Pass YES to activateIgnoringOtherApps: to actually activate Chrome. Passing NO doesn't activate our app in most cases; it seems to be mainly useful when an app is launched (so there may be a long delay before it's ready to become active) but the user might have switched to another app in the mean time. In that case, the activation isn't the immediate result of a user gesture, so it shouldn't steal focus. This is a recent regression from crrev.com/2386343004. BUG=654441,653483,650845 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== [Mac] Pass YES to activateIgnoringOtherApps: to actually activate Chrome. Passing NO doesn't activate our app in most cases; it seems to be mainly useful when an app is launched (so there may be a long delay before it's ready to become active) but the user might have switched to another app in the mean time. In that case, the activation isn't the immediate result of a user gesture, so it shouldn't steal focus. This is a recent regression from crrev.com/2386343004. BUG=654441,653483,650845 ========== to ========== [Mac] Pass YES to activateIgnoringOtherApps: to actually activate Chrome. Passing NO doesn't activate our app in most cases; it seems to be mainly useful when an app is launched (so there may be a long delay before it's ready to become active) but the user might have switched to another app in the mean time. In that case, the activation isn't the immediate result of a user gesture, so it shouldn't steal focus. This is a recent regression from crrev.com/2386343004. BUG=654441,653483,650845 Committed: https://crrev.com/d694190ff0e1e1d594ccd644f807a1309ee61d5e Cr-Commit-Position: refs/heads/master@{#424182} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/d694190ff0e1e1d594ccd644f807a1309ee61d5e Cr-Commit-Position: refs/heads/master@{#424182} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
