Skip to content

Commit 370c2a7

Browse files
author
[email protected]
committed
[WK2] Keyboard menu key should show context menu
https://bugs.webkit.org/show_bug.cgi?id=72099 Source/WebCore: Reviewed by Carlos Garcia Campos. Show the context menu when the GtkWidget::popup-menu signal is emitted. This signal is triggered by pressing a key (usually the Menu key or the Shift + F10 shortcut) or it could be emitted on WebKitWebView. Test: fast/events/context-activated-by-key-event.html Also could be tested by: ManualTests/keyboard-menukey-event.html ManualTests/win/contextmenu-key.html ManualTests/win/contextmenu-key2.html * page/EventHandler.cpp: (WebCore::EventHandler::sendContextMenuEventForKey): Correctly send the mouse event that used for showing the context menu. Previously the event was immediately dispatched as it is, but this was only the right way if some element was focused on the page. If there was no focused element or non-empty text range then the event lacked the right node, where it was supposed to be shown. The correct node is determined and added to the event in the sendContextMenuEvent() so we have to use this function to send the event. Also use absoluteBoundingBoxRect() instead of pixelSnappedAbsoluteClippedOverflowRect() when determining a coordinate where to show the context menu for the currently focus element. The latter is not returning a right box (it is bigger) which could lead to the situation that no menu will be displayed at all, because the HitTest won't contain the right element as the determined coordinates could be outside of the element. * page/EventHandler.h: Source/WebKit2: Reviewed by Carlos Garcia Campos. Show the context menu when the GtkWidget::popup-menu signal is emitted. This signal is triggered by pressing a key (usually the Menu key or the Shift + F10 shortcut) or it could be emitted on WebKitWebView. * UIProcess/API/gtk/WebKitWebView.cpp: (webkit_web_view_class_init): (webkit_web_view_class_init): Update the documentation for the context-menu signal * UIProcess/API/gtk/WebKitWebViewBase.cpp: (webkitWebViewBasePopupMenu): Connect to the popup-menu signal and save the event that was used to trigger the signal. If there is no such event create a new GdkEvent with GDK_NOTHING type. (webkitWebViewBasePopupMenu): (webkit_web_view_base_class_init): * UIProcess/WebPageProxy.cpp: (WebKit::WebPageProxy::handleContextMenuKeyEvent): * UIProcess/WebPageProxy.h: * WebProcess/WebPage/WebPage.cpp: (WebKit::WebPage::contextMenuForKeyEvent): * WebProcess/WebPage/WebPage.h: * WebProcess/WebPage/WebPage.messages.in: Tools: Show the context menu when the GtkWidget::popup-menu signal is emitted. This signal is triggered by pressing a key (usually the Menu key or the Shift + F10 shortcut) or it could be emitted on WebKitWebView. Reviewed by Carlos Garcia Campos. * TestWebKitAPI/Tests/WebKit2Gtk/TestContextMenu.cpp: (testContextMenuDefaultMenu): * TestWebKitAPI/gtk/WebKit2Gtk/WebViewTest.cpp: (WebViewTest::emitPopupMenuSignal): * TestWebKitAPI/gtk/WebKit2Gtk/WebViewTest.h: LayoutTests: Reviewed by Carlos Garcia Campos. Skip the fast/events/context-activated-by-key-event.html on Mac as it does not have a key to activate the context menu and on iOS as well. * platform/ios-simulator-wk2/TestExpectations: * platform/mac-wk2/TestExpectations: * platform/mac/TestExpectations: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@213278 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 9bcaceb commit 370c2a7

21 files changed

+315
-11
lines changed

‎LayoutTests/ChangeLog

+14
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,17 @@
1+
2017-03-02 Tomas Popela <[email protected]>
2+
3+
[WK2] Keyboard menu key should show context menu
4+
https://bugs.webkit.org/show_bug.cgi?id=72099
5+
6+
Reviewed by Carlos Garcia Campos.
7+
8+
Skip the fast/events/context-activated-by-key-event.html on Mac as it
9+
does not have a key to activate the context menu and on iOS as well.
10+
11+
* platform/ios-simulator-wk2/TestExpectations:
12+
* platform/mac-wk2/TestExpectations:
13+
* platform/mac/TestExpectations:
14+
115
2017-03-02 Javier Fernandez <[email protected]>
216

317
[GTK] Unreviewed test gardening

‎LayoutTests/fast/events/context-activated-by-key-event-expected.txt

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
This tests whether the context menu is displayed on the menu key press.
2+
example.com
3+
PASS WINDOW
4+
PASS CONTENTEDITABLE
5+
PASS ELEMENT
6+
PASS CONTENTEDITABLE SELECTION
7+

‎LayoutTests/fast/events/context-activated-by-key-event.html

+56
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
<html>
2+
<body>
3+
<div id="contenteditable" contenteditable>This tests whether the context menu is displayed on the menu key press.</div>
4+
<a id="link" href="example.com">example.com</a>
5+
<p id='result'></p>
6+
</body>
7+
</html>
8+
9+
10+
<script>
11+
function log(text) {
12+
document.getElementById('result').appendChild(document.createTextNode(text));
13+
document.getElementById('result').appendChild(document.createElement("br"));
14+
}
15+
16+
function onWindowContextMenu(event) {
17+
log('PASS WINDOW');
18+
event.stopPropagation();
19+
}
20+
function onContentEditableContextMenu(event) {
21+
if (window.getSelection().toString())
22+
log('PASS CONTENTEDITABLE SELECTION');
23+
else
24+
log('PASS CONTENTEDITABLE');
25+
event.stopPropagation();
26+
}
27+
function onFocusedElementContextMenu(event) {
28+
log('PASS ELEMENT');
29+
event.stopPropagation();
30+
}
31+
32+
window.addEventListener('contextmenu', onWindowContextMenu);
33+
document.getElementById('contenteditable').addEventListener('contextmenu', onContentEditableContextMenu);
34+
document.getElementById('link').addEventListener('contextmenu', onFocusedElementContextMenu);
35+
36+
if (window.testRunner) {
37+
eventSender.keyDown('menu');
38+
39+
var rect = document.getElementById('contenteditable').getBoundingClientRect();
40+
var x = rect.left + rect.width / 2;
41+
var y = rect.top + rect.height / 2;
42+
eventSender.mouseMoveTo(x, y);
43+
eventSender.mouseDown();
44+
eventSender.mouseUp();
45+
eventSender.keyDown('menu');
46+
47+
document.getElementById('link').focus();
48+
eventSender.keyDown('menu');
49+
50+
window.getSelection().selectAllChildren(document.getElementById('contenteditable'));
51+
eventSender.keyDown('menu');
52+
53+
testRunner.dumpAsText();
54+
}
55+
56+
</script>

‎LayoutTests/platform/ios-simulator-wk2/TestExpectations

+3
Original file line numberDiff line numberDiff line change
@@ -1916,3 +1916,6 @@ fast/css/deferred-parsing/hover-test.html [ Skip ]
19161916
webkit.org/b/166025 http/tests/fetch/fetching-same-resource-with-diffferent-options.html [ Pass Failure ]
19171917

19181918
imported/w3c/web-platform-tests/webrtc [ Skip ]
1919+
1920+
# Skipped because there is no key to show the context menu
1921+
fast/events/context-activated-by-key-event.html [ Skip ]

‎LayoutTests/platform/mac-wk2/TestExpectations

+4
Original file line numberDiff line numberDiff line change
@@ -638,4 +638,8 @@ webkit.org/b/168089 [ Release ] animations/trigger-container-scroll-empty.html [
638638

639639
webkit.org/b/168391 [ ElCapitan Debug ] storage/indexeddb/modern/idbcursor-continue-primary-key-1.html [ Pass Timeout ]
640640

641+
641642
webkit.org/b/168380 [ ElCapitan Debug ] imported/w3c/web-platform-tests/IndexedDB/idb-binary-key-roundtrip.htm [ Pass Failure ]
643+
644+
# Skipped because Mac doesn't have a key to show the context menu
645+
fast/events/context-activated-by-key-event.html [ Skip ]

‎LayoutTests/platform/mac/TestExpectations

+3
Original file line numberDiff line numberDiff line change
@@ -1550,3 +1550,6 @@ webkit.org/b/168503 editing/pasteboard/drag-drop-copy-content.html [ Failure ]
15501550
webkit.org/b/168936 imported/w3c/web-platform-tests/IndexedDB/idbdatabase-deleteObjectStore-exception-order.htm [ Pass Failure ]
15511551

15521552
webkit.org/b/168927 fast/dom/timer-throttling-hidden-page.html [ Pass Failure ]
1553+
1554+
# Skipped because Mac doesn't have a key to show the context menu
1555+
fast/events/context-activated-by-key-event.html [ Skip ]

‎Source/WebCore/ChangeLog

+39
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,42 @@
1+
2017-03-02 Tomas Popela <[email protected]>
2+
3+
[WK2] Keyboard menu key should show context menu
4+
https://bugs.webkit.org/show_bug.cgi?id=72099
5+
6+
Reviewed by Carlos Garcia Campos.
7+
8+
Show the context menu when the GtkWidget::popup-menu signal is
9+
emitted. This signal is triggered by pressing a key (usually
10+
the Menu key or the Shift + F10 shortcut) or it could be emitted on
11+
WebKitWebView.
12+
13+
Test: fast/events/context-activated-by-key-event.html
14+
15+
Also could be tested by:
16+
17+
ManualTests/keyboard-menukey-event.html
18+
ManualTests/win/contextmenu-key.html
19+
ManualTests/win/contextmenu-key2.html
20+
21+
* page/EventHandler.cpp:
22+
(WebCore::EventHandler::sendContextMenuEventForKey):
23+
Correctly send the mouse event that used for showing the context menu.
24+
Previously the event was immediately dispatched as it is, but this was
25+
only the right way if some element was focused on the page. If there
26+
was no focused element or non-empty text range then the event lacked
27+
the right node, where it was supposed to be shown. The correct node
28+
is determined and added to the event in the sendContextMenuEvent() so
29+
we have to use this function to send the event.
30+
31+
Also use absoluteBoundingBoxRect() instead of
32+
pixelSnappedAbsoluteClippedOverflowRect() when determining
33+
a coordinate where to show the context menu for the currently focus
34+
element. The latter is not returning a right box (it is bigger) which
35+
could lead to the situation that no menu will be displayed at all,
36+
because the HitTest won't contain the right element as the
37+
determined coordinates could be outside of the element.
38+
* page/EventHandler.h:
39+
140
2017-03-02 Carlos Garcia Campos <[email protected]>
241

342
[GTK] Crash in WebCore::CoordinatedGraphicsLayer::notifyFlushRequired

‎Source/WebCore/page/EventHandler.cpp

+4-3
Original file line numberDiff line numberDiff line change
@@ -2899,8 +2899,9 @@ bool EventHandler::sendContextMenuEventForKey()
28992899
RenderBoxModelObject* box = focusedElement->renderBoxModelObject();
29002900
if (!box)
29012901
return false;
2902-
IntRect clippedRect = box->pixelSnappedAbsoluteClippedOverflowRect();
2903-
location = IntPoint(clippedRect.x(), clippedRect.maxY() - 1);
2902+
2903+
IntRect boundingBoxRect = box->absoluteBoundingBoxRect(true);
2904+
location = IntPoint(boundingBoxRect.x(), boundingBoxRect.maxY() - 1);
29042905
} else {
29052906
location = IntPoint(
29062907
rightAligned ? view->contentsWidth() - kContextMenuMargin : kContextMenuMargin,
@@ -2932,7 +2933,7 @@ bool EventHandler::sendContextMenuEventForKey()
29322933

29332934
PlatformMouseEvent platformMouseEvent(position, globalPosition, RightButton, eventType, 1, false, false, false, false, WTF::currentTime(), ForceAtClick, NoTap);
29342935

2935-
return !dispatchMouseEvent(eventNames().contextmenuEvent, targetNode, true, 0, platformMouseEvent, false);
2936+
return sendContextMenuEvent(platformMouseEvent);
29362937
}
29372938
#endif // ENABLE(CONTEXT_MENUS)
29382939

‎Source/WebCore/page/EventHandler.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ class EventHandler {
234234

235235
#if ENABLE(CONTEXT_MENUS)
236236
WEBCORE_EXPORT bool sendContextMenuEvent(const PlatformMouseEvent&);
237-
bool sendContextMenuEventForKey();
237+
WEBCORE_EXPORT bool sendContextMenuEventForKey();
238238
#endif
239239

240240
void setMouseDownMayStartAutoscroll() { m_mouseDownMayStartAutoscroll = true; }

‎Source/WebKit2/ChangeLog

+30
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,33 @@
1+
2017-03-02 Tomas Popela <[email protected]>
2+
3+
[WK2] Keyboard menu key should show context menu
4+
https://bugs.webkit.org/show_bug.cgi?id=72099
5+
6+
Reviewed by Carlos Garcia Campos.
7+
8+
Show the context menu when the GtkWidget::popup-menu signal is
9+
emitted. This signal is triggered by pressing a key (usually
10+
the Menu key or the Shift + F10 shortcut) or it could be emitted on
11+
WebKitWebView.
12+
13+
* UIProcess/API/gtk/WebKitWebView.cpp:
14+
(webkit_web_view_class_init):
15+
(webkit_web_view_class_init): Update the documentation for the
16+
context-menu signal
17+
* UIProcess/API/gtk/WebKitWebViewBase.cpp:
18+
(webkitWebViewBasePopupMenu): Connect to the popup-menu signal and
19+
save the event that was used to trigger the signal. If there is no
20+
such event create a new GdkEvent with GDK_NOTHING type.
21+
(webkitWebViewBasePopupMenu):
22+
(webkit_web_view_base_class_init):
23+
* UIProcess/WebPageProxy.cpp:
24+
(WebKit::WebPageProxy::handleContextMenuKeyEvent):
25+
* UIProcess/WebPageProxy.h:
26+
* WebProcess/WebPage/WebPage.cpp:
27+
(WebKit::WebPage::contextMenuForKeyEvent):
28+
* WebProcess/WebPage/WebPage.h:
29+
* WebProcess/WebPage/WebPage.messages.in:
30+
131
2017-03-01 Wenson Hsieh <[email protected]>
232

333
Replace -[WKWebView adjustedDataInteractionItemProviders:] with a UI delegate method

‎Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp

+15
Original file line numberDiff line numberDiff line change
@@ -1616,6 +1616,21 @@ static void webkit_web_view_class_init(WebKitWebViewClass* webViewClass)
16161616
* </para></listitem>
16171617
* </itemizedlist>
16181618
*
1619+
* The @event is expected to be one of the following types:
1620+
* <itemizedlist>
1621+
* <listitem><para>
1622+
* a #GdkEventButton of type %GDK_BUTTON_PRESS when the context menu
1623+
* was triggered with mouse.
1624+
* <listitem><para>
1625+
* a #GdkEventKey of type %GDK_KEY_PRESS if the keyboard was used to show
1626+
* the menu.
1627+
* </para></listitem>
1628+
* <listitem><para>
1629+
* a generic #GdkEvent of type %GDK_NOTHING when the #GtkWidget:popup-menu
1630+
* signal was used to show the context menu.
1631+
* </para></listitem>
1632+
* </itemizedlist>
1633+
*
16191634
* If the signal handler returns %FALSE the context menu represented by @context_menu
16201635
* will be shown, if it return %TRUE the context menu will not be shown.
16211636
*

‎Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp

+15
Original file line numberDiff line numberDiff line change
@@ -807,6 +807,20 @@ static gboolean webkitWebViewBaseScrollEvent(GtkWidget* widget, GdkEventScroll*
807807
return TRUE;
808808
}
809809

810+
static gboolean webkitWebViewBasePopupMenu(GtkWidget* widget)
811+
{
812+
WebKitWebViewBase* webViewBase = WEBKIT_WEB_VIEW_BASE(widget);
813+
WebKitWebViewBasePrivate* priv = webViewBase->priv;
814+
815+
GdkEvent* currentEvent = gtk_get_current_event();
816+
if (!currentEvent)
817+
currentEvent = gdk_event_new(GDK_NOTHING);
818+
priv->contextMenuEvent.reset(currentEvent);
819+
priv->pageProxy->handleContextMenuKeyEvent();
820+
821+
return TRUE;
822+
}
823+
810824
static gboolean webkitWebViewBaseMotionNotifyEvent(GtkWidget* widget, GdkEventMotion* event)
811825
{
812826
WebKitWebViewBase* webViewBase = WEBKIT_WEB_VIEW_BASE(widget);
@@ -1121,6 +1135,7 @@ static void webkit_web_view_base_class_init(WebKitWebViewBaseClass* webkitWebVie
11211135
widgetClass->button_press_event = webkitWebViewBaseButtonPressEvent;
11221136
widgetClass->button_release_event = webkitWebViewBaseButtonReleaseEvent;
11231137
widgetClass->scroll_event = webkitWebViewBaseScrollEvent;
1138+
widgetClass->popup_menu = webkitWebViewBasePopupMenu;
11241139
widgetClass->motion_notify_event = webkitWebViewBaseMotionNotifyEvent;
11251140
widgetClass->enter_notify_event = webkitWebViewBaseCrossingNotifyEvent;
11261141
widgetClass->leave_notify_event = webkitWebViewBaseCrossingNotifyEvent;

‎Source/WebKit2/UIProcess/WebPageProxy.cpp

+5
Original file line numberDiff line numberDiff line change
@@ -4654,6 +4654,11 @@ void WebPageProxy::contextMenuItemSelected(const WebContextMenuItemData& item)
46544654

46554655
m_process->send(Messages::WebPage::DidSelectItemFromActiveContextMenu(item), m_pageID);
46564656
}
4657+
4658+
void WebPageProxy::handleContextMenuKeyEvent()
4659+
{
4660+
m_process->send(Messages::WebPage::ContextMenuForKeyEvent(), m_pageID);
4661+
}
46574662
#endif // ENABLE(CONTEXT_MENUS)
46584663

46594664
#if PLATFORM(IOS)

‎Source/WebKit2/UIProcess/WebPageProxy.h

+1
Original file line numberDiff line numberDiff line change
@@ -897,6 +897,7 @@ class WebPageProxy : public API::ObjectImpl<API::Object::Type::Page>
897897
#if ENABLE(CONTEXT_MENUS)
898898
// Called by the WebContextMenuProxy.
899899
void contextMenuItemSelected(const WebContextMenuItemData&);
900+
void handleContextMenuKeyEvent();
900901
#endif
901902

902903
// Called by the WebOpenPanelResultListenerProxy.

‎Source/WebKit2/WebProcess/WebPage/WebPage.cpp

+10
Original file line numberDiff line numberDiff line change
@@ -2201,6 +2201,16 @@ static bool handleContextMenuEvent(const PlatformMouseEvent& platformMouseEvent,
22012201

22022202
return handled;
22032203
}
2204+
2205+
void WebPage::contextMenuForKeyEvent()
2206+
{
2207+
corePage()->contextMenuController().clearContextMenu();
2208+
2209+
Frame& frame = m_page->focusController().focusedOrMainFrame();
2210+
bool handled = frame.eventHandler().sendContextMenuEventForKey();
2211+
if (handled)
2212+
contextMenu()->show();
2213+
}
22042214
#endif
22052215

22062216
static bool handleMouseEvent(const WebMouseEvent& mouseEvent, WebPage* page, bool onlyUpdateScrollbars)

‎Source/WebKit2/WebProcess/WebPage/WebPage.h

+1
Original file line numberDiff line numberDiff line change
@@ -1060,6 +1060,7 @@ class WebPage : public API::ObjectImpl<API::Object::Type::BundlePage>, public IP
10601060
#endif
10611061
#if ENABLE(CONTEXT_MENUS)
10621062
void contextMenuHidden() { m_isShowingContextMenu = false; }
1063+
void contextMenuForKeyEvent();
10631064
#endif
10641065

10651066
static bool scroll(WebCore::Page*, WebCore::ScrollDirection, WebCore::ScrollGranularity);

‎Source/WebKit2/WebProcess/WebPage/WebPage.messages.in

+1
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ messages -> WebPage LegacyReceiver {
121121

122122
#if ENABLE(CONTEXT_MENUS)
123123
ContextMenuHidden()
124+
ContextMenuForKeyEvent()
124125
#endif
125126

126127
ScrollBy(uint32_t scrollDirection, uint32_t scrollGranularity)

‎Tools/ChangeLog

+18
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,21 @@
1+
2017-03-02 Tomas Popela <[email protected]>
2+
3+
[WK2] Keyboard menu key should show context menu
4+
https://bugs.webkit.org/show_bug.cgi?id=72099
5+
6+
Show the context menu when the GtkWidget::popup-menu signal is
7+
emitted. This signal is triggered by pressing a key (usually
8+
the Menu key or the Shift + F10 shortcut) or it could be emitted on
9+
WebKitWebView.
10+
11+
Reviewed by Carlos Garcia Campos.
12+
13+
* TestWebKitAPI/Tests/WebKit2Gtk/TestContextMenu.cpp:
14+
(testContextMenuDefaultMenu):
15+
* TestWebKitAPI/gtk/WebKit2Gtk/WebViewTest.cpp:
16+
(WebViewTest::emitPopupMenuSignal):
17+
* TestWebKitAPI/gtk/WebKit2Gtk/WebViewTest.h:
18+
119
2017-03-01 Wenson Hsieh <[email protected]>
220

321
Replace -[WKWebView adjustedDataInteractionItemProviders:] with a UI delegate method

0 commit comments

Comments
 (0)