Closing shortcut viewer doesn't activate underlying browser window |
|||
Issue descriptionlinux-chromeos, ToT * Open a browser window * Ctrl-Alt-/ to open shortcut viewer * Ctrl-W to close shortcut viewer Browser window doesn't activate. Ctrl-W doesn't work to close, Ctrl-L doesn't work to focus omnibox, etc. msw, sky suggested I pass this to you. :-P
,
Aug 6
,
Aug 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d00fd14396903eb767d86d9fbb97c7dbec8b58d5 commit d00fd14396903eb767d86d9fbb97c7dbec8b58d5 Author: Scott Violet <sky@chromium.org> Date: Tue Aug 07 03:13:24 2018 window-service: ignore requests to focus null Here's my comment as to why this is ignored: // The client is asking to remove focus from a window. This is typically a // side effect of the window becoming, or about to become, an unfocusable // Window (for example, the Window is hiding). Windows becoming unfocusable is // handled locally. Assume the request is for such a scenario and return // false to ignore the change. // // To process null requests conflicts with top-level activation changes. For // example, the typical sequence when a window is hidden is to first remove // focus, and then hide the window. FocusController keys off window hiding to // move activation. If this code where to set focus to null, FocusController // would not see the window hiding (because the active window was set to null) // and not automatically activate the next window. // // Another possibility for this code is to handle null as a signal to move // focus to the active window (if there is one). I'm going with the simpler // approach for now. BUG= 867654 TEST=covered by tests Change-Id: Iee8efd8895284acca4603ff22bde19adf16d8a7f Reviewed-on: https://chromium-review.googlesource.com/1164409 Reviewed-by: Michael Wasserman <msw@chromium.org> Commit-Queue: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#581113} [modify] https://crrev.com/d00fd14396903eb767d86d9fbb97c7dbec8b58d5/ash/main.cc [modify] https://crrev.com/d00fd14396903eb767d86d9fbb97c7dbec8b58d5/ash/shell/content/client/shell_main_delegate.cc [modify] https://crrev.com/d00fd14396903eb767d86d9fbb97c7dbec8b58d5/services/ui/ws2/focus_handler.cc [modify] https://crrev.com/d00fd14396903eb767d86d9fbb97c7dbec8b58d5/services/ui/ws2/focus_handler_unittest.cc
,
Aug 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f792e7d88b0265bf47a7707272ecdfb571cd9679 commit f792e7d88b0265bf47a7707272ecdfb571cd9679 Author: Rune Lillesveen <futhark@chromium.org> Date: Tue Aug 07 06:35:39 2018 Revert "window-service: ignore requests to focus null" This reverts commit d00fd14396903eb767d86d9fbb97c7dbec8b58d5. Reason for revert: Caused test flakiness https://crbug.com/871652 Original change's description: > window-service: ignore requests to focus null > > Here's my comment as to why this is ignored: > > // The client is asking to remove focus from a window. This is typically a > // side effect of the window becoming, or about to become, an unfocusable > // Window (for example, the Window is hiding). Windows becoming unfocusable is > // handled locally. Assume the request is for such a scenario and return > // false to ignore the change. > // > // To process null requests conflicts with top-level activation changes. For > // example, the typical sequence when a window is hidden is to first remove > // focus, and then hide the window. FocusController keys off window hiding to > // move activation. If this code where to set focus to null, FocusController > // would not see the window hiding (because the active window was set to null) > // and not automatically activate the next window. > // > // Another possibility for this code is to handle null as a signal to move > // focus to the active window (if there is one). I'm going with the simpler > // approach for now. > > BUG= 867654 > TEST=covered by tests > > Change-Id: Iee8efd8895284acca4603ff22bde19adf16d8a7f > Reviewed-on: https://chromium-review.googlesource.com/1164409 > Reviewed-by: Michael Wasserman <msw@chromium.org> > Commit-Queue: Scott Violet <sky@chromium.org> > Cr-Commit-Position: refs/heads/master@{#581113} TBR=sky@chromium.org,msw@chromium.org Change-Id: I7710ea88b1dcb53d5ad299ff1295ecec31ce3f87 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 867654 Reviewed-on: https://chromium-review.googlesource.com/1164782 Reviewed-by: Rune Lillesveen <futhark@chromium.org> Commit-Queue: Rune Lillesveen <futhark@chromium.org> Cr-Commit-Position: refs/heads/master@{#581148} [modify] https://crrev.com/f792e7d88b0265bf47a7707272ecdfb571cd9679/ash/main.cc [modify] https://crrev.com/f792e7d88b0265bf47a7707272ecdfb571cd9679/ash/shell/content/client/shell_main_delegate.cc [modify] https://crrev.com/f792e7d88b0265bf47a7707272ecdfb571cd9679/services/ui/ws2/focus_handler.cc [modify] https://crrev.com/f792e7d88b0265bf47a7707272ecdfb571cd9679/services/ui/ws2/focus_handler_unittest.cc
,
Aug 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a985282e71b5bb45048538d83e3e0591da5f123c commit a985282e71b5bb45048538d83e3e0591da5f123c Author: Scott Violet <sky@chromium.org> Date: Thu Aug 09 22:29:32 2018 extensions: only close the popup on activation changes if visible As the widget is shown only once the webcontents has loaded it is possible to close the popup before it's shown. Ignore activation changes until the popup is shown. BUG= 867654 TEST=covered by tests in mash Change-Id: I74956b137a119f2b745a52698cb9dee6543b85e4 Reviewed-on: https://chromium-review.googlesource.com/1169525 Reviewed-by: Michael Wasserman <msw@chromium.org> Commit-Queue: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#581924} [modify] https://crrev.com/a985282e71b5bb45048538d83e3e0591da5f123c/chrome/browser/ui/views/extensions/extension_popup.cc
,
Aug 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bef3de4ccd2c52deb936d39dd3a31d86394c5906 commit bef3de4ccd2c52deb936d39dd3a31d86394c5906 Author: Scott Violet <sky@chromium.org> Date: Thu Aug 09 23:11:26 2018 reland: window-service: ignore requests to focus null This differs from the first version in that SetFocus(null) now returns true. As you suggested in the last patch, returning false caused the client to attempt to revert, which lead to some oddities. So, it seems safer to return true. views_mus_unittests no longer fails with this version, and https://chromium-review.googlesource.com/c/chromium/src/+/1169525 fixs the issue with the browser_test NavigatingExtensionPopupBrowserTest.Webpage. Here's my comment as to why this is ignored: // The client is asking to remove focus from a window. This is typically a // side effect of the window becoming, or about to become, an unfocusable // Window (for example, the Window is hiding). Windows becoming unfocusable is // handled locally. Assume the request is for such a scenario and return // true. Returning false means the client will attempt to revert to the // previously focused window, which may cause unexpected activation changes. // // To process null requests conflicts with top-level activation changes. For // example, the typical sequence when a window is hidden is to first remove // focus, and then hide the window. FocusController keys off window hiding to // move activation. If this code were to set focus to null, FocusController // would not see the window hiding (because the active window was set to null) // and not automatically activate the next window. // // Another possibility for this code is to handle null as a signal to move // focus to the active window (if there is one). I'm going with the simpler // approach for now. BUG= 867654 , 871652 TEST=covered by tests Change-Id: Iff2ddb31c2b99b977bca8e56fb653b6ce4eb6813 Reviewed-on: https://chromium-review.googlesource.com/1165692 Commit-Queue: Scott Violet <sky@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Cr-Commit-Position: refs/heads/master@{#581951} [modify] https://crrev.com/bef3de4ccd2c52deb936d39dd3a31d86394c5906/ash/main.cc [modify] https://crrev.com/bef3de4ccd2c52deb936d39dd3a31d86394c5906/ash/shell/content/client/shell_main_delegate.cc [modify] https://crrev.com/bef3de4ccd2c52deb936d39dd3a31d86394c5906/services/ui/ws2/focus_handler.cc [modify] https://crrev.com/bef3de4ccd2c52deb936d39dd3a31d86394c5906/services/ui/ws2/focus_handler_unittest.cc [modify] https://crrev.com/bef3de4ccd2c52deb936d39dd3a31d86394c5906/ui/views/focus/focus_manager_unittest.cc
,
Aug 9
|
|||
►
Sign in to add a comment |
|||
Comment 1 by msw@chromium.org
, Jul 25