[PDF viewer] Always show "copy" in the context menu for selected text.
Currently, if some texts are selected and the PDF has restriction on copy action, the context menu won't display "copy" option for users due to the restriction. This might lead to users thinking PDF viewer is broken. To improve user experience, we want to always show copy option in the the context menu when texts are selected regardless of the copy restriction. And "copy" will be greyed out if copy restriction exists for that specific PDF to prevent copy action. This CL adds a new method HasCopyRestriction() in `WebPlugin` so that plugins can have copy permissions and selected text as two independent pieces of information. Then use the copy restriction to determine whether the "copy" option should be enabled inside the context menu. This CL also adds hello_world2_with_copy_restriction.pdf for testing PDFiumEngine::SelectText(). The test file was created by pdftk with an owner password "foo" and their copy permissions are exempted. Bug: 1195543 Change-Id: Ica26e60d53829a1d566661c87e96a5a7d81456b6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3710793 Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org> Commit-Queue: Nigi <nigi@chromium.org> Reviewed-by: Dave Tapuska <dtapuska@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Cr-Commit-Position: refs/heads/main@{#1018381}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
56555f9969
commit
ba50f4eae6
chrome/browser/renderer_context_menu
pdf
third_party/blink
@ -99,6 +99,7 @@
|
||||
#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/context_menu_data.h"
|
||||
#include "third_party/blink/public/common/context_menu_data/edit_flags.h"
|
||||
#include "third_party/blink/public/common/input/web_input_event.h"
|
||||
#include "third_party/blink/public/common/switches.h"
|
||||
#include "third_party/blink/public/mojom/context_menu/context_menu.mojom.h"
|
||||
@ -368,11 +369,21 @@ class PdfPluginContextMenuBrowserTest : public InProcessBrowserTest {
|
||||
}
|
||||
|
||||
protected:
|
||||
struct PdfInfo {
|
||||
// The selected text in the PDF when context menu is created.
|
||||
std::u16string selection_text;
|
||||
|
||||
// Whether the PDF has copy permission.
|
||||
bool can_copy;
|
||||
};
|
||||
|
||||
guest_view::TestGuestViewManager* test_guest_view_manager() const {
|
||||
return test_guest_view_manager_;
|
||||
}
|
||||
|
||||
std::unique_ptr<TestRenderViewContextMenu> SetupAndCreateMenu() {
|
||||
// Creates a context menu with the given `info`:
|
||||
std::unique_ptr<TestRenderViewContextMenu> SetupAndCreateMenuWithPdfInfo(
|
||||
const PdfInfo& info) {
|
||||
// Load a pdf page.
|
||||
GURL page_url = ui_test_utils::GetTestUrl(
|
||||
base::FilePath(FILE_PATH_LITERAL("pdf")),
|
||||
@ -399,12 +410,22 @@ class PdfPluginContextMenuBrowserTest : public InProcessBrowserTest {
|
||||
params.frame_url = extension_frame_->GetLastCommittedURL();
|
||||
params.media_type = blink::mojom::ContextMenuDataMediaType::kPlugin;
|
||||
params.media_flags |= blink::ContextMenuData::kMediaCanRotate;
|
||||
params.selection_text = info.selection_text;
|
||||
// Mimic how `edit_flag` is set in ContextMenuController::ShowContextMenu().
|
||||
if (info.can_copy)
|
||||
params.edit_flags |= blink::ContextMenuDataEditFlags::kCanCopy;
|
||||
|
||||
auto menu =
|
||||
std::make_unique<TestRenderViewContextMenu>(*extension_frame_, params);
|
||||
menu->Init();
|
||||
return menu;
|
||||
}
|
||||
|
||||
std::unique_ptr<TestRenderViewContextMenu> SetupAndCreateMenu() {
|
||||
return SetupAndCreateMenuWithPdfInfo(
|
||||
{/*selection_text=*/u"", /*can_copy=*/true});
|
||||
}
|
||||
|
||||
// Helper function for testing context menu of a pdf plugin inside a web page.
|
||||
void TestContextMenuOfPdfInsideWebPage(
|
||||
const base::FilePath::CharType* file_name) {
|
||||
@ -2142,6 +2163,34 @@ IN_PROC_BROWSER_TEST_F(PdfPluginContextMenuBrowserTest,
|
||||
ASSERT_FALSE(menu->IsCommandIdEnabled(IDC_CONTENT_CONTEXT_ROTATECCW));
|
||||
}
|
||||
|
||||
IN_PROC_BROWSER_TEST_F(PdfPluginContextMenuBrowserTest, CopyWithoutText) {
|
||||
std::unique_ptr<TestRenderViewContextMenu> menu = SetupAndCreateMenu();
|
||||
|
||||
// Test that 'Copy' doesn't exist.
|
||||
ASSERT_FALSE(menu->IsItemPresent(IDC_CONTENT_CONTEXT_COPY));
|
||||
}
|
||||
|
||||
IN_PROC_BROWSER_TEST_F(PdfPluginContextMenuBrowserTest, CopyText) {
|
||||
std::unique_ptr<TestRenderViewContextMenu> menu =
|
||||
SetupAndCreateMenuWithPdfInfo(
|
||||
{/*selection_text=*/u"text", /*can_copy=*/true});
|
||||
|
||||
// Test that 'Copy' exists and it is enabled.
|
||||
ASSERT_TRUE(menu->IsItemPresent(IDC_CONTENT_CONTEXT_COPY));
|
||||
ASSERT_TRUE(menu->IsCommandIdEnabled(IDC_CONTENT_CONTEXT_COPY));
|
||||
}
|
||||
|
||||
IN_PROC_BROWSER_TEST_F(PdfPluginContextMenuBrowserTest,
|
||||
CopyTextWithRestriction) {
|
||||
std::unique_ptr<TestRenderViewContextMenu> menu =
|
||||
SetupAndCreateMenuWithPdfInfo(
|
||||
{/*selection_text=*/u"text", /*can_copy=*/false});
|
||||
|
||||
// Test that 'Copy' exists and it is disabled.
|
||||
ASSERT_TRUE(menu->IsItemPresent(IDC_CONTENT_CONTEXT_COPY));
|
||||
ASSERT_FALSE(menu->IsCommandIdEnabled(IDC_CONTENT_CONTEXT_COPY));
|
||||
}
|
||||
|
||||
IN_PROC_BROWSER_TEST_F(PdfPluginContextMenuBrowserTest,
|
||||
IframedPdfHasNoPageItems) {
|
||||
TestContextMenuOfPdfInsideWebPage(FILE_PATH_LITERAL("test-iframe-pdf.html"));
|
||||
|
@ -639,6 +639,10 @@ bool PdfViewWebPlugin::CanRedo() const {
|
||||
return engine_->CanRedo();
|
||||
}
|
||||
|
||||
bool PdfViewWebPlugin::CanCopy() const {
|
||||
return engine_->HasPermission(DocumentPermission::kCopy);
|
||||
}
|
||||
|
||||
bool PdfViewWebPlugin::ExecuteEditCommand(const blink::WebString& name,
|
||||
const blink::WebString& value) {
|
||||
if (name == "SelectAll")
|
||||
|
@ -241,6 +241,7 @@ class PdfViewWebPlugin final : public PdfViewPluginBase,
|
||||
bool HasEditableText() const override;
|
||||
bool CanUndo() const override;
|
||||
bool CanRedo() const override;
|
||||
bool CanCopy() const override;
|
||||
bool ExecuteEditCommand(const blink::WebString& name,
|
||||
const blink::WebString& value) override;
|
||||
blink::WebURL LinkAtPosition(const gfx::Point& /*position*/) const override;
|
||||
|
@ -835,6 +835,7 @@ TEST_F(PdfViewWebPluginTest, GetContentRestrictionsWithNoPermissions) {
|
||||
EXPECT_EQ(kContentRestrictionCopy | kContentRestrictionCut |
|
||||
kContentRestrictionPaste | kContentRestrictionPrint,
|
||||
plugin_->GetContentRestrictions());
|
||||
EXPECT_FALSE(plugin_->CanCopy());
|
||||
}
|
||||
|
||||
TEST_F(PdfViewWebPluginTest, GetContentRestrictionsWithCopyAllowed) {
|
||||
@ -845,6 +846,7 @@ TEST_F(PdfViewWebPluginTest, GetContentRestrictionsWithCopyAllowed) {
|
||||
EXPECT_EQ(kContentRestrictionCut | kContentRestrictionPaste |
|
||||
kContentRestrictionPrint,
|
||||
plugin_->GetContentRestrictions());
|
||||
EXPECT_TRUE(plugin_->CanCopy());
|
||||
}
|
||||
|
||||
TEST_F(PdfViewWebPluginTest, GetContentRestrictionsWithPrintLowQualityAllowed) {
|
||||
|
@ -2168,9 +2168,6 @@ void PDFiumEngine::InvalidateAllPages() {
|
||||
}
|
||||
|
||||
std::string PDFiumEngine::GetSelectedText() {
|
||||
if (!HasPermission(DocumentPermission::kCopy))
|
||||
return std::string();
|
||||
|
||||
std::u16string result;
|
||||
for (size_t i = 0; i < selection_.size(); ++i) {
|
||||
std::u16string current_selection_text = selection_[i].GetText();
|
||||
|
@ -678,23 +678,43 @@ TEST_F(PDFiumEngineTest, HandleInputEventRawKeyDown) {
|
||||
EXPECT_TRUE(engine->HandleInputEvent(raw_key_down_event));
|
||||
}
|
||||
|
||||
namespace {
|
||||
#if BUILDFLAG(IS_WIN)
|
||||
constexpr char kSelectTextExpectedText[] =
|
||||
"Hello, world!\r\nGoodbye, world!\r\nHello, world!\r\nGoodbye, world!";
|
||||
#else
|
||||
constexpr char kSelectTextExpectedText[] =
|
||||
"Hello, world!\nGoodbye, world!\nHello, world!\nGoodbye, world!";
|
||||
#endif
|
||||
} // namespace
|
||||
|
||||
TEST_F(PDFiumEngineTest, SelectText) {
|
||||
NiceMock<MockTestClient> client;
|
||||
std::unique_ptr<PDFiumEngine> engine =
|
||||
InitializeEngine(&client, FILE_PATH_LITERAL("hello_world2.pdf"));
|
||||
ASSERT_TRUE(engine);
|
||||
|
||||
EXPECT_TRUE(engine->HasPermission(DocumentPermission::kCopy));
|
||||
|
||||
EXPECT_THAT(engine->GetSelectedText(), IsEmpty());
|
||||
|
||||
engine->SelectAll();
|
||||
#if BUILDFLAG(IS_WIN)
|
||||
constexpr char kExpectedText[] =
|
||||
"Hello, world!\r\nGoodbye, world!\r\nHello, world!\r\nGoodbye, world!";
|
||||
#else
|
||||
constexpr char kExpectedText[] =
|
||||
"Hello, world!\nGoodbye, world!\nHello, world!\nGoodbye, world!";
|
||||
#endif
|
||||
EXPECT_EQ(kExpectedText, engine->GetSelectedText());
|
||||
EXPECT_EQ(kSelectTextExpectedText, engine->GetSelectedText());
|
||||
}
|
||||
|
||||
TEST_F(PDFiumEngineTest, SelectTextWithCopyRestriction) {
|
||||
NiceMock<MockTestClient> client;
|
||||
std::unique_ptr<PDFiumEngine> engine = InitializeEngine(
|
||||
&client, FILE_PATH_LITERAL("hello_world2_with_copy_restriction.pdf"));
|
||||
ASSERT_TRUE(engine);
|
||||
|
||||
EXPECT_FALSE(engine->HasPermission(DocumentPermission::kCopy));
|
||||
|
||||
// The copy restriction should not affect the text selection hehavior.
|
||||
EXPECT_THAT(engine->GetSelectedText(), IsEmpty());
|
||||
|
||||
engine->SelectAll();
|
||||
EXPECT_EQ(kSelectTextExpectedText, engine->GetSelectedText());
|
||||
}
|
||||
|
||||
TEST_F(PDFiumEngineTest, SelectCroppedText) {
|
||||
|
BIN
pdf/test/data/hello_world2_with_copy_restriction.pdf
Normal file
BIN
pdf/test/data/hello_world2_with_copy_restriction.pdf
Normal file
Binary file not shown.
1
third_party/blink/public/web/web_plugin.h
vendored
1
third_party/blink/public/web/web_plugin.h
vendored
@ -167,6 +167,7 @@ class WebPlugin {
|
||||
|
||||
virtual bool CanUndo() const { return false; }
|
||||
virtual bool CanRedo() const { return false; }
|
||||
virtual bool CanCopy() const { return true; }
|
||||
|
||||
virtual bool ExecuteEditCommand(const WebString& name,
|
||||
const WebString& value) {
|
||||
|
@ -559,7 +559,8 @@ bool ContextMenuController::ShowContextMenu(LocalFrame* frame,
|
||||
WebString text = plugin->SelectionAsText();
|
||||
if (!text.IsEmpty()) {
|
||||
data.selected_text = text.Utf8();
|
||||
data.edit_flags |= ContextMenuDataEditFlags::kCanCopy;
|
||||
if (plugin->CanCopy())
|
||||
data.edit_flags |= ContextMenuDataEditFlags::kCanCopy;
|
||||
}
|
||||
bool plugin_can_edit_text = plugin->CanEditText();
|
||||
if (plugin_can_edit_text) {
|
||||
|
Reference in New Issue
Block a user