[NSWindow makeKeyAndOrderFront:] Unexpectedly Changes the First Responder |
||||
Issue description
This affects BrowserFocusTest.BrowsersRememberFocus and likely others.
* thread #1, name = 'CrBrowserMain', queue = 'com.apple.main-thread', stop reason = watchpoint 1
* frame #0: 0x000000011c20152f libviews.dylib`views::FocusManager::SetFocusedViewWithReason(this=0x0000000155d8ce90, view=0x0000000000000000, reason=kReasonDirectFocusChange) at focus_manager.cc:334
frame #1: 0x000000011c080dc2 libviews.dylib`views::FocusManager::SetFocusedView(this=0x0000000155d8ce90, view=0x0000000000000000) at focus_manager.h:175
frame #2: 0x000000011c20288a libviews.dylib`views::FocusManager::ClearFocus(this=0x0000000155d8ce90) at focus_manager.cc:355
frame #3: 0x000000011c202a9a libviews.dylib`views::FocusManager::StoreFocusedView(this=0x0000000155d8ce90, clear_native_focus=true) at focus_manager.cc:391
frame #4: 0x000000011c0d8494 libviews.dylib`::-[BridgedContentView resignFirstResponder](self=0x0000000155d95210, _cmd="resignFirstResponder") at bridged_content_view.mm:634
frame #5: 0x00007fff315250bf AppKit`-[NSWindow _realMakeFirstResponder:] + 258
frame #6: 0x00007fff315a3b04 AppKit`-[NSWindow _selectFirstKeyView] + 848
frame #7: 0x00007fff315a3788 AppKit`-[NSWindow _setUpFirstResponder] + 240
frame #8: 0x00007fff315a2609 AppKit`-[NSWindow _doWindowWillBeVisibleAsSheet:] + 174
frame #9: 0x00007fff31d47d62 AppKit`-[NSWindow _reallyDoOrderWindowAboveOrBelow:relativeTo:findKey:forCounter:force:isModal:] + 1582
frame #10: 0x00007fff315a0b69 AppKit`-[NSWindow _doOrderWindow:relativeTo:findKey:forCounter:force:isModal:] + 287
frame #11: 0x00007fff315a09cf AppKit`-[NSWindow orderWindow:relativeTo:] + 169
frame #12: 0x000000011c10201f libviews.dylib`::-[NativeWidgetMacNSWindow orderWindow:relativeTo:](self=0x000000015b28fa90, _cmd="orderWindow:relativeTo:", orderingMode=NSWindowAbove, otherWindowNumber=0) at native_widget_mac_nswindow.mm:166
frame #13: 0x000000010941b54f interactive_ui_tests`::-[BrowserNativeWidgetWindow orderWindow:relativeTo:](self=0x000000015b28fa90, _cmd="orderWindow:relativeTo:", place=NSWindowAbove, otherWin=0) at browser_native_widget_window_mac.mm:77
frame #14: 0x00007fff316445c1 AppKit`-[NSWindow makeKeyAndOrderFront:] + 110
frame #15: 0x0000000100314caf interactive_ui_tests`ui_test_utils::ShowAndFocusNativeWindow(window=0x000000015b28fa90) at interactive_test_utils_mac.mm:167
frame #16: 0x000000010021cf4f interactive_ui_tests`(anonymous namespace)::BrowserFocusTest_BrowsersRememberFocus_Test::RunTestOnMainThread(this=0x000000015b62ff50) at browser_focus_uitest.cc:259
frame #17: 0x0000000104d3dcba interactive_ui_tests`content::BrowserTestBase::ProxyRunTestOnMainThreadLoop(this=0x000000015b62ff50) at browser_test_base.cc:379
,
Mar 19 2018
robliao: Is this polychrome or shipping Cocoa?
,
Mar 19 2018
+sdy@ could this be related to https://chromium-review.googlesource.com/c/chromium/src/+/935424
,
Mar 19 2018
lgrey: This is polychrome + mac_views_browser, and is blocking getting interactive_ui_tests passing.
,
Mar 19 2018
lgrey@: I don't think that change is involved, but if the issue bisects to it then I'll take a look.
,
Mar 20 2018
,
Mar 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/caf6dca0b9885dc1e63e73d48fe3e4807478ca8a commit caf6dca0b9885dc1e63e73d48fe3e4807478ca8a Author: Leonard Grey <lgrey@chromium.org> Date: Wed Mar 21 15:54:25 2018 Don't claim BridgedContentView can't be first responder when it's first responder BridgedContentView can and should be first responder when a browser chrome element is focused, but was changed to always respond |NO| to |acceptsFirstResponder| in https://chromium-review.googlesource.com/c/chromium/src/+/670479 This doesn't prevent it from being made first responder explicitly, but is meant to prevent mouse-clicks on non-interactable chrome from stealing focus from web content* Unfortunately, when ordering a window in, if the window's first responder claims not to accept first responder, AppKit finds one which will. I don't think this is reproable in normal use, but it breaks some interactive_ui_tests. This change is a variation on a comment tapted@ made on the original CL. Now, we only claim to accept first responder if we are already first responder. * Though I can't actually repro this by changing the method to always return |YES|, so who knows. Bug: 822824 Change-Id: I7ed3334f5ddd8ef9334acb13f6b6ea71529b0196 Reviewed-on: https://chromium-review.googlesource.com/971945 Commit-Queue: Leonard Grey <lgrey@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Cr-Commit-Position: refs/heads/master@{#544717} [modify] https://crrev.com/caf6dca0b9885dc1e63e73d48fe3e4807478ca8a/ui/views/cocoa/bridged_content_view.mm
,
Apr 10 2018
|
||||
►
Sign in to add a comment |
||||
Comment 1 by ellyjo...@chromium.org
, Mar 19 2018Status: Assigned (was: Available)