Commit d8bee629 authored by Aaron Leventhal's avatar Aaron Leventhal Committed by Commit Bot

Pass through correct target for keyboard-initiated context menu events

The object at the point in the middle of the focused target isn't
necessarily the same as the focused target. For example, the object
in the middle of a 2-line link is often an object behind it.

Bug: 1082489
Change-Id: If5aff8dc97a917a2465957dd509cabdd9b2c45cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2303307
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Reviewed-by: default avatarAaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792793}
parent d185121d
......@@ -2144,17 +2144,13 @@ WebInputEventResult EventHandler::ShowNonLocatedContextMenu(
IntRect(location_in_viewport, IntSize()), frame_->View())
.Location();
Node* target_node =
override_target_element ? override_target_element : doc->FocusedElement();
if (!target_node)
target_node = doc;
// Use the focused node as the target for hover and active.
HitTestRequest request(HitTestRequest::kActive |
HitTestRequest::kRetargetForInert);
HitTestLocation location(location_in_root_frame);
HitTestResult result(request, location);
result.SetInnerNode(target_node);
result.SetInnerNode(focused_element ? static_cast<Node*>(focused_element)
: doc);
doc->UpdateHoverActiveState(request.Active(), !request.Move(),
result.InnerElement());
......@@ -2176,7 +2172,7 @@ WebInputEventResult EventHandler::ShowNonLocatedContextMenu(
// coordinates instead of root frame coordinates.
mouse_event.SetFrameScale(1);
return SendContextMenuEvent(mouse_event, override_target_element);
return SendContextMenuEvent(mouse_event, focused_element);
}
void EventHandler::ScheduleHoverStateUpdate() {
......
......@@ -203,6 +203,11 @@ class CORE_EXPORT EventHandler final : public GarbageCollected<EventHandler> {
Node*& target_node);
void CacheTouchAdjustmentResult(uint32_t, FloatPoint);
// Dispatch a context menu event. If |override_target_element| is provided,
// the context menu event will use that, so that the browser-generated context
// menu will be filled with options relevant to it, rather than the element
// found via hit testing the event's screen point. This is used so that a
// context menu generated via the keyboard reliably uses the correct target.
WebInputEventResult SendContextMenuEvent(
const WebMouseEvent&,
Element* override_target_element = nullptr);
......
......@@ -255,8 +255,21 @@ bool ContextMenuController::ShowContextMenu(LocalFrame* frame,
To<LocalFrame>(page_->GetFocusController().FocusedOrMainFrame())
->GetEditor());
// Links, Images, Media tags, and Image/Media-Links take preference over
// all else.
if (mouse_event && source_type == kMenuSourceKeyboard) {
Node* target_node = mouse_event->target()->ToNode();
if (target_node && IsA<Element>(target_node)) {
// Get the url from an explicitly set target, e.g. the focused element
// when the context menu is evoked from the keyboard. Note: the innerNode
// could also be set. It is used to identify a relevant inner media
// element. In most cases, the innerNode will already be set to any
// relevant inner media element via the median x,y point from the focused
// element's bounding box. As the media element in most cases fills the
// entire area of a focused link or button, this generally suffices.
// Example: When Shift+F10 is used with <a><img></a>, any image-related
// context menu options, such as open image in new tab, must be presented.
result.SetURLElement(target_node->EnclosingLinkEventParentOrSelf());
}
}
data.link_url = result.AbsoluteLinkURL();
auto* html_element = DynamicTo<HTMLElement>(result.InnerNode());
......@@ -265,6 +278,8 @@ bool ContextMenuController::ShowContextMenu(LocalFrame* frame,
data.alt_text = html_element->AltText();
}
// Links, Images, Media tags, and Image/Media-Links take preference over
// all else.
if (IsA<HTMLCanvasElement>(result.InnerNode())) {
data.media_type = ContextMenuDataMediaType::kCanvas;
data.has_image_contents = true;
......
......@@ -20,7 +20,7 @@ function focusLinkAndAssertLinkIsTargetOfContextMenu() {
<div contenteditable oncontextmenu="catchContextMenu(event);">Some editable text.</div>
<span oncontextmenu="catchContextMenu(event);">Some text to select.</span>
<input oncontextmenu="catchContextMenu(event);">
<a href="www" oncontextmenu="catchContextMenu(event);">A link</a>
<a href="www" oncontextmenu="catchContextMenu(event);">A<br>link</a>
<script>
test(function() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment