Bug 207250 - [Win][MiniBrowser] Accelerator keys don't work unless the main window is focused
Summary: [Win][MiniBrowser] Accelerator keys don't work unless the main window is focused
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-02-04 23:06 PST by Fujii Hironori
Modified: 2020-02-06 17:43 PST (History)
8 users (show)

See Also:


Attachments
WIP patch (2.92 KB, patch)
2020-02-04 23:07 PST, Fujii Hironori
no flags Details | Formatted Diff | Diff
WIP patch (6.57 KB, patch)
2020-02-05 03:25 PST, Fujii Hironori
no flags Details | Formatted Diff | Diff
Patch (11.59 KB, patch)
2020-02-05 22:13 PST, Fujii Hironori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2020-02-04 23:06:35 PST
[Win][MiniBrowser] Accelerator keys don't work unless the main window is focused

MiniBrowser has accelerator keys, Ctrl-W to close window, Ctrl-R to reload.

In Firefox and Chrome, Ctrl-R can be prevented, but Ctrl-W can't.
Reloading by Ctrl-R should be implemented in WebKitBrowserWindow::didNotHandleKeyEvent.
You can test it by using https://javascript.info/keyboard-events

I think TranslateAccelerator should be used only for the message for main window.
https://stackoverflow.com/a/20138696
Comment 1 Fujii Hironori 2020-02-04 23:07:24 PST
Created attachment 389774 [details]
WIP patch
Comment 2 Fujii Hironori 2020-02-04 23:40:13 PST
I realized two problems in this WIP patch.

1. Ctrl-W can't close the window while loading a page

Ctrl-W should be processed without dispatching to WebKit.

2. accelerator keys don't work if toolbar buttons or URL bar are focused.

toolbar buttons should give up WM_SETFOCUS.
https://stackoverflow.com/a/18540038
Comment 3 Fujii Hironori 2020-02-05 00:23:10 PST
3. By pressing Alt-Tab to switch active windows, the main window gets focused.

This is the only way to focus the main window at the moment, but this seems a bug.

For example, Windows File Explore restores the previous focus when re-activating.
Restoring the previous focus seems a application's task by using WM_ACTIVATE.
Comment 4 Fujii Hironori 2020-02-05 00:29:32 PST
4. Ctrl++ (zoom in) and Ctrl+- (zoom out) is effective only for numpad
Comment 5 Fujii Hironori 2020-02-05 02:29:06 PST
To solve the issue #1, it seems that I should add WM_COMMAND bubble up code in WebView.

>    case WM_COMMAND:
>        PostMessage(GetParent(hWnd), message, wParam, lParam);

https://stackoverflow.com/a/1963016
Comment 6 Fujii Hironori 2020-02-05 03:25:33 PST
Created attachment 389789 [details]
WIP patch

* Split the accelerator table into pre and post.
* WebKit2 WebView bubbles up WM_COMMAND message to the parent
* SetFocus the brower window when WM_ACTIVATE of the main window

issue#2 and issue#4 still remain
Ctrl+R doesn't work for WebKit1 because it doesn't have didNotHandleKeyEvent
Ctrl+W doesn't work for WebKit1 because I din'd add the code bubbling up WM_COMMAND. Shold I add it to WebKit1?
Comment 7 Fujii Hironori 2020-02-05 22:13:38 PST
Created attachment 389937 [details]
Patch
Comment 8 Fujii Hironori 2020-02-06 17:42:28 PST
Comment on attachment 389937 [details]
Patch

Clearing flags on attachment: 389937

Committed r255995: <https://trac.webkit.org/changeset/255995>
Comment 9 Fujii Hironori 2020-02-06 17:42:31 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2020-02-06 17:43:14 PST
<rdar://problem/59246127>