Commit cd9df201 authored by Lukasz Anforowicz's avatar Lukasz Anforowicz Committed by Commit Bot

Main frame's context menu should exclude frame-specific commands.

If content::ContextMenuParams includes a non-empty |frame_url|, then the
context menu (the menu shown after right clicking in a frame) includes
frame-specific commands like "View frame source" and/or "Reload frame".
Before r749036 / 82.0.4084.0, context menu shown for the main frame
would exclude such frame-specific items.  This behavior has regressed
in r749036, because it failed to preserve the following logic from
//third_party/blink/renderer/core/page/context_menu_controller.cc:

  if (selected_frame != page_->MainFrame())
    data.frame_url = WebURL(UrlFromFrame(selected_frame));

and instead started to unconditionally populate the |frame_url| field in
RenderFrameHostImpl::OnContextMenu.  This CL fixes the regression.

Bug: 1085040
Change-Id: I149447654bee858037a4230828e2c4a7b3c7c0bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2212995Reviewed-by: default avatarIstiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: default avatarNasko Oskov <nasko@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774333}
parent 50cb14c1
......@@ -80,6 +80,8 @@
#include "net/test/embedded_test_server/http_request.h"
#include "net/test/embedded_test_server/http_response.h"
#include "services/service_manager/public/cpp/interface_provider.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
#include "third_party/blink/public/common/context_menu_data/media_type.h"
#include "third_party/blink/public/common/input/web_input_event.h"
......@@ -892,6 +894,85 @@ IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest, SuggestedFileName) {
ASSERT_EQ(kSuggestedFilename, base::UTF16ToUTF8(suggested_filename).c_str());
}
// Check which commands are present after opening the context menu for the main
// frame. This is a regression test for https://crbug.com/1085040.
IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest,
MenuContentsVerification_MainFrame) {
ASSERT_TRUE(embedded_test_server()->Start());
GURL url(embedded_test_server()->GetURL("/iframe.html"));
ui_test_utils::NavigateToURL(browser(), url);
// Open a context menu.
ContextMenuWaiter menu_observer;
blink::WebMouseEvent mouse_event(
blink::WebInputEvent::Type::kMouseDown,
blink::WebInputEvent::kNoModifiers,
blink::WebInputEvent::GetStaticTimeStampForTests());
mouse_event.button = blink::WebMouseEvent::Button::kRight;
mouse_event.SetPositionInWidget(2, 2); // This is over the main frame.
content::WebContents* tab =
browser()->tab_strip_model()->GetActiveWebContents();
tab->GetRenderViewHost()->GetWidget()->ForwardMouseEvent(mouse_event);
mouse_event.SetType(blink::WebInputEvent::Type::kMouseUp);
tab->GetRenderViewHost()->GetWidget()->ForwardMouseEvent(mouse_event);
// Wait for context menu to be visible.
menu_observer.WaitForMenuOpenAndClose();
// Verify that the expected context menu items are present. This also
// verifies that unwanted commands are missing (e.g. that
// IDC_CONTENT_CONTEXT_VIEWFRAMESOURCE is not present when clicking in the
// main frame - see https://crbug.com/1085040).
EXPECT_THAT(menu_observer.GetCapturedCommandIds(),
testing::ElementsAre(IDC_BACK, IDC_FORWARD, IDC_RELOAD,
-1, // separator
IDC_SAVE_PAGE, IDC_PRINT,
IDC_ROUTE_MEDIA, // aka "Cast"
IDC_CONTENT_CONTEXT_TRANSLATE,
-1, // separator
IDC_VIEW_SOURCE,
IDC_CONTENT_CONTEXT_INSPECTELEMENT));
}
// Check which commands are present after opening the context menu for a
// subframe.
IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest,
MenuContentsVerification_Subframe) {
ASSERT_TRUE(embedded_test_server()->Start());
GURL url(embedded_test_server()->GetURL("/iframe.html"));
ui_test_utils::NavigateToURL(browser(), url);
// Open a context menu.
ContextMenuWaiter menu_observer;
blink::WebMouseEvent mouse_event(
blink::WebInputEvent::Type::kMouseDown,
blink::WebInputEvent::kNoModifiers,
blink::WebInputEvent::GetStaticTimeStampForTests());
mouse_event.button = blink::WebMouseEvent::Button::kRight;
mouse_event.SetPositionInWidget(25, 25); // This is over the subframe.
content::WebContents* tab =
browser()->tab_strip_model()->GetActiveWebContents();
tab->GetRenderViewHost()->GetWidget()->ForwardMouseEvent(mouse_event);
mouse_event.SetType(blink::WebInputEvent::Type::kMouseUp);
tab->GetRenderViewHost()->GetWidget()->ForwardMouseEvent(mouse_event);
// Wait for context menu to be visible.
menu_observer.WaitForMenuOpenAndClose();
// Verify that only the expected context menu items are present.
EXPECT_THAT(
menu_observer.GetCapturedCommandIds(),
testing::ElementsAre(IDC_BACK, IDC_FORWARD, IDC_RELOAD,
-1, // separator
IDC_SAVE_PAGE, IDC_PRINT,
IDC_ROUTE_MEDIA, // aka "Cast"
IDC_CONTENT_CONTEXT_TRANSLATE,
-1, // separator
IDC_VIEW_SOURCE, IDC_CONTENT_CONTEXT_VIEWFRAMESOURCE,
IDC_CONTENT_CONTEXT_RELOADFRAME,
IDC_CONTENT_CONTEXT_INSPECTELEMENT));
}
// Check filename on clicking "Save Link As" is ignored for cross origin.
IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest, SuggestedFileNameCrossOrigin) {
// Register observer.
......
......@@ -64,8 +64,18 @@ content::ContextMenuParams& ContextMenuWaiter::params() {
return params_;
}
const std::vector<int>& ContextMenuWaiter::GetCapturedCommandIds() const {
return captured_command_ids_;
}
void ContextMenuWaiter::Cancel(RenderViewContextMenu* context_menu) {
params_ = context_menu->params();
const ui::SimpleMenuModel& menu_model = context_menu->menu_model();
captured_command_ids_.reserve(menu_model.GetItemCount());
for (int i = 0; i < menu_model.GetItemCount(); ++i)
captured_command_ids_.push_back(menu_model.GetCommandIdAt(i));
if (maybe_command_to_execute_)
context_menu->ExecuteCommand(*maybe_command_to_execute_, 0);
context_menu->Cancel();
......
......@@ -36,6 +36,7 @@ class ContextMenuWaiter {
~ContextMenuWaiter();
content::ContextMenuParams& params();
const std::vector<int>& GetCapturedCommandIds() const;
// Wait until the context menu is opened and closed.
void WaitForMenuOpenAndClose();
......@@ -46,6 +47,8 @@ class ContextMenuWaiter {
void Cancel(RenderViewContextMenu* context_menu);
content::ContextMenuParams params_;
std::vector<int> captured_command_ids_;
base::RunLoop run_loop_;
base::Optional<int> maybe_command_to_execute_;
......
......@@ -3152,8 +3152,9 @@ void RenderFrameHostImpl::OnContextMenu(
// Validate the URLs in |params|. If the renderer can't request the URLs
// directly, don't show them in the context menu.
ContextMenuParams validated_params(params);
validated_params.frame_url = GetLastCommittedURL();
validated_params.page_url = GetMainFrame()->GetLastCommittedURL();
if (GetParent()) // Only populate |frame_url| for subframes.
validated_params.frame_url = GetLastCommittedURL();
// We don't validate |unfiltered_link_url| so that this field can be used
// when users want to copy the original link URL.
......
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