Commit 363ba85c authored by Andrew Xu's avatar Andrew Xu Committed by Chromium LUCI CQ

[Multipaste] Enable all menu items regardless of clipboard data access

Currently menu items may be disabled due to lack of access to clipboard
data. In fact, the code in the DLP side should decide whether pasting
clipboard data should work and abort the paste if the paste destination
has no data access. Hence, disabling menu items in the view side is
meaningless.

In this CL, the code of disabling menu items is removed.

Bug: 1151497
Change-Id: Iaf140ac9151a21a4ff17859d6702cafe3a5ebe08
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2561105Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Reviewed-by: default avatarDavid Black <dmblack@google.com>
Commit-Queue: Andrew Xu <andrewxu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833743}
parent 01e8a28e
......@@ -12,8 +12,6 @@
#include "base/metrics/histogram_macros.h"
#include "ui/accessibility/ax_enums.mojom.h"
#include "ui/base/clipboard/clipboard.h"
#include "ui/base/data_transfer_policy/data_transfer_endpoint.h"
#include "ui/base/data_transfer_policy/data_transfer_policy_controller.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/ui_base_types.h"
#include "ui/gfx/geometry/rect.h"
......@@ -27,17 +25,6 @@
namespace ash {
namespace {
bool IsDataReadAllowed(const ui::DataTransferEndpoint* source,
const ui::DataTransferEndpoint* destination) {
ui::DataTransferPolicyController* policy_controller =
ui::DataTransferPolicyController::Get();
if (!policy_controller)
return true;
return policy_controller->IsDataReadAllowed(source, destination);
}
} // namespace
// static
std::unique_ptr<ClipboardHistoryMenuModelAdapter>
ClipboardHistoryMenuModelAdapter::Create(
......@@ -74,14 +61,6 @@ void ClipboardHistoryMenuModelAdapter::Run(
/*notify_if_restricted=*/false);
for (const auto& item : items) {
model_->AddItem(command_id, base::string16());
// Enable or disable the command depending on whether its corresponding
// clipboard history item is allowed to read or not.
// This clipboard read isn't initiated by the user, that's why it shouldn't
// notify if the clipboard is restricted.
model_->SetEnabledAt(model_->GetIndexOfCommandId(command_id),
IsDataReadAllowed(item.data().source(), &data_dst));
item_snapshots_.emplace(command_id, item);
++command_id;
}
......@@ -274,34 +253,26 @@ void ClipboardHistoryMenuModelAdapter::AdvancePseudoFocusFromSelectedItem(
SelectMenuItemWithCommandId(next_selected_item_command);
}
base::Optional<int>
ClipboardHistoryMenuModelAdapter::CalculateSelectedCommandIdAfterDeletion(
int ClipboardHistoryMenuModelAdapter::CalculateSelectedCommandIdAfterDeletion(
int command_id) const {
// If the menu item view to be deleted is the last one, Cancel()
// should be called so this function should not be hit.
DCHECK_GT(item_snapshots_.size(), 1u);
auto start_item = item_snapshots_.find(command_id);
DCHECK(start_item != item_snapshots_.cend());
// Search in the forward direction.
auto check_function = [this](const auto& key_value) -> bool {
return model_->IsEnabledAt(model_->GetIndexOfCommandId(key_value.first));
};
auto selectable_iter_forward = std::find_if(
std::next(start_item, 1), item_snapshots_.cend(), check_function);
if (selectable_iter_forward != item_snapshots_.cend())
return selectable_iter_forward->first;
// If no selectable item is found, then search in the reverse direction.
auto selectable_iter_reverse =
std::find_if(std::make_reverse_iterator(start_item),
item_snapshots_.crend(), check_function);
if (selectable_iter_reverse != item_snapshots_.crend())
return selectable_iter_reverse->first;
// No other selectable item, then returns the invalid id.
return base::nullopt;
auto item_to_delete = item_snapshots_.find(command_id);
DCHECK(item_to_delete != item_snapshots_.cend());
// Use the menu item right after the one to be deleted if any. Otherwise,
// select the previous one.
auto next_item_iter = item_to_delete;
++next_item_iter;
if (next_item_iter != item_snapshots_.cend())
return next_item_iter->first;
auto previous_item_iter = item_to_delete;
--previous_item_iter;
return previous_item_iter->first;
}
void ClipboardHistoryMenuModelAdapter::RemoveItemView(int command_id) {
......
......@@ -105,11 +105,9 @@ class ASH_EXPORT ClipboardHistoryMenuModelAdapter : views::MenuModelAdapter {
// `reverse` is true).
void AdvancePseudoFocusFromSelectedItem(bool reverse);
// Returns the command id of the menu item to be selected if any after the
// menu item specified by `command_id` is deleted. If no menu item is
// selectable after deletion, an absent value is returned.
base::Optional<int> CalculateSelectedCommandIdAfterDeletion(
int command_id) const;
// Returns the command id of the menu item to be selected after the
// menu item specified by `command_id` is deleted.
int CalculateSelectedCommandIdAfterDeletion(int command_id) const;
// Removes the item view specified by `command_id` from the root menu.
void RemoveItemView(int command_id);
......
......@@ -19,7 +19,6 @@
#include "ui/compositor/layer_animation_observer.h"
#include "ui/compositor/scoped_layer_animation_settings.h"
#include "ui/gfx/image/image_skia.h"
#include "ui/gfx/image/image_skia_operations.h"
#include "ui/strings/grit/ui_strings.h"
#include "ui/views/controls/image_view.h"
#include "ui/views/layout/box_layout.h"
......@@ -51,13 +50,11 @@ class FadeImageView : public RoundedImageView,
public:
FadeImageView(const ClipboardHistoryItem* clipboard_history_item,
const ClipboardHistoryResourceManager* resource_manager,
float opacity,
base::RepeatingClosure update_callback)
: RoundedImageView(ClipboardHistoryViews::kImageRoundedCornerRadius,
RoundedImageView::Alignment::kCenter),
resource_manager_(resource_manager),
clipboard_history_item_(*clipboard_history_item),
opacity_(opacity),
update_callback_(update_callback) {
resource_manager_->AddObserver(this);
SetImageFromModel();
......@@ -119,12 +116,7 @@ class FadeImageView : public RoundedImageView,
*(resource_manager_->GetImageModel(clipboard_history_item_)
.GetImage()
.ToImageSkia());
if (opacity_ != 1.f) {
SetImage(
gfx::ImageSkiaOperations::CreateTransparentImage(image, opacity_));
} else {
SetImage(image);
}
// When fading in a new image, the ImageView's image has likely changed
// sizes.
......@@ -149,9 +141,6 @@ class FadeImageView : public RoundedImageView,
// The ClipboardHistoryItem represented by this class.
const ClipboardHistoryItem clipboard_history_item_;
// The opacity of the image content.
const float opacity_;
// Used to notify of image changes.
base::RepeatingClosure update_callback_;
};
......@@ -231,16 +220,11 @@ class ClipboardHistoryBitmapItemView::BitmapContentsView
// if menu items have their own layers, the part beyond the container's
// bounds is still visible when the context menu is in overflow.
const float image_opacity =
container_->IsItemEnabled()
? 1.f
: ClipboardHistoryViews::kDisabledImageAlpha;
const auto* clipboard_history_item = container_->clipboard_history_item();
switch (container_->data_format_) {
case ui::ClipboardInternalFormat::kHtml:
return std::make_unique<FadeImageView>(
clipboard_history_item, container_->resource_manager_,
image_opacity,
base::BindRepeating(&BitmapContentsView::UpdateImageViewSize,
weak_ptr_factory_.GetWeakPtr()));
case ui::ClipboardInternalFormat::kBitmap: {
......@@ -249,10 +233,6 @@ class ClipboardHistoryBitmapItemView::BitmapContentsView
RoundedImageView::Alignment::kCenter);
gfx::ImageSkia bitmap_image = gfx::ImageSkia::CreateFrom1xBitmap(
clipboard_history_item->data().bitmap());
if (image_opacity != 1.f) {
bitmap_image = gfx::ImageSkiaOperations::CreateTransparentImage(
bitmap_image, image_opacity);
}
image_view->SetImage(bitmap_image);
return image_view;
}
......
......@@ -234,10 +234,6 @@ void ClipboardHistoryItemView::RecordButtonPressedHistogram() const {
}
}
bool ClipboardHistoryItemView::IsItemEnabled() const {
return container_->GetEnabled();
}
gfx::Size ClipboardHistoryItemView::CalculatePreferredSize() const {
const int preferred_width =
views::MenuConfig::instance().touchable_menu_width;
......@@ -283,7 +279,7 @@ Action ClipboardHistoryItemView::CalculateActionForMainButtonClick() const {
}
bool ClipboardHistoryItemView::ShouldHighlight() const {
return pseudo_focus_ == PseudoFocus::kMainButton && IsItemEnabled();
return pseudo_focus_ == PseudoFocus::kMainButton;
}
bool ClipboardHistoryItemView::ShouldShowDeleteButton() const {
......
......@@ -109,10 +109,6 @@ class ClipboardHistoryItemView : public views::View {
// Returns the name of the accessible node.
virtual base::string16 GetAccessibleName() const = 0;
// Returns whether the item view is enabled. The item view is disabled when
// it is not allowed to read clipboard data.
bool IsItemEnabled() const;
const ClipboardHistoryItem* clipboard_history_item() const {
return clipboard_history_item_;
}
......
......@@ -32,12 +32,8 @@ void ClipboardHistoryLabel::OnThemeChanged() {
// TODO(andrewxu): remove this line after https://crbug.com/1143009 is fixed.
ash::ScopedLightModeAsDefault scoped_light_mode_as_default;
const auto color_type =
GetEnabled()
? ash::AshColorProvider::ContentLayerType::kTextColorPrimary
: ash::AshColorProvider::ContentLayerType::kTextColorSecondary;
SetEnabledColor(
ash::AshColorProvider::Get()->GetContentLayerColor(color_type));
SetEnabledColor(ash::AshColorProvider::Get()->GetContentLayerColor(
ash::AshColorProvider::ContentLayerType::kTextColorPrimary));
}
} // namespace ash
......@@ -33,7 +33,6 @@ class ClipboardHistoryTextItemView::TextContentsView
auto* label =
AddChildView(std::make_unique<ClipboardHistoryLabel>(container->text_));
layout->SetFlexForView(label, /*flex_weight=*/1);
label->SetEnabled(container->IsItemEnabled());
InstallDeleteButton();
}
......
......@@ -41,9 +41,6 @@ constexpr int kImageRoundedCornerRadius = 4;
// The thickness of the image border.
constexpr int kImageBorderThickness = 1;
// The opacity of the image shown in a disabled item view.
constexpr float kDisabledImageAlpha = 0.38f;
} // namespace ClipboardHistoryViews
} // namespace ash
......
......@@ -866,26 +866,9 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMockDLPBrowserTest, Basics) {
GetContextMenu()->GetMenuItemViewAtForTest(/*index=*/0);
GetEventGenerator()->MoveMouseTo(
inaccessible_menu_item_view->GetBoundsInScreen().CenterPoint());
// Verify that `inaccessible_menu_item_view` cannot be selected by mouse
// hovering. It does not respond to mouse click either.
EXPECT_FALSE(inaccessible_menu_item_view->IsSelected());
GetEventGenerator()->ClickLeftButton();
EXPECT_EQ("", base::UTF16ToUTF8(textfield_->GetText()));
// Move the selection through the arrow key. Then delete the item by the
// backspace key. After deletion, `inaccessible_menu_item_view` is left.
PressAndRelease(ui::KeyboardCode::VKEY_DOWN, ui::EF_NONE);
PressAndRelease(ui::KeyboardCode::VKEY_BACK, ui::EF_NONE);
EXPECT_TRUE(VerifyClipboardTextData({"B"}));
EXPECT_EQ(1, GetContextMenu()->GetMenuItemsCount());
// Move the selection through the arrow key again. Verify that
// `inaccessible_menu_item_view` cannot be selected. Pressing the backspace
// key does not delete the item.
PressAndRelease(ui::KeyboardCode::VKEY_DOWN, ui::EF_NONE);
PressAndRelease(ui::KeyboardCode::VKEY_BACK, ui::EF_NONE);
EXPECT_FALSE(inaccessible_menu_item_view->IsSelected());
EXPECT_TRUE(VerifyClipboardTextData({"B"}));
EXPECT_EQ(1, GetContextMenu()->GetMenuItemsCount());
// Verify that the text is not pasted and menu is closed after click.
EXPECT_EQ("", base::UTF16ToUTF8(textfield_->GetText()));
EXPECT_FALSE(GetClipboardHistoryController()->IsMenuShowing());
}
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