shrike@: I suspect that you got into a state where the parent browser window got focus and hence your key presses were actually going to the parent window. You'll get into a similar state with the normal Cocoa browser, if you were to click on the browser title bar with the cookie dialog active. I can't seem to repro this if in step 3, I don't click on the title bar of the original window. Also, if you explicitly click inside the cookie dialog after step 3, does it work fine?

Also, you weren't on MacViewsBrowser, right? It seems to be the case that clicks outside a modal window are giving focus to the parent window on MacViewsBrowser, which is probably incorrect. 

Hi karandeepb@,

This is not with MacViewsBrowser, but with the native Cocoa browser.

It does seem that clicking in the main window's title bar causes the same problem, and clicking somewhere back in the dialog fixes the problem. It makes sense to allow the user to click in the window title bar (e.g. to be able to drag the window around), but doing so should not take key focus away from the dialog.

I can also reproduce the problem without clicking:

(1) Bring up the cookies dialogs
(2) Press Cmd-` to switch to another window
(3) Press Shift-Cmd-` to switch back to the window displaying the cookies dialog

I think it's by-design that the parent browser window can accept key focus. E.g. you can close the tab that hosts the dialog, or even focus the Omnibox, type a new URL and navigate away; closing the dialog automatically.

For the dialog-always-gets-focus behaviour they'd need to be window-modal. E.g. a sheet, like the Edit Bookmark dialog.

Cmd-W also silently fails.

It (sort of) makes sense that you can type in the omnibox, in which case the dialog obviously needs to lose keyboard focus. I don't think there's a good reason for it to lose focus if I just click in the window title bar.

> Cmd-W also silently fails.

The same thing happens for the native Cocoa dialog. (also mac_views_browser, but not Aura platforms: but on aura platforms Ctrl+W will always close the tab, rather than NSBeep() the way we do on Mac).

I think the problem is that the WebContents is getting (some kind of) focus/firstResponder state.

When the dialog is created, WebContentsModalDialogManager::ShowDialogWithManager(..) calls BlockWebContentsInteraction(true), which invokes

RenderViewHost::SetIgnoreInputEvents(), and
WebContentsModalDialogManagerDelegate::SetWebContentsBlocked()

SetWebContentsBlocked calls TabStripModel::SetTabBlocked, which sets WebContentsData::blocked

... anyway. To fix this on Cocoa and MacViews, an option might be to check TabStripModel::IsTabBlocked when the browser window is activated. If it is, try to activate that tab's tab-modal dialog instead.

OR: Figure out what Aura is doing and simulate that

We also need to figure out if we want to keep NSBeep for Cmd+W or actually have Cmd+W do something like: close the tab or, close the dialog.

Note that all kinds of keyboard events are weirding out while that dialog is up. Command-T silently doesn't open a tab, Command-N silently doesn't open a window.

Note that they don't work on the Mac even with MacViews while the equivalent Control-T and Control-N on Windows work. So your issues with Command-W might be a symptom of a deeper event routing issue with command keys or menu equivalent keys on the Mac.

I think there's a straightforward frameworky fix for the command dispatch that will work on MacViews: all MacViews NSWindows are (or inherit from) NativeWidgetMacNSWindow which implements `CommandDispatchingWindow` from /ui/base/cocoa/command_dispatcher.h

It was added so that windows hosting renderers correctly forward events to the WebContents. But it would be straightforward to call -[CommandDispatchingWindow setCommandHandler:] for every dialog window we create as well.

Then install a BrowserWindowCommandHandler, and modify ChromeCommandDispatcherDelegate to walk up through [NSWindow parentWindow] until it finds one that reports a value for `chrome::FindBrowserWithWindow(window);`


The second part is ensuring that a dialog takes key status when activating the tab that it blocks.

Moving to pri=3 since I don't think there's a regression here, and there may be more moving parts with the proposed tab-modality changes in the pipeline.

sdy, can you take a peek at this? :)

SGTM!

I just re-tested this and it's still live on trunk.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e8bb3cdb4fa4571b82bb6f93765acbdf20e81e5e

commit e8bb3cdb4fa4571b82bb6f93765acbdf20e81e5e
Author: Sidney San Martín <sdy@chromium.org>
Date: Wed Mar 14 20:02:29 2018

Fix dialogs losing focus while using ⌘` to switch windows.

- If a browser window becomes key while a dialog is up and the web
  content is first responder, give focus to the dialog.

- If the web content *loses* first responder while the dialog is key,
  make the browser window key. This makes sure that clicking the omnibox
  or typing ⌘L, for instance, focuses the browser window.

The final version of this change is incomplete: The separation between
"focused" ("the web content is focused within its window") and "active" ("the
web content's window is the active window") is mostly an illusion because it
only exists on Mac and I kept running into cases where it breaks down (e.g.
setting "focused" implicitly sets "active" deep inside WebViewImpl). A bunch of
tests also check that certain functions and IPCs happen in a certain order, so
fiddling with anything just breaks a bunch of tests in ways that don't map to
real-world behavior. Now isn't the right time to mess with that, especially in
Cocoa code, so this leaves one case unfixed:

- If a dialog becomes key some other way, make sure that the web content
  is first responder in its parent window (so that focus returns to the
  web content on dismissing the dialog, and no other views have first
  responder appearance).

I filed  https://crbug.com/821931  to track it.

Bug:  658405 
Change-Id: I7ec11568d860e7371f552baa44a0d6b45db6dc22
Reviewed-on: https://chromium-review.googlesource.com/935424
Commit-Queue: Sidney San Martín <sdy@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543170}
[modify] https://crrev.com/e8bb3cdb4fa4571b82bb6f93765acbdf20e81e5e/chrome/browser/renderer_host/chrome_render_widget_host_view_mac_delegate.mm
[modify] https://crrev.com/e8bb3cdb4fa4571b82bb6f93765acbdf20e81e5e/chrome/browser/ui/cocoa/web_contents_modal_dialog_manager_views_mac.mm
[modify] https://crrev.com/e8bb3cdb4fa4571b82bb6f93765acbdf20e81e5e/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/e8bb3cdb4fa4571b82bb6f93765acbdf20e81e5e/content/public/browser/render_widget_host_view_mac_delegate.h

Tested the issue on mac 10.13.3 using chrome version #67.0.3371.0 as per the issue id: 821931 in comment #14.
Attached screen cast for reference.
Observed that the omnibox still appears active and get the focus. Same behaviour is observed in the chrome reported version #56.0.2897.0 also.

sdy@ - Could you please check the attached screen cast and please let us know the expected behaviour and please confirm the fix.

Thanks...!!

Deleted: 658405@M67.mp4 775 KB

	658405@M67.mp4 775 KB View Download

Hi krajshree@, it looks like that video is following the repro steps for  issue 821931 , which isn't fixed by this change.

 Issue 842477  has been merged into this issue.