Skip to content

GitLab

T
tangled
Commit 99a9f55b authored by Vikash Sharma's avatar Vikash Sharma Committed by Commit Bot
Browse files
Options

Fix event hit testing for non-located context menu of large elements 

Keyboard shortcut(Shift + F10) triggers event with non-located context
menu. In codepath where hit testing is done for the receiving element
of event, calculation of center of focused element is not clipped to
visual viewport bounds. This calculation of center can be incorrect and
fall outside viewport area if element dimensions are much larger than
visual viewport. In this case root html element is returned as result
of hit testing, hence incorrect context menu shows up.
This CL replaces BoundsInViewport() with
VisibleBoundsInVisualViewport() to clip rect within viewport bounds.
This calculates center of element area in viewport and eventually hit
test for correct element to show up its context menu.

Bug: 1101653
Change-Id: I6a6ff4bf5ba2d70eb3221449cad609b77370e6fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2299663
Commit-Queue: Vikash Sharma <vikash@microsoft.com>
Reviewed-by: Ankit Kumar 🌪️ <ankk@microsoft.com>
Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Reviewed-by: default avatarSiye Liu <siliu@microsoft.com>
Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Reviewed-by: default avatarDaniel Libby <dlibby@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#792914}
parent e4a7f92a
Hide whitespace changes
Inline Side-by-side
Showing with 77 additions and 1 deletion
chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc
View file @ 99a9f55b
......@@ -1007,6 +1007,48 @@ IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest,
IDC_CONTENT_CONTEXT_INSPECTELEMENT}));
}
#if !defined(OS_MAC)
// Check whether correct non-located context menu shows up for image element
// with height more than visual viewport bounds.
IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest,
NonLocatedContextMenuOnLargeImageElement) {
ASSERT_TRUE(embedded_test_server()->Start());
GURL image_url(
"data:text/html,<html><img src=\"http://example.test/cat.jpg\" "
"width=\"200\" height=\"10000\" tabindex=\"-1\" /></html>");
ui_test_utils::NavigateToURL(browser(), image_url);
// Open and close a context menu.
ContextMenuWaiter menu_observer;
content::WebContents* tab =
browser()->tab_strip_model()->GetActiveWebContents();
// Focus on the image element with height more than visual viewport bounds
// and center of element falls outside viewport area.
content::SimulateMouseClickAt(tab, /*modifier=*/0,
blink::WebMouseEvent::Button::kLeft,
gfx::Point(15, 15));
// Simulate non-located context menu on image element with Shift + F10.
content::SimulateKeyPress(tab, ui::DomKey::F10, ui::DomCode::F10,
ui::VKEY_F10, /*control=*/false, /*shift=*/true,
/*alt=*/false, /*command=*/false);
menu_observer.WaitForMenuOpenAndClose();
// Verify that the expected context menu items are present.
//
// Note that the assertion below doesn't use exact matching via
// testing::ElementsAre, because some platforms may include unexpected extra
// elements (e.g. an extra separator and IDC=100 has been observed on some Mac
// bots).
EXPECT_THAT(menu_observer.GetCapturedCommandIds(),
testing::IsSupersetOf({IDC_CONTENT_CONTEXT_OPENIMAGENEWTAB,
IDC_CONTENT_CONTEXT_COPYIMAGE,
IDC_CONTENT_CONTEXT_COPYIMAGELOCATION,
IDC_CONTENT_CONTEXT_SAVEIMAGEAS}));
}
#endif
// Check filename on clicking "Save Link As" is ignored for cross origin.
IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest, SuggestedFileNameCrossOrigin) {
// Register observer.
......
third_party/blink/renderer/core/input/event_handler.cc
View file @ 99a9f55b
......@@ -2127,7 +2127,7 @@ WebInputEventResult EventHandler::ShowNonLocatedContextMenu(
view->ConvertToRootFrame(selection_rect.Center());
}
} else if (focused_element) {
IntRect clipped_rect = focused_element->BoundsInViewport();
IntRect clipped_rect = focused_element->VisibleBoundsInVisualViewport();
location_in_root_frame =
visual_viewport.ViewportToRootFrame(clipped_rect.Center());
} else {
......
third_party/blink/renderer/core/page/context_menu_controller_test.cc
View file @ 99a9f55b
......@@ -5,8 +5,10 @@
#include "third_party/blink/renderer/core/page/context_menu_controller.h"
#include "base/optional.h"
#include "build/build_config.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/common/context_menu_data/edit_flags.h"
#include "third_party/blink/public/common/input/web_keyboard_event.h"
#include "third_party/blink/public/common/input/web_menu_source_type.h"
#include "third_party/blink/public/platform/web_rect.h"
#include "third_party/blink/public/web/web_context_menu_data.h"
......@@ -601,6 +603,38 @@ TEST_F(ContextMenuControllerTest, ShowNonLocatedContextMenuEvent) {
EXPECT_EQ(context_menu_data.selected_text, "Sample Input Text");
}
#if !defined(OS_MAC)
// Mac has no way to open a context menu based on a keyboard event.
TEST_F(ContextMenuControllerTest,
ValidateNonLocatedContextMenuOnLargeImageElement) {
GetDocument()->documentElement()->setInnerHTML(
"<img src=\"http://example.test/cat.jpg\" id=\"sample_image\" "
"width=\"200\" height=\"10000\" tabindex=\"-1\" />");
Document* document = GetDocument();
Element* image_element = document->getElementById("sample_image");
// Set focus on the image element.
image_element->focus();
document->UpdateStyleAndLayout(DocumentUpdateReason::kTest);
// Simulate Shift + F10 key event.
WebKeyboardEvent key_event(WebInputEvent::Type::kRawKeyDown,
WebInputEvent::kShiftKey,
WebInputEvent::GetStaticTimeStampForTests());
key_event.windows_key_code = ui::VKEY_F10;
GetWebView()->MainFrameWidget()->HandleInputEvent(
WebCoalescedInputEvent(key_event, ui::LatencyInfo()));
key_event.SetType(WebInputEvent::Type::kKeyUp);
GetWebView()->MainFrameWidget()->HandleInputEvent(
WebCoalescedInputEvent(key_event, ui::LatencyInfo()));
WebContextMenuData context_menu_data =
GetWebFrameClient().GetContextMenuData();
EXPECT_EQ(context_menu_data.media_type, ContextMenuDataMediaType::kImage);
}
#endif
TEST_F(ContextMenuControllerTest, SelectionRectClipped) {
GetDocument()->documentElement()->setInnerHTML(
"<textarea id='text-area' cols=6 rows=2>Sample editable text</textarea>");
......
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