diff options
Diffstat (limited to 'chromium/ui/views/controls')
87 files changed, 1547 insertions, 1755 deletions
diff --git a/chromium/ui/views/controls/animated_icon_view.cc b/chromium/ui/views/controls/animated_icon_view.cc deleted file mode 100644 index 68453c7fac2..00000000000 --- a/chromium/ui/views/controls/animated_icon_view.cc +++ /dev/null @@ -1,91 +0,0 @@ -// Copyright 2017 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "ui/views/controls/animated_icon_view.h" - -#include "ui/compositor/compositor.h" -#include "ui/gfx/canvas.h" -#include "ui/gfx/color_palette.h" -#include "ui/gfx/paint_vector_icon.h" -#include "ui/views/widget/widget.h" - -namespace views { - -AnimatedIconView::AnimatedIconView(const gfx::VectorIcon& icon) - : icon_(icon), - color_(gfx::kPlaceholderColor), - duration_(gfx::GetDurationOfAnimation(icon)) { - UpdateStaticImage(); -} - -AnimatedIconView::~AnimatedIconView() { - if (compositor_ && compositor_->HasAnimationObserver(this)) - compositor_->RemoveAnimationObserver(this); -} - -void AnimatedIconView::SetColor(SkColor color) { - if (color_ != color) { - color_ = color; - UpdateStaticImage(); - } -} - -void AnimatedIconView::Animate(State target) { - SetState(target); - if (!IsAnimating()) { - compositor_ = GetWidget()->GetCompositor(); - compositor_->AddAnimationObserver(this); - } - start_time_ = base::TimeTicks::Now(); -} - -void AnimatedIconView::SetState(State state) { - state_ = state; - UpdateStaticImage(); -} - -bool AnimatedIconView::IsAnimating() const { - return start_time_ != base::TimeTicks(); -} - -void AnimatedIconView::OnPaint(gfx::Canvas* canvas) { - if (!IsAnimating()) { - views::ImageView::OnPaint(canvas); - return; - } - - auto timestamp = base::TimeTicks::Now(); - base::TimeDelta elapsed = timestamp - start_time_; - if (state_ == END) - elapsed = start_time_ + duration_ - timestamp; - - canvas->Translate(GetImageBounds().OffsetFromOrigin()); - gfx::PaintVectorIcon(canvas, icon_, color_, elapsed); -} - -void AnimatedIconView::OnAnimationStep(base::TimeTicks timestamp) { - base::TimeDelta elapsed = timestamp - start_time_; - if (elapsed > duration_) { - compositor_->RemoveAnimationObserver(this); - compositor_ = nullptr; - start_time_ = base::TimeTicks(); - } - - SchedulePaint(); -} - -void AnimatedIconView::OnCompositingShuttingDown(ui::Compositor* compositor) { - DCHECK_EQ(compositor, compositor_); - compositor_->RemoveAnimationObserver(this); - compositor_ = nullptr; -} - -void AnimatedIconView::UpdateStaticImage() { - gfx::IconDescription description( - icon_, 0, color_, state_ == START ? base::TimeDelta() : duration_, - gfx::kNoneIcon); - SetImage(gfx::CreateVectorIcon(description)); -} - -} // namespace views diff --git a/chromium/ui/views/controls/animated_icon_view.h b/chromium/ui/views/controls/animated_icon_view.h deleted file mode 100644 index 2f0eddf5dde..00000000000 --- a/chromium/ui/views/controls/animated_icon_view.h +++ /dev/null @@ -1,71 +0,0 @@ -// Copyright 2017 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef UI_VIEWS_CONTROLS_ANIMATED_ICON_VIEW_H_ -#define UI_VIEWS_CONTROLS_ANIMATED_ICON_VIEW_H_ - -#include "base/macros.h" -#include "base/time/time.h" -#include "ui/compositor/compositor_animation_observer.h" -#include "ui/gfx/vector_icon_types.h" -#include "ui/views/controls/image_view.h" - -namespace views { - -// This class hosts a vector icon that defines transitions. It can be in the -// start steady state, the end steady state, or transitioning in between. -class VIEWS_EXPORT AnimatedIconView : public views::ImageView, - public ui::CompositorAnimationObserver { - public: - enum State { - START, - END, - }; - - explicit AnimatedIconView(const gfx::VectorIcon& icon); - ~AnimatedIconView() override; - - void SetColor(SkColor color); - - // Animates to the end or start state. - void Animate(State target); - - // Jumps to the end or start state. - void SetState(State state); - - bool IsAnimating() const; - - // views::ImageView - void OnPaint(gfx::Canvas* canvas) override; - - // ui::CompositorAnimationObserver - void OnAnimationStep(base::TimeTicks timestamp) override; - void OnCompositingShuttingDown(ui::Compositor* compositor) override; - - private: - void UpdateStaticImage(); - - const gfx::VectorIcon& icon_; - SkColor color_; - - // Tracks the last time Animate() was called. - base::TimeTicks start_time_; - - // The amount of time that must elapse until all transitions are done, i.e. - // the length of the animation. - const base::TimeDelta duration_; - - // The current state, or when transitioning the goal state. - State state_ = START; - - // The compositor that |this| is observing, if and when there is an active - // animation. Otherwise it is null. - ui::Compositor* compositor_ = nullptr; - - DISALLOW_COPY_AND_ASSIGN(AnimatedIconView); -}; - -} // namespace views - -#endif // UI_VIEWS_CONTROLS_ANIMATED_ICON_VIEW_H_ diff --git a/chromium/ui/views/controls/animated_image_view.cc b/chromium/ui/views/controls/animated_image_view.cc index 2d56a88b440..bb379bc3d7e 100644 --- a/chromium/ui/views/controls/animated_image_view.cc +++ b/chromium/ui/views/controls/animated_image_view.cc @@ -7,8 +7,8 @@ #include <utility> #include "base/logging.h" +#include "cc/paint/skottie_wrapper.h" #include "ui/gfx/canvas.h" -#include "ui/gfx/skottie_wrapper.h" #include "ui/views/widget/widget.h" namespace views { @@ -52,21 +52,17 @@ void AnimatedImageView::Play() { state_ = State::kPlaying; - // We cannot play the animation unless we have a valid compositor. - if (!compositor_) - return; - - // Ensure the class is added as an observer to receive clock ticks. - if (!compositor_->HasAnimationObserver(this)) - compositor_->AddAnimationObserver(this); + SetCompositorFromWidget(); animated_image_->Start(); } void AnimatedImageView::Stop() { + if (state_ == State::kStopped) + return; + DCHECK(animated_image_); - if (compositor_) - compositor_->RemoveAnimationObserver(this); + ClearCurrentCompositor(); animated_image_->Stop(); state_ = State::kStopped; @@ -99,27 +95,21 @@ const char* AnimatedImageView::GetClassName() const { } void AnimatedImageView::NativeViewHierarchyChanged() { - // When switching a window from one display to another, the compositor - // associated with the widget changes. - AddedToWidget(); -} - -void AnimatedImageView::AddedToWidget() { ui::Compositor* compositor = GetWidget()->GetCompositor(); DCHECK(compositor); if (compositor_ != compositor) { - if (compositor_ && compositor_->HasAnimationObserver(this)) - compositor_->RemoveAnimationObserver(this); - compositor_ = compositor; + ClearCurrentCompositor(); + + // Restore the Play() state with the new compositor. + if (state_ == State::kPlaying) + SetCompositorFromWidget(); } } void AnimatedImageView::RemovedFromWidget() { if (compositor_) { Stop(); - if (compositor_->HasAnimationObserver(this)) - compositor_->RemoveAnimationObserver(this); - compositor_ = nullptr; + ClearCurrentCompositor(); } } @@ -131,6 +121,22 @@ void AnimatedImageView::OnAnimationStep(base::TimeTicks timestamp) { void AnimatedImageView::OnCompositingShuttingDown(ui::Compositor* compositor) { if (compositor_ == compositor) { Stop(); + ClearCurrentCompositor(); + } +} + +void AnimatedImageView::SetCompositorFromWidget() { + DCHECK(!compositor_); + auto* widget = GetWidget(); + DCHECK(widget); + compositor_ = widget->GetCompositor(); + DCHECK(!compositor_->HasAnimationObserver(this)); + compositor_->AddAnimationObserver(this); +} + +void AnimatedImageView::ClearCurrentCompositor() { + if (compositor_) { + DCHECK(compositor_->HasAnimationObserver(this)); compositor_->RemoveAnimationObserver(this); compositor_ = nullptr; } diff --git a/chromium/ui/views/controls/animated_image_view.h b/chromium/ui/views/controls/animated_image_view.h index c62fe5c3723..f3868c28a97 100644 --- a/chromium/ui/views/controls/animated_image_view.h +++ b/chromium/ui/views/controls/animated_image_view.h @@ -48,7 +48,8 @@ class VIEWS_EXPORT AnimatedImageView : public ImageViewBase, void SetAnimatedImage( std::unique_ptr<gfx::SkiaVectorAnimation> animated_image); - // Plays the animation in loop. + // Plays the animation in loop and must only be called when this view has + // access to a widget. void Play(); // Stops any animation and resets it to the start frame. @@ -61,13 +62,15 @@ class VIEWS_EXPORT AnimatedImageView : public ImageViewBase, void OnPaint(gfx::Canvas* canvas) override; const char* GetClassName() const override; void NativeViewHierarchyChanged() override; - void AddedToWidget() override; void RemovedFromWidget() override; // Overridden from ui::CompositorAnimationObserver: void OnAnimationStep(base::TimeTicks timestamp) override; void OnCompositingShuttingDown(ui::Compositor* compositor) override; + void SetCompositorFromWidget(); + void ClearCurrentCompositor(); + // Overridden from ImageViewBase: gfx::Size GetImageSize() const override; diff --git a/chromium/ui/views/controls/button/blue_button.cc b/chromium/ui/views/controls/button/blue_button.cc deleted file mode 100644 index fcebb90db00..00000000000 --- a/chromium/ui/views/controls/button/blue_button.cc +++ /dev/null @@ -1,78 +0,0 @@ -// Copyright 2013 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "ui/views/controls/button/blue_button.h" - -#include <utility> - -#include "ui/base/resource/resource_bundle.h" -#include "ui/gfx/color_utils.h" -#include "ui/gfx/geometry/vector2d.h" -#include "ui/views/controls/button/label_button_border.h" -#include "ui/views/resources/grit/views_resources.h" - -namespace views { - -// static -const char BlueButton::kViewClassName[] = "views/BlueButton"; - -BlueButton::BlueButton(ButtonListener* listener, const base::string16& text) - : LabelButton(listener, text) { - // Inherit STYLE_BUTTON insets, minimum size, alignment, etc. - SetStyleDeprecated(STYLE_BUTTON); - UpdateThemedBorder(); -} - -BlueButton::~BlueButton() {} - -void BlueButton::ResetColorsFromNativeTheme() { - LabelButton::ResetColorsFromNativeTheme(); - if (!color_utils::IsInvertedColorScheme()) { - SetTextColor(STATE_NORMAL, GetNativeTheme()-> - GetSystemColor(ui::NativeTheme::kColorId_BlueButtonEnabledColor)); - SetTextColor(STATE_HOVERED, GetNativeTheme()-> - GetSystemColor(ui::NativeTheme::kColorId_BlueButtonHoverColor)); - SetTextColor(STATE_PRESSED, GetNativeTheme()-> - GetSystemColor(ui::NativeTheme::kColorId_BlueButtonPressedColor)); - SetTextColor(STATE_DISABLED, GetNativeTheme()-> - GetSystemColor(ui::NativeTheme::kColorId_BlueButtonDisabledColor)); - - label()->SetShadows(gfx::ShadowValues( - 1, gfx::ShadowValue( - gfx::Vector2d(0, 1), 0, - GetNativeTheme()->GetSystemColor( - ui::NativeTheme::kColorId_BlueButtonShadowColor)))); - } -} - -const char* BlueButton::GetClassName() const { - return BlueButton::kViewClassName; -} - -std::unique_ptr<LabelButtonBorder> BlueButton::CreateDefaultBorder() const { - // Insets for splitting the images. - const gfx::Insets insets(5); - std::unique_ptr<LabelButtonAssetBorder> button_border( - new LabelButtonAssetBorder(style())); - ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance(); - button_border->SetPainter(false, STATE_NORMAL, Painter::CreateImagePainter( - *rb.GetImageSkiaNamed(IDR_BLUE_BUTTON_NORMAL), insets)); - button_border->SetPainter(false, STATE_HOVERED, Painter::CreateImagePainter( - *rb.GetImageSkiaNamed(IDR_BLUE_BUTTON_HOVER), insets)); - button_border->SetPainter(false, STATE_PRESSED, Painter::CreateImagePainter( - *rb.GetImageSkiaNamed(IDR_BLUE_BUTTON_PRESSED), insets)); - button_border->SetPainter(false, STATE_DISABLED, Painter::CreateImagePainter( - *rb.GetImageSkiaNamed(IDR_BLUE_BUTTON_DISABLED), insets)); - button_border->SetPainter(true, STATE_NORMAL, Painter::CreateImagePainter( - *rb.GetImageSkiaNamed(IDR_BLUE_BUTTON_FOCUSED_NORMAL), insets)); - button_border->SetPainter(true, STATE_HOVERED, Painter::CreateImagePainter( - *rb.GetImageSkiaNamed(IDR_BLUE_BUTTON_FOCUSED_HOVER), insets)); - button_border->SetPainter(true, STATE_PRESSED, Painter::CreateImagePainter( - *rb.GetImageSkiaNamed(IDR_BLUE_BUTTON_FOCUSED_PRESSED), insets)); - button_border->SetPainter(true, STATE_DISABLED, Painter::CreateImagePainter( - *rb.GetImageSkiaNamed(IDR_BLUE_BUTTON_DISABLED), insets)); - return std::move(button_border); -} - -} // namespace views diff --git a/chromium/ui/views/controls/button/blue_button.h b/chromium/ui/views/controls/button/blue_button.h deleted file mode 100644 index ec414de1790..00000000000 --- a/chromium/ui/views/controls/button/blue_button.h +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright 2013 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef UI_VIEWS_CONTROLS_BUTTON_BLUE_BUTTON_H_ -#define UI_VIEWS_CONTROLS_BUTTON_BLUE_BUTTON_H_ - -#include "base/compiler_specific.h" -#include "base/macros.h" -#include "ui/views/controls/button/label_button.h" - -namespace views { - -// A class representing a blue button. -class VIEWS_EXPORT BlueButton : public LabelButton { - public: - static const char kViewClassName[]; - - BlueButton(ButtonListener* listener, const base::string16& text); - ~BlueButton() override; - - private: - // Overridden from LabelButton: - void ResetColorsFromNativeTheme() override; - const char* GetClassName() const override; - std::unique_ptr<LabelButtonBorder> CreateDefaultBorder() const override; - - DISALLOW_COPY_AND_ASSIGN(BlueButton); -}; - -} // namespace views - -#endif // UI_VIEWS_CONTROLS_BUTTON_BLUE_BUTTON_H_ diff --git a/chromium/ui/views/controls/button/blue_button_unittest.cc b/chromium/ui/views/controls/button/blue_button_unittest.cc deleted file mode 100644 index f516f44d0ed..00000000000 --- a/chromium/ui/views/controls/button/blue_button_unittest.cc +++ /dev/null @@ -1,69 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "ui/views/controls/button/blue_button.h" - -#include "base/strings/string16.h" -#include "base/strings/utf_string_conversions.h" -#include "build/build_config.h" -#include "cc/paint/skia_paint_canvas.h" -#include "ui/base/material_design/material_design_controller.h" -#include "ui/gfx/canvas.h" -#include "ui/gfx/skia_util.h" -#include "ui/views/controls/button/label_button_border.h" -#include "ui/views/test/widget_test.h" - -namespace views { - -using BlueButtonTest = test::WidgetTest; - -TEST_F(BlueButtonTest, Border) { - // The buttons must be added to a Widget so that borders are correctly - // applied once the NativeTheme is determined. - Widget* widget = CreateTopLevelPlatformWidget(); - - // Compared to a normal LabelButton... - LabelButton* button = new LabelButton(nullptr, base::ASCIIToUTF16("foo")); - EXPECT_EQ(Button::STYLE_TEXTBUTTON, button->style()); - // Focus painter by default. - EXPECT_TRUE(button->focus_painter_.get()); - - // Switch to the same style as BlueButton for a more compelling comparison. - button->SetStyleDeprecated(Button::STYLE_BUTTON); - EXPECT_EQ(Button::STYLE_BUTTON, button->style()); - EXPECT_FALSE(button->focus_painter_.get()); - - widget->GetContentsView()->AddChildView(button); - button->SizeToPreferredSize(); - - SkBitmap button_bitmap; - button_bitmap.allocN32Pixels(button->size().width(), button->size().height(), - true /* opaque */); - cc::SkiaPaintCanvas button_paint_canvas(button_bitmap); - gfx::Canvas button_canvas(&button_paint_canvas, 1.f); - button->border()->Paint(*button, &button_canvas); - - // ... a special blue border should be used. - BlueButton* blue_button = new BlueButton(nullptr, base::ASCIIToUTF16("foo")); - EXPECT_EQ(Button::STYLE_BUTTON, blue_button->style()); - EXPECT_FALSE(blue_button->focus_painter_.get()); - - widget->GetContentsView()->AddChildView(blue_button); - blue_button->SizeToPreferredSize(); - - SkBitmap blue_button_bitmap; - blue_button_bitmap.allocN32Pixels(blue_button->size().width(), - blue_button->size().height(), - true /* opaque */); - cc::SkiaPaintCanvas blue_button_paint_canvas(blue_button_bitmap); - gfx::Canvas blue_button_canvas(&blue_button_paint_canvas, 1.f); - blue_button->border()->Paint(*blue_button, &blue_button_canvas); - EXPECT_EQ(button->GetText(), blue_button->GetText()); - EXPECT_EQ(button->size(), blue_button->size()); - EXPECT_FALSE(gfx::BitmapsAreEqual(button_bitmap, blue_button_bitmap)); - - widget->CloseNow(); -} - -} // namespace views diff --git a/chromium/ui/views/controls/button/button.cc b/chromium/ui/views/controls/button/button.cc index ebf086404e3..ad5ab5f0470 100644 --- a/chromium/ui/views/controls/button/button.cc +++ b/chromium/ui/views/controls/button/button.cc @@ -15,7 +15,6 @@ #include "ui/native_theme/native_theme.h" #include "ui/views/animation/ink_drop_highlight.h" #include "ui/views/animation/ink_drop_impl.h" -#include "ui/views/controls/button/blue_button.h" #include "ui/views/controls/button/checkbox.h" #include "ui/views/controls/button/image_button.h" #include "ui/views/controls/button/label_button.h" @@ -44,6 +43,30 @@ const int kHoverFadeDurationMs = 150; } // namespace //////////////////////////////////////////////////////////////////////////////// +// WidgetObserverButtonBridge: +Button::WidgetObserverButtonBridge::WidgetObserverButtonBridge(Button* button) + : owner_(button) { + DCHECK(button->GetWidget()); + button->GetWidget()->AddObserver(this); +} + +Button::WidgetObserverButtonBridge::~WidgetObserverButtonBridge() { + if (owner_) + owner_->GetWidget()->RemoveObserver(this); +} + +void Button::WidgetObserverButtonBridge::OnWidgetActivationChanged( + Widget* widget, + bool active) { + owner_->WidgetActivationChanged(widget, active); +} + +void Button::WidgetObserverButtonBridge::OnWidgetDestroying(Widget* widget) { + widget->RemoveObserver(this); + owner_ = nullptr; +} + +//////////////////////////////////////////////////////////////////////////////// // Button, static public: // static @@ -89,8 +112,6 @@ void Button::SetFocusForPlatform() { void Button::SetTooltipText(const base::string16& tooltip_text) { tooltip_text_ = tooltip_text; - if (accessible_name_.empty()) - accessible_name_ = tooltip_text_; OnSetTooltipText(tooltip_text); TooltipTextChanged(); } @@ -100,6 +121,10 @@ void Button::SetAccessibleName(const base::string16& name) { NotifyAccessibilityEvent(ax::mojom::Event::kTextChanged, true); } +const base::string16& Button::GetAccessibleName() const { + return accessible_name_.empty() ? tooltip_text_ : accessible_name_; +} + void Button::SetState(ButtonState state) { if (state == state_) return; @@ -129,6 +154,14 @@ void Button::SetState(ButtonState state) { SchedulePaint(); } +Button::ButtonState Button::GetVisualState() const { + if (PlatformStyle::kInactiveWidgetControlsAppearDisabled && GetWidget() && + !GetWidget()->IsActive()) { + return STATE_DISABLED; + } + return state(); +} + void Button::StartThrobbing(int cycles_til_stop) { if (!animate_on_state_change_) return; @@ -318,11 +351,6 @@ bool Button::OnKeyReleased(const ui::KeyEvent& event) { } void Button::OnGestureEvent(ui::GestureEvent* event) { - if (state_ == STATE_DISABLED) { - InkDropHostView::OnGestureEvent(event); - return; - } - if (event->type() == ui::ET_GESTURE_TAP && IsTriggerableEvent(*event)) { // Set the button state to hot and start the animation fully faded in. The // GESTURE_END event issued immediately after will set the state to @@ -341,8 +369,6 @@ void Button::OnGestureEvent(ui::GestureEvent* event) { event->type() == ui::ET_GESTURE_END) { SetState(STATE_NORMAL); } - if (!event->handled()) - InkDropHostView::OnGestureEvent(event); } bool Button::AcceleratorPressed(const ui::Accelerator& accelerator) { @@ -399,7 +425,7 @@ void Button::OnPaint(gfx::Canvas* canvas) { void Button::GetAccessibleNodeData(ui::AXNodeData* node_data) { node_data->role = ax::mojom::Role::kButton; - node_data->SetName(accessible_name_); + node_data->SetName(GetAccessibleName()); if (!enabled()) node_data->SetRestriction(ax::mojom::Restriction::kDisabled); @@ -456,6 +482,15 @@ void Button::OnBlur() { SchedulePaint(); } +void Button::AddedToWidget() { + if (PlatformStyle::kInactiveWidgetControlsAppearDisabled) + widget_observer_ = std::make_unique<WidgetObserverButtonBridge>(this); +} + +void Button::RemovedFromWidget() { + widget_observer_.reset(); +} + std::unique_ptr<InkDrop> Button::CreateInkDrop() { std::unique_ptr<views::InkDropImpl> ink_drop = CreateDefaultInkDropImpl(); ink_drop->SetShowHighlightOnFocus(!focus_ring_); @@ -568,4 +603,8 @@ bool Button::ShouldEnterHoveredState() { return check_mouse_position && IsMouseHovered(); } +void Button::WidgetActivationChanged(Widget* widget, bool active) { + StateChanged(state()); +} + } // namespace views diff --git a/chromium/ui/views/controls/button/button.h b/chromium/ui/views/controls/button/button.h index 050512ffa34..6cdedf42582 100644 --- a/chromium/ui/views/controls/button/button.h +++ b/chromium/ui/views/controls/button/button.h @@ -15,6 +15,7 @@ #include "ui/views/animation/ink_drop_state.h" #include "ui/views/controls/focus_ring.h" #include "ui/views/painter.h" +#include "ui/views/widget/widget_observer.h" namespace views { @@ -87,7 +88,7 @@ class VIEWS_EXPORT Button : public InkDropHostView, void set_tag(int tag) { tag_ = tag; } void SetAccessibleName(const base::string16& name); - const base::string16& accessible_name() const { return accessible_name_; } + const base::string16& GetAccessibleName() const; // Get/sets the current display state of the button. ButtonState state() const { return state_; } @@ -96,6 +97,9 @@ class VIEWS_EXPORT Button : public InkDropHostView, // like event dispatching, focus traversals, etc. Calling SetEnabled(false) // will also set the state of |this| to STATE_DISABLED. void SetState(ButtonState state); + // Returns the visual appearance state of the button. This takes into account + // both the button's display state and the state of the containing widget. + ButtonState GetVisualState() const; // Starts throbbing. See HoverAnimation for a description of cycles_til_stop. // This method does nothing if |animate_on_state_change_| is false. @@ -183,6 +187,8 @@ class VIEWS_EXPORT Button : public InkDropHostView, const ViewHierarchyChangedDetails& details) override; void OnFocus() override; void OnBlur() override; + void AddedToWidget() override; + void RemovedFromWidget() override; // Overridden from InkDropHostView: std::unique_ptr<InkDrop> CreateInkDrop() override; @@ -261,6 +267,28 @@ class VIEWS_EXPORT Button : public InkDropHostView, private: FRIEND_TEST_ALL_PREFIXES(BlueButtonTest, Border); + // Bridge class to allow Button to observe a Widget without being a + // WidgetObserver. This is desirable because many Button subclasses are + // themselves WidgetObservers, and if Button is a WidgetObserver, any change + // to its WidgetObserver overrides requires updating all the subclasses as + // well. + class WidgetObserverButtonBridge : public WidgetObserver { + public: + WidgetObserverButtonBridge(Button* owner); + ~WidgetObserverButtonBridge() override; + + // WidgetObserver: + void OnWidgetActivationChanged(Widget* widget, bool active) override; + void OnWidgetDestroying(Widget* widget) override; + + private: + Button* owner_; + + DISALLOW_COPY_AND_ASSIGN(WidgetObserverButtonBridge); + }; + + void WidgetActivationChanged(Widget* widget, bool active); + // The text shown in a tooltip. base::string16 tooltip_text_; @@ -306,6 +334,8 @@ class VIEWS_EXPORT Button : public InkDropHostView, std::unique_ptr<Painter> focus_painter_; + std::unique_ptr<WidgetObserverButtonBridge> widget_observer_; + DISALLOW_COPY_AND_ASSIGN(Button); }; diff --git a/chromium/ui/views/controls/button/button_unittest.cc b/chromium/ui/views/controls/button/button_unittest.cc index 067653b4945..2164d40e437 100644 --- a/chromium/ui/views/controls/button/button_unittest.cc +++ b/chromium/ui/views/controls/button/button_unittest.cc @@ -13,7 +13,7 @@ #include "ui/display/screen.h" #include "ui/events/event_utils.h" #include "ui/events/test/event_generator.h" -#include "ui/views/animation/ink_drop_host.h" +#include "ui/views/animation/ink_drop_host_view.h" #include "ui/views/animation/ink_drop_impl.h" #include "ui/views/animation/test/ink_drop_host_view_test_api.h" #include "ui/views/animation/test/test_ink_drop.h" @@ -29,6 +29,7 @@ #include "ui/views/controls/textfield/textfield.h" #include "ui/views/style/platform_style.h" #include "ui/views/test/views_test_base.h" +#include "ui/views/widget/widget_utils.h" #if defined(USE_AURA) #include "ui/aura/test/test_cursor_client.h" @@ -78,7 +79,7 @@ class TestButton : public Button, public ButtonListener { void OnClickCanceled(const ui::Event& event) override { canceled_ = true; } - // InkDropHostView: + // Button: void AddInkDropLayer(ui::Layer* ink_drop_layer) override { ++ink_drop_layer_add_count_; Button::AddInkDropLayer(ink_drop_layer); @@ -177,7 +178,7 @@ class ButtonTest : public ViewsTestBase { // Tests that hover state changes correctly when visiblity/enableness changes. TEST_F(ButtonTest, HoverStateOnVisibilityChange) { - ui::test::EventGenerator generator(widget()->GetNativeWindow()); + ui::test::EventGenerator generator(GetRootWindow(widget())); generator.PressLeftButton(); EXPECT_EQ(Button::STATE_PRESSED, button()->state()); @@ -226,8 +227,7 @@ TEST_F(ButtonTest, HoverStateOnVisibilityChange) { // Disabling cursor events occurs for touch events and the Ash magnifier. There // is no touch on desktop Mac. Tracked in http://crbug.com/445520. #if !defined(OS_MACOSX) || defined(USE_AURA) - aura::test::TestCursorClient cursor_client( - widget()->GetNativeView()->GetRootWindow()); + aura::test::TestCursorClient cursor_client(GetRootWindow(widget())); // In Aura views, no new hover effects are invoked if mouse events // are disabled. @@ -250,7 +250,7 @@ TEST_F(ButtonTest, HoverStateOnVisibilityChange) { // Tests that the hover state is preserved during a view hierarchy update of a // button's child View. TEST_F(ButtonTest, HoverStatePreservedOnDescendantViewHierarchyChange) { - ui::test::EventGenerator generator(widget()->GetNativeWindow()); + ui::test::EventGenerator generator(GetRootWindow(widget())); generator.MoveMouseTo(button()->GetBoundsInScreen().CenterPoint()); EXPECT_EQ(Button::STATE_HOVERED, button()->state()); @@ -343,8 +343,7 @@ void PerformGesture(Button* button, ui::EventType event_type) { // Tests that gesture events correctly change the button state. TEST_F(ButtonTest, GestureEventsSetState) { - aura::test::TestCursorClient cursor_client( - widget()->GetNativeView()->GetRootWindow()); + aura::test::TestCursorClient cursor_client(GetRootWindow(widget())); EXPECT_EQ(Button::STATE_NORMAL, button()->state()); @@ -364,10 +363,10 @@ TEST_F(ButtonTest, GestureEventsSetState) { TEST_F(ButtonTest, AsButton) { base::string16 text; - LabelButton label_button(NULL, text); + LabelButton label_button(nullptr, text); EXPECT_TRUE(Button::AsButton(&label_button)); - ImageButton image_button(NULL); + ImageButton image_button(nullptr); EXPECT_TRUE(Button::AsButton(&image_button)); Checkbox checkbox(text); @@ -376,10 +375,10 @@ TEST_F(ButtonTest, AsButton) { RadioButton radio_button(text, 0); EXPECT_TRUE(Button::AsButton(&radio_button)); - MenuButton menu_button(text, NULL, false); + MenuButton menu_button(text, nullptr); EXPECT_TRUE(Button::AsButton(&menu_button)); - ToggleButton toggle_button(NULL); + ToggleButton toggle_button(nullptr); EXPECT_TRUE(Button::AsButton(&toggle_button)); Label label; @@ -400,8 +399,8 @@ TEST_F(ButtonTest, ButtonClickTogglesInkDrop) { TestInkDrop* ink_drop = new TestInkDrop(); CreateButtonWithInkDrop(base::WrapUnique(ink_drop), false); - ui::test::EventGenerator generator(widget()->GetNativeWindow()); - generator.set_current_location(gfx::Point(50, 50)); + ui::test::EventGenerator generator(GetRootWindow(widget())); + generator.set_current_screen_location(gfx::Point(50, 50)); generator.PressLeftButton(); EXPECT_EQ(InkDropState::ACTION_PENDING, ink_drop->GetTargetInkDropState()); @@ -415,8 +414,8 @@ TEST_F(ButtonTest, CaptureLossHidesInkDrop) { TestInkDrop* ink_drop = new TestInkDrop(); CreateButtonWithInkDrop(base::WrapUnique(ink_drop), false); - ui::test::EventGenerator generator(widget()->GetNativeWindow()); - generator.set_current_location(gfx::Point(50, 50)); + ui::test::EventGenerator generator(GetRootWindow(widget())); + generator.set_current_screen_location(gfx::Point(50, 50)); generator.PressLeftButton(); EXPECT_EQ(InkDropState::ACTION_PENDING, ink_drop->GetTargetInkDropState()); @@ -487,7 +486,7 @@ TEST_F(ButtonTest, HideInkDropHighlightOnDisable) { TestInkDrop* ink_drop = new TestInkDrop(); CreateButtonWithInkDrop(base::WrapUnique(ink_drop), false); - ui::test::EventGenerator generator(widget()->GetNativeWindow()); + ui::test::EventGenerator generator(GetRootWindow(widget())); generator.MoveMouseToInHost(10, 10); EXPECT_TRUE(ink_drop->is_hovered()); button()->SetEnabled(false); @@ -522,7 +521,7 @@ TEST_F(ButtonTest, HideInkDropHighlightWhenRemoved) { // Make sure that the button ink drop is hidden after the button gets removed. widget()->SetContentsView(&test_container); test_container.AddChildView(button()); - ui::test::EventGenerator generator(widget()->GetNativeWindow()); + ui::test::EventGenerator generator(GetRootWindow(widget())); generator.MoveMouseToInHost(2, 2); EXPECT_TRUE(ink_drop->is_hovered()); // Set ink-drop state to ACTIVATED to make sure that removing the container diff --git a/chromium/ui/views/controls/button/checkbox.cc b/chromium/ui/views/controls/button/checkbox.cc index 1a84d97ab26..1b688c6184e 100644 --- a/chromium/ui/views/controls/button/checkbox.cc +++ b/chromium/ui/views/controls/button/checkbox.cc @@ -164,7 +164,7 @@ SkColor Checkbox::GetIconImageColor(int icon_state) const { const SkColor active_color = (icon_state & IconState::CHECKED) ? GetNativeTheme()->GetSystemColor( - ui::NativeTheme::kColorId_FocusedBorderColor) + ui::NativeTheme::kColorId_ProminentButtonColor) // When unchecked, the icon color matches push button text color. : style::GetColor(*this, style::CONTEXT_BUTTON_MD, style::STYLE_PRIMARY); diff --git a/chromium/ui/views/controls/button/image_button_factory.cc b/chromium/ui/views/controls/button/image_button_factory.cc index baf33d4ef8d..9932416dc82 100644 --- a/chromium/ui/views/controls/button/image_button_factory.cc +++ b/chromium/ui/views/controls/button/image_button_factory.cc @@ -14,8 +14,9 @@ namespace views { -ImageButton* CreateVectorImageButton(ButtonListener* listener) { - ImageButton* button = new ImageButton(listener); +namespace { + +void ConfigureVectorImageButton(ImageButton* button) { button->SetInkDropMode(Button::InkDropMode::ON); button->set_has_ink_drop_action_on_click(true); button->SetImageAlignment(ImageButton::ALIGN_CENTER, @@ -23,21 +24,62 @@ ImageButton* CreateVectorImageButton(ButtonListener* listener) { button->SetFocusPainter(nullptr); button->SetBorder(CreateEmptyBorder( LayoutProvider::Get()->GetInsetsMetric(INSETS_VECTOR_IMAGE_BUTTON))); +} + +} // namespace + +ImageButton* CreateVectorImageButton(ButtonListener* listener) { + ImageButton* button = new ImageButton(listener); + ConfigureVectorImageButton(button); + return button; +} + +ToggleImageButton* CreateVectorToggleImageButton(ButtonListener* listener) { + ToggleImageButton* button = new ToggleImageButton(listener); + ConfigureVectorImageButton(button); return button; } void SetImageFromVectorIcon(ImageButton* button, const gfx::VectorIcon& icon, SkColor related_text_color) { + SetImageFromVectorIcon(button, icon, GetDefaultSizeOfVectorIcon(icon), + related_text_color); +} + +void SetImageFromVectorIcon(ImageButton* button, + const gfx::VectorIcon& icon, + int dip_size, + SkColor related_text_color) { const SkColor icon_color = color_utils::DeriveDefaultIconColor(related_text_color); const SkColor disabled_color = SkColorSetA(icon_color, gfx::kDisabledControlAlpha); - button->SetImage(Button::STATE_NORMAL, - gfx::CreateVectorIcon(icon, icon_color)); - button->SetImage(Button::STATE_DISABLED, - gfx::CreateVectorIcon(icon, disabled_color)); + const gfx::ImageSkia& normal_image = + gfx::CreateVectorIcon(icon, dip_size, icon_color); + const gfx::ImageSkia& disabled_image = + gfx::CreateVectorIcon(icon, dip_size, disabled_color); + + button->SetImage(Button::STATE_NORMAL, normal_image); + button->SetImage(Button::STATE_DISABLED, disabled_image); button->set_ink_drop_base_color(icon_color); } +void SetToggledImageFromVectorIcon(ToggleImageButton* button, + const gfx::VectorIcon& icon, + int dip_size, + SkColor related_text_color) { + const SkColor icon_color = + color_utils::DeriveDefaultIconColor(related_text_color); + const SkColor disabled_color = + SkColorSetA(icon_color, gfx::kDisabledControlAlpha); + const gfx::ImageSkia normal_image = + gfx::CreateVectorIcon(icon, dip_size, icon_color); + const gfx::ImageSkia disabled_image = + gfx::CreateVectorIcon(icon, dip_size, disabled_color); + + button->SetToggledImage(Button::STATE_NORMAL, &normal_image); + button->SetToggledImage(Button::STATE_DISABLED, &disabled_image); +} + } // views diff --git a/chromium/ui/views/controls/button/image_button_factory.h b/chromium/ui/views/controls/button/image_button_factory.h index 2bb9a229bd4..0ce421eee68 100644 --- a/chromium/ui/views/controls/button/image_button_factory.h +++ b/chromium/ui/views/controls/button/image_button_factory.h @@ -17,11 +17,18 @@ namespace views { class ButtonListener; class ImageButton; +class ToggleImageButton; // Creates an ImageButton with an ink drop and a centered image in preparation // for applying a vector icon with SetImageFromVectorIcon below. VIEWS_EXPORT ImageButton* CreateVectorImageButton(ButtonListener* listener); +// Creates a ToggleImageButton with an ink drop and a centered image in +// preperation for applying a vector icon from SetImageFromVectorIcon and +// SetToggledImageFromVectorIcon below. +VIEWS_EXPORT ToggleImageButton* CreateVectorToggleImageButton( + ButtonListener* listener); + // Sets images on |button| for STATE_NORMAL and STATE_DISABLED from the given // vector icon and color. |related_text_color| is normally the main text color // used in the parent view, and the actual color used is derived from that. Call @@ -31,6 +38,20 @@ VIEWS_EXPORT void SetImageFromVectorIcon( const gfx::VectorIcon& icon, SkColor related_text_color = gfx::kGoogleGrey900); +// As above, but creates the images at the given size. +VIEWS_EXPORT void SetImageFromVectorIcon( + ImageButton* button, + const gfx::VectorIcon& icon, + int dip_size, + SkColor related_text_color = gfx::kGoogleGrey900); + +// As above, but sets the toggled images for a toggled image button. +VIEWS_EXPORT void SetToggledImageFromVectorIcon( + ToggleImageButton* button, + const gfx::VectorIcon& icon, + int dip_size, + SkColor related_text_color = gfx::kGoogleGrey900); + } // namespace views #endif // UI_VIEWS_CONTROLS_BUTTON_IMAGE_BUTTON_FACTORY_H_ diff --git a/chromium/ui/views/controls/button/image_button_factory_unittest.cc b/chromium/ui/views/controls/button/image_button_factory_unittest.cc index 6a2e776d07d..f56c56cf044 100644 --- a/chromium/ui/views/controls/button/image_button_factory_unittest.cc +++ b/chromium/ui/views/controls/button/image_button_factory_unittest.cc @@ -20,7 +20,6 @@ TEST_F(ImageButtonFactoryTest, CreateVectorImageButton) { ImageButton* button = CreateVectorImageButton(nullptr); EXPECT_EQ(ImageButton::ALIGN_CENTER, button->h_alignment_); EXPECT_EQ(ImageButton::ALIGN_MIDDLE, button->v_alignment_); - EXPECT_TRUE(test::InkDropHostViewTestApi(button).HasGestureHandler()); delete button; } diff --git a/chromium/ui/views/controls/button/label_button.cc b/chromium/ui/views/controls/button/label_button.cc index 60907bd19cd..8fd404aa239 100644 --- a/chromium/ui/views/controls/button/label_button.cc +++ b/chromium/ui/views/controls/button/label_button.cc @@ -11,6 +11,7 @@ #include "base/lazy_instance.h" #include "base/logging.h" #include "build/build_config.h" +#include "ui/accessibility/ax_node_data.h" #include "ui/gfx/animation/throb_animation.h" #include "ui/gfx/canvas.h" #include "ui/gfx/color_utils.h" @@ -28,12 +29,16 @@ #include "ui/views/layout/layout_provider.h" #include "ui/views/painter.h" #include "ui/views/style/platform_style.h" +#include "ui/views/view_properties.h" #include "ui/views/window/dialog_delegate.h" namespace views { +namespace { +// The length of the hover fade animation. +constexpr int kHoverAnimationDurationMs = 170; +} // namespace // static -const int LabelButton::kHoverAnimationDurationMs = 170; const char LabelButton::kViewClassName[] = "LabelButton"; LabelButton::LabelButton(ButtonListener* listener, @@ -386,6 +391,10 @@ std::unique_ptr<InkDrop> LabelButton::CreateInkDrop() { } std::unique_ptr<views::InkDropRipple> LabelButton::CreateInkDropRipple() const { + // Views that use a highlight path use the base style and do not need the + // overrides in this file. + if (GetProperty(views::kHighlightPathKey)) + return InkDropHostView::CreateInkDropRipple(); return ShouldUseFloodFillInkDrop() ? std::make_unique<views::FloodFillInkDropRipple>( size(), GetInkDropCenterBasedOnLastEvent(), @@ -396,6 +405,10 @@ std::unique_ptr<views::InkDropRipple> LabelButton::CreateInkDropRipple() const { std::unique_ptr<views::InkDropHighlight> LabelButton::CreateInkDropHighlight() const { + // Views that use a highlight path use the base style and do not need the + // overrides in this file. + if (GetProperty(views::kHighlightPathKey)) + return InkDropHostView::CreateInkDropHighlight(); return ShouldUseFloodFillInkDrop() ? std::make_unique<views::InkDropHighlight>( size(), ink_drop_small_corner_radius(), @@ -405,6 +418,12 @@ std::unique_ptr<views::InkDropHighlight> LabelButton::CreateInkDropHighlight() gfx::RectF(image()->GetMirroredBounds()).CenterPoint()); } +void LabelButton::GetAccessibleNodeData(ui::AXNodeData* node_data) { + if (is_default()) + node_data->AddState(ax::mojom::State::kDefault); + Button::GetAccessibleNodeData(node_data); +} + void LabelButton::StateChanged(ButtonState old_state) { const gfx::Size previous_image_size(image_->GetPreferredSize()); UpdateImage(); @@ -484,7 +503,7 @@ void LabelButton::UpdateStyleToIndicateDefaultStatus() { } void LabelButton::UpdateImage() { - image_->SetImage(GetImage(state())); + image_->SetImage(GetImage(GetVisualState())); ResetCachedPreferredSize(); } diff --git a/chromium/ui/views/controls/button/label_button.h b/chromium/ui/views/controls/button/label_button.h index b2323dae3d9..1a1ab365ad7 100644 --- a/chromium/ui/views/controls/button/label_button.h +++ b/chromium/ui/views/controls/button/label_button.h @@ -28,9 +28,6 @@ class LabelButtonLabel; // LabelButton is a button with text and an icon, it's not focusable by default. class VIEWS_EXPORT LabelButton : public Button, public NativeThemeDelegate { public: - // The length of the hover fade animation. - static const int kHoverAnimationDurationMs; - static const char kViewClassName[]; // Creates a LabelButton with ButtonPressed() events sent to |listener| and @@ -101,6 +98,7 @@ class VIEWS_EXPORT LabelButton : public Button, public NativeThemeDelegate { std::unique_ptr<InkDrop> CreateInkDrop() override; std::unique_ptr<InkDropRipple> CreateInkDropRipple() const override; std::unique_ptr<InkDropHighlight> CreateInkDropHighlight() const override; + void GetAccessibleNodeData(ui::AXNodeData* node_data) override; protected: ImageView* image() const { return image_; } diff --git a/chromium/ui/views/controls/button/label_button_unittest.cc b/chromium/ui/views/controls/button/label_button_unittest.cc index 966ab6fec75..af7f28da1a0 100644 --- a/chromium/ui/views/controls/button/label_button_unittest.cc +++ b/chromium/ui/views/controls/button/label_button_unittest.cc @@ -22,12 +22,14 @@ #include "ui/gfx/geometry/vector2d.h" #include "ui/gfx/text_utils.h" #include "ui/native_theme/native_theme.h" +#include "ui/views/accessibility/view_accessibility.h" #include "ui/views/animation/test/ink_drop_host_view_test_api.h" #include "ui/views/animation/test/test_ink_drop.h" #include "ui/views/layout/layout_provider.h" #include "ui/views/style/platform_style.h" #include "ui/views/test/views_test_base.h" #include "ui/views/test/widget_test.h" +#include "ui/views/widget/widget_utils.h" using base::ASCIIToUTF16; @@ -212,6 +214,30 @@ TEST_F(LabelButtonTest, AccessibleState) { EXPECT_EQ(tooltip_text, tooltip); } +// Test View::GetAccessibleNodeData() for default buttons. +TEST_F(LabelButtonTest, AccessibleDefaultState) { + { + // If SetIsDefault() is not called, the ax default state should not be set. + ui::AXNodeData ax_data; + button_->GetViewAccessibility().GetAccessibleNodeData(&ax_data); + EXPECT_FALSE(ax_data.HasState(ax::mojom::State::kDefault)); + } + + { + button_->SetIsDefault(true); + ui::AXNodeData ax_data; + button_->GetViewAccessibility().GetAccessibleNodeData(&ax_data); + EXPECT_TRUE(ax_data.HasState(ax::mojom::State::kDefault)); + } + + { + button_->SetIsDefault(false); + ui::AXNodeData ax_data; + button_->GetViewAccessibility().GetAccessibleNodeData(&ax_data); + EXPECT_FALSE(ax_data.HasState(ax::mojom::State::kDefault)); + } +} + TEST_F(LabelButtonTest, Image) { const int small_size = 50, large_size = 100; const gfx::ImageSkia small_image = CreateTestImage(small_size, small_size); @@ -583,7 +609,7 @@ class InkDropLabelButtonTest : public ViewsTestBase { }; TEST_F(InkDropLabelButtonTest, HoverStateAfterMouseEnterAndExitEvents) { - ui::test::EventGenerator event_generator(widget_->GetNativeWindow()); + ui::test::EventGenerator event_generator(GetRootWindow(widget_.get())); const gfx::Point out_of_bounds_point(button_->bounds().bottom_right() + gfx::Vector2d(1, 1)); const gfx::Point in_bounds_point(button_->bounds().CenterPoint()); diff --git a/chromium/ui/views/controls/button/md_text_button.cc b/chromium/ui/views/controls/button/md_text_button.cc index 0d9f69cab39..c47cf87dcda 100644 --- a/chromium/ui/views/controls/button/md_text_button.cc +++ b/chromium/ui/views/controls/button/md_text_button.cc @@ -17,7 +17,6 @@ #include "ui/views/animation/ink_drop_painted_layer_delegates.h" #include "ui/views/background.h" #include "ui/views/border.h" -#include "ui/views/controls/button/blue_button.h" #include "ui/views/controls/focus_ring.h" #include "ui/views/layout/layout_provider.h" #include "ui/views/painter.h" diff --git a/chromium/ui/views/controls/button/menu_button.cc b/chromium/ui/views/controls/button/menu_button.cc index 7f38894e57c..23e59877636 100644 --- a/chromium/ui/views/controls/button/menu_button.cc +++ b/chromium/ui/views/controls/button/menu_button.cc @@ -4,185 +4,34 @@ #include "ui/views/controls/button/menu_button.h" -#include "base/strings/utf_string_conversions.h" -#include "ui/accessibility/ax_node_data.h" -#include "ui/base/dragdrop/drag_drop_types.h" -#include "ui/base/resource/resource_bundle.h" -#include "ui/display/screen.h" #include "ui/events/event.h" #include "ui/events/event_constants.h" -#include "ui/events/event_utils.h" #include "ui/gfx/canvas.h" -#include "ui/gfx/image/image.h" #include "ui/gfx/text_constants.h" -#include "ui/resources/grit/ui_resources.h" -#include "ui/views/controls/button/button.h" #include "ui/views/controls/button/menu_button_listener.h" #include "ui/views/mouse_constants.h" -#include "ui/views/widget/root_view.h" -#include "ui/views/widget/widget.h" - -using base::TimeTicks; -using base::TimeDelta; namespace views { - -// Default menu offset. -static const int kDefaultMenuOffsetX = -2; -static const int kDefaultMenuOffsetY = -4; - // static const char MenuButton::kViewClassName[] = "MenuButton"; const int MenuButton::kMenuMarkerPaddingLeft = 3; const int MenuButton::kMenuMarkerPaddingRight = -1; -//////////////////////////////////////////////////////////////////////////////// -// -// MenuButton::PressedLock -// -//////////////////////////////////////////////////////////////////////////////// - -MenuButton::PressedLock::PressedLock(MenuButton* menu_button) - : PressedLock(menu_button, false, nullptr) {} - -MenuButton::PressedLock::PressedLock(MenuButton* menu_button, - bool is_sibling_menu_show, - const ui::LocatedEvent* event) - : menu_button_(menu_button->weak_factory_.GetWeakPtr()) { - menu_button_->IncrementPressedLocked(is_sibling_menu_show, event); -} - -MenuButton::PressedLock::~PressedLock() { - if (menu_button_) - menu_button_->DecrementPressedLocked(); -} - -//////////////////////////////////////////////////////////////////////////////// -// -// MenuButton - constructors, destructors, initialization -// -//////////////////////////////////////////////////////////////////////////////// - MenuButton::MenuButton(const base::string16& text, - MenuButtonListener* menu_button_listener, - bool show_menu_marker) + MenuButtonListener* menu_button_listener) : LabelButton(nullptr, text), - menu_offset_(kDefaultMenuOffsetX, kDefaultMenuOffsetY), - listener_(menu_button_listener), - show_menu_marker_(show_menu_marker), - menu_marker_(ui::ResourceBundle::GetSharedInstance() - .GetImageNamed(IDR_MENU_DROPARROW) - .ToImageSkia()), - weak_factory_(this) { + menu_button_event_handler_(this, menu_button_listener) { SetHorizontalAlignment(gfx::ALIGN_LEFT); } MenuButton::~MenuButton() = default; -//////////////////////////////////////////////////////////////////////////////// -// -// MenuButton - Public APIs -// -//////////////////////////////////////////////////////////////////////////////// - bool MenuButton::Activate(const ui::Event* event) { - if (listener_) { - gfx::Rect lb = GetLocalBounds(); - - // The position of the menu depends on whether or not the locale is - // right-to-left. - gfx::Point menu_position(lb.right(), lb.bottom()); - if (base::i18n::IsRTL()) - menu_position.set_x(lb.x()); - - View::ConvertPointToScreen(this, &menu_position); - if (base::i18n::IsRTL()) - menu_position.Offset(-menu_offset_.x(), menu_offset_.y()); - else - menu_position.Offset(menu_offset_.x(), menu_offset_.y()); - - int max_x_coordinate = GetMaximumScreenXCoordinate(); - if (max_x_coordinate && max_x_coordinate <= menu_position.x()) - menu_position.set_x(max_x_coordinate - 1); - - // We're about to show the menu from a mouse press. By showing from the - // mouse press event we block RootView in mouse dispatching. This also - // appears to cause RootView to get a mouse pressed BEFORE the mouse - // release is seen, which means RootView sends us another mouse press no - // matter where the user pressed. To force RootView to recalculate the - // mouse target during the mouse press we explicitly set the mouse handler - // to NULL. - static_cast<internal::RootView*>(GetWidget()->GetRootView()) - ->SetMouseHandler(nullptr); - - DCHECK(increment_pressed_lock_called_ == nullptr); - // Observe if IncrementPressedLocked() was called so we can trigger the - // correct ink drop animations. - bool increment_pressed_lock_called = false; - increment_pressed_lock_called_ = &increment_pressed_lock_called; - - // Allow for OnMenuButtonClicked() to delete this. - auto ref = weak_factory_.GetWeakPtr(); - - // We don't set our state here. It's handled in the MenuController code or - // by our click listener. - listener_->OnMenuButtonClicked(this, menu_position, event); - - if (!ref) { - // The menu was deleted while showing. Don't attempt any processing. - return false; - } - - increment_pressed_lock_called_ = nullptr; - - if (!increment_pressed_lock_called && pressed_lock_count_ == 0) { - AnimateInkDrop(InkDropState::ACTION_TRIGGERED, - ui::LocatedEvent::FromIfValid(event)); - } - - // We must return false here so that the RootView does not get stuck - // sending all mouse pressed events to us instead of the appropriate - // target. - return false; - } - - AnimateInkDrop(InkDropState::HIDDEN, ui::LocatedEvent::FromIfValid(event)); - return true; + return menu_button_event_handler_.Activate(event); } bool MenuButton::IsTriggerableEventType(const ui::Event& event) { - if (event.IsMouseEvent()) { - const ui::MouseEvent& mouseev = static_cast<const ui::MouseEvent&>(event); - // Active on left mouse button only, to prevent a menu from being activated - // when a right-click would also activate a context menu. - if (!mouseev.IsOnlyLeftMouseButton()) - return false; - // If dragging is supported activate on release, otherwise activate on - // pressed. - ui::EventType active_on = - GetDragOperations(mouseev.location()) == ui::DragDropTypes::DRAG_NONE - ? ui::ET_MOUSE_PRESSED - : ui::ET_MOUSE_RELEASED; - return event.type() == active_on; - } - - return event.type() == ui::ET_GESTURE_TAP; -} - -//////////////////////////////////////////////////////////////////////////////// -// -// MenuButton - Events -// -//////////////////////////////////////////////////////////////////////////////// - -gfx::Size MenuButton::CalculatePreferredSize() const { - gfx::Size prefsize = LabelButton::CalculatePreferredSize(); - if (show_menu_marker_) { - prefsize.Enlarge(menu_marker_->width() + kMenuMarkerPaddingLeft + - kMenuMarkerPaddingRight, - 0); - } - return prefsize; + return menu_button_event_handler_.IsTriggerableEventType(event); } const char* MenuButton::GetClassName() const { @@ -190,222 +39,48 @@ const char* MenuButton::GetClassName() const { } bool MenuButton::OnMousePressed(const ui::MouseEvent& event) { - if (request_focus_on_press()) - RequestFocus(); - if (state() != STATE_DISABLED && HitTestPoint(event.location()) && - IsTriggerableEventType(event)) { - if (IsTriggerableEvent(event)) - return Activate(&event); - } - return true; + return menu_button_event_handler_.OnMousePressed(event); } void MenuButton::OnMouseReleased(const ui::MouseEvent& event) { - if (state() != STATE_DISABLED && IsTriggerableEvent(event) && - HitTestPoint(event.location()) && !InDrag()) { - Activate(&event); - } else { - AnimateInkDrop(InkDropState::HIDDEN, &event); - LabelButton::OnMouseReleased(event); - } -} - -void MenuButton::OnMouseEntered(const ui::MouseEvent& event) { - if (pressed_lock_count_ == 0) // Ignore mouse movement if state is locked. - LabelButton::OnMouseEntered(event); -} - -void MenuButton::OnMouseExited(const ui::MouseEvent& event) { - if (pressed_lock_count_ == 0) // Ignore mouse movement if state is locked. - LabelButton::OnMouseExited(event); -} - -void MenuButton::OnMouseMoved(const ui::MouseEvent& event) { - if (pressed_lock_count_ == 0) // Ignore mouse movement if state is locked. - LabelButton::OnMouseMoved(event); -} - -void MenuButton::OnGestureEvent(ui::GestureEvent* event) { - if (state() != STATE_DISABLED) { - auto ref = weak_factory_.GetWeakPtr(); - if (IsTriggerableEvent(*event) && !Activate(event)) { - if (!ref) - return; - // When |Activate()| returns |false|, it means the click was handled by - // a button listener and has handled the gesture event. So, there is no - // need to further process the gesture event here. However, if the - // listener didn't run menu code, we should make sure to reset our state. - if (state() == Button::STATE_HOVERED) - SetState(Button::STATE_NORMAL); - return; - } - if (event->type() == ui::ET_GESTURE_TAP_DOWN) { - event->SetHandled(); - if (pressed_lock_count_ == 0) - SetState(Button::STATE_HOVERED); - } else if (state() == Button::STATE_HOVERED && - (event->type() == ui::ET_GESTURE_TAP_CANCEL || - event->type() == ui::ET_GESTURE_END) && - pressed_lock_count_ == 0) { - SetState(Button::STATE_NORMAL); - } - } - LabelButton::OnGestureEvent(event); + menu_button_event_handler_.OnMouseReleased(event); } bool MenuButton::OnKeyPressed(const ui::KeyEvent& event) { - switch (event.key_code()) { - case ui::VKEY_SPACE: - // Alt-space on windows should show the window menu. - if (event.IsAltDown()) - break; - FALLTHROUGH; - case ui::VKEY_RETURN: - case ui::VKEY_UP: - case ui::VKEY_DOWN: { - // WARNING: we may have been deleted by the time Activate returns. - Activate(&event); - // This is to prevent the keyboard event from being dispatched twice. If - // the keyboard event is not handled, we pass it to the default handler - // which dispatches the event back to us causing the menu to get displayed - // again. Return true to prevent this. - return true; - } - default: - break; - } - return false; + return menu_button_event_handler_.OnKeyPressed(event); } bool MenuButton::OnKeyReleased(const ui::KeyEvent& event) { - // Override Button's implementation, which presses the button when - // you press space and clicks it when you release space. For a MenuButton - // we always activate the menu on key press. - return false; + return menu_button_event_handler_.OnKeyReleased(event); } void MenuButton::GetAccessibleNodeData(ui::AXNodeData* node_data) { - Button::GetAccessibleNodeData(node_data); - node_data->role = ax::mojom::Role::kPopUpButton; - node_data->SetHasPopup(ax::mojom::HasPopup::kMenu); - if (enabled()) - node_data->SetDefaultActionVerb(ax::mojom::DefaultActionVerb::kOpen); -} - -void MenuButton::PaintMenuMarker(gfx::Canvas* canvas) { - gfx::Insets insets = GetInsets(); - - // Using the Views mirroring infrastructure incorrectly flips icon content. - // Instead, manually mirror the position of the down arrow. - gfx::Rect arrow_bounds(width() - insets.right() - - menu_marker_->width() - kMenuMarkerPaddingRight, - height() / 2 - menu_marker_->height() / 2, - menu_marker_->width(), - menu_marker_->height()); - arrow_bounds.set_x(GetMirroredXForRect(arrow_bounds)); - canvas->DrawImageInt(*menu_marker_, arrow_bounds.x(), arrow_bounds.y()); + menu_button_event_handler_.GetAccessibleNodeData(node_data); } -gfx::Rect MenuButton::GetChildAreaBounds() { - gfx::Size s = size(); - - if (show_menu_marker_) { - s.set_width(s.width() - menu_marker_->width() - kMenuMarkerPaddingLeft - - kMenuMarkerPaddingRight); - } +void MenuButton::OnMouseEntered(const ui::MouseEvent& event) {} +void MenuButton::OnMouseExited(const ui::MouseEvent& event) {} +void MenuButton::OnMouseMoved(const ui::MouseEvent& event) {} +void MenuButton::OnGestureEvent(ui::GestureEvent* event) {} - return gfx::Rect(s); +void MenuButton::LabelButtonStateChanged(ButtonState old_state) { + LabelButton::StateChanged(old_state); } bool MenuButton::IsTriggerableEvent(const ui::Event& event) { - if (!IsTriggerableEventType(event)) - return false; - - TimeDelta delta = TimeTicks::Now() - menu_closed_time_; - if (delta.InMilliseconds() < kMinimumMsBetweenButtonClicks) - return false; // Not enough time since the menu closed. - - return true; + return menu_button_event_handler_.IsTriggerableEvent(event); } bool MenuButton::ShouldEnterPushedState(const ui::Event& event) { - return IsTriggerableEventType(event); + return menu_button_event_handler_.ShouldEnterPushedState(event); } void MenuButton::StateChanged(ButtonState old_state) { - if (pressed_lock_count_ != 0) { - // The button's state was changed while it was supposed to be locked in a - // pressed state. This shouldn't happen, but conceivably could if a caller - // tries to switch from enabled to disabled or vice versa while the button - // is pressed. - if (state() == STATE_NORMAL) - should_disable_after_press_ = false; - else if (state() == STATE_DISABLED) - should_disable_after_press_ = true; - } else { - LabelButton::StateChanged(old_state); - } + menu_button_event_handler_.StateChanged(old_state); } void MenuButton::NotifyClick(const ui::Event& event) { - // We don't forward events to the normal button listener, instead using the - // MenuButtonListener. - Activate(&event); -} - -void MenuButton::PaintButtonContents(gfx::Canvas* canvas) { - if (show_menu_marker_) - PaintMenuMarker(canvas); -} - -void MenuButton::IncrementPressedLocked(bool snap_ink_drop_to_activated, - const ui::LocatedEvent* event) { - ++pressed_lock_count_; - if (increment_pressed_lock_called_) - *increment_pressed_lock_called_ = true; - should_disable_after_press_ = state() == STATE_DISABLED; - if (state() != STATE_PRESSED) { - if (snap_ink_drop_to_activated) - GetInkDrop()->SnapToActivated(); - else - AnimateInkDrop(InkDropState::ACTIVATED, event); - } - SetState(STATE_PRESSED); - GetInkDrop()->SetHovered(false); -} - -void MenuButton::DecrementPressedLocked() { - --pressed_lock_count_; - DCHECK_GE(pressed_lock_count_, 0); - - // If this was the last lock, manually reset state to the desired state. - if (pressed_lock_count_ == 0) { - menu_closed_time_ = TimeTicks::Now(); - ButtonState desired_state = STATE_NORMAL; - if (should_disable_after_press_) { - desired_state = STATE_DISABLED; - should_disable_after_press_ = false; - } else if (GetWidget() && !GetWidget()->dragged_view() && - ShouldEnterHoveredState()) { - desired_state = STATE_HOVERED; - GetInkDrop()->SetHovered(true); - } - SetState(desired_state); - // The widget may be null during shutdown. If so, it doesn't make sense to - // try to add an ink drop effect. - if (GetWidget() && state() != STATE_PRESSED) - AnimateInkDrop(InkDropState::DEACTIVATED, nullptr /* event */); - } -} - -int MenuButton::GetMaximumScreenXCoordinate() { - if (!GetWidget()) { - NOTREACHED(); - return 0; - } - - gfx::Rect monitor_bounds = GetWidget()->GetWorkAreaBoundsInScreen(); - return monitor_bounds.right() - 1; + menu_button_event_handler_.NotifyClick(event); } -} // namespace views +} // namespace views
\ No newline at end of file diff --git a/chromium/ui/views/controls/button/menu_button.h b/chromium/ui/views/controls/button/menu_button.h index 8fb11e1cf9c..87e8faf7484 100644 --- a/chromium/ui/views/controls/button/menu_button.h +++ b/chromium/ui/views/controls/button/menu_button.h @@ -5,14 +5,10 @@ #ifndef UI_VIEWS_CONTROLS_BUTTON_MENU_BUTTON_H_ #define UI_VIEWS_CONTROLS_BUTTON_MENU_BUTTON_H_ -#include <string> - #include "base/macros.h" -#include "base/memory/weak_ptr.h" #include "base/strings/string16.h" -#include "base/time/time.h" -#include "ui/views/background.h" #include "ui/views/controls/button/label_button.h" +#include "ui/views/controls/button/menu_button_event_handler.h" namespace views { @@ -27,23 +23,6 @@ class MenuButtonListener; //////////////////////////////////////////////////////////////////////////////// class VIEWS_EXPORT MenuButton : public LabelButton { public: - // A scoped lock for keeping the MenuButton in STATE_PRESSED e.g., while a - // menu is running. These are cumulative. - class VIEWS_EXPORT PressedLock { - public: - explicit PressedLock(MenuButton* menu_button); - // |event| is the event that caused the button to be pressed. May be null. - PressedLock(MenuButton* menu_button, - bool is_sibling_menu_show, - const ui::LocatedEvent* event); - ~PressedLock(); - - private: - base::WeakPtr<MenuButton> menu_button_; - - DISALLOW_COPY_AND_ASSIGN(PressedLock); - }; - static const char kViewClassName[]; // How much padding to put on the left and right of the menu marker. @@ -52,104 +31,58 @@ class VIEWS_EXPORT MenuButton : public LabelButton { // Create a Button. MenuButton(const base::string16& text, - MenuButtonListener* menu_button_listener, - bool show_menu_marker); + MenuButtonListener* menu_button_listener); ~MenuButton() override; - bool show_menu_marker() const { return show_menu_marker_; } - void set_menu_marker(const gfx::ImageSkia* menu_marker) { - menu_marker_ = menu_marker; - } - const gfx::ImageSkia* menu_marker() const { return menu_marker_; } - - const gfx::Point& menu_offset() const { return menu_offset_; } - void set_menu_offset(int x, int y) { menu_offset_.SetPoint(x, y); } - - // Activate the button (called when the button is pressed). |event| is the - // event triggering the activation, if any. bool Activate(const ui::Event* event); - // Returns true if the event is of the proper type to potentially trigger an - // action. Since MenuButtons have properties other than event type (like - // last menu open time) to determine if an event is valid to activate the - // menu, this is distinct from IsTriggerableEvent(). + // TODO(cyan): Move into MenuButtonEventHandler. virtual bool IsTriggerableEventType(const ui::Event& event); - // Overridden from View: - gfx::Size CalculatePreferredSize() const override; + // View: const char* GetClassName() const override; bool OnMousePressed(const ui::MouseEvent& event) override; void OnMouseReleased(const ui::MouseEvent& event) override; - void OnMouseEntered(const ui::MouseEvent& event) override; - void OnMouseExited(const ui::MouseEvent& event) override; - void OnMouseMoved(const ui::MouseEvent& event) override; - void OnGestureEvent(ui::GestureEvent* event) override; bool OnKeyPressed(const ui::KeyEvent& event) override; bool OnKeyReleased(const ui::KeyEvent& event) override; void GetAccessibleNodeData(ui::AXNodeData* node_data) override; + void OnGestureEvent(ui::GestureEvent* event) override; - protected: - // Paint the menu marker image. - void PaintMenuMarker(gfx::Canvas* canvas); + // Disallow overriding of these methods. + // No-op implementations as these are handled in MenuButtonEventHandler (and + // should not be handled here as well). To override these you need to subclass + // MenuButtonEventHandler and replace this event handler of the button. + void OnMouseEntered(const ui::MouseEvent& event) final; + void OnMouseExited(const ui::MouseEvent& event) final; + void OnMouseMoved(const ui::MouseEvent& event) final; + + // Protected methods needed for MenuButtonEventHandler. + // TODO (cyan): Move these to a delegate interface. + using InkDropHostView::GetInkDrop; + using View::InDrag; + using View::GetDragOperations; + using Button::ShouldEnterHoveredState; + void LabelButtonStateChanged(ButtonState old_state); + + // Button: + bool IsTriggerableEvent(const ui::Event& event) override; - // Overridden from LabelButton: - gfx::Rect GetChildAreaBounds() override; + MenuButtonEventHandler* menu_button_event_handler() { + return &menu_button_event_handler_; + } - // Overridden from Button: - bool IsTriggerableEvent(const ui::Event& event) override; - bool ShouldEnterPushedState(const ui::Event& event) override; + protected: void StateChanged(ButtonState old_state) override; - void NotifyClick(const ui::Event& event) override; - void PaintButtonContents(gfx::Canvas* canvas) override; - // Offset of the associated menu position. - gfx::Point menu_offset_; + // Button: + bool ShouldEnterPushedState(const ui::Event& event) final; + void NotifyClick(const ui::Event& event) final; private: - friend class PressedLock; - - // Increment/decrement the number of "pressed" locks this button has, and - // set the state accordingly. The ink drop is snapped to the final ACTIVATED - // state if |snap_ink_drop_to_activated| is true, otherwise the ink drop will - // be animated to the ACTIVATED node_data. The ink drop is animated at the - // location of |event| if non-null, otherwise at the default location. - void IncrementPressedLocked(bool snap_ink_drop_to_activated, - const ui::LocatedEvent* event); - void DecrementPressedLocked(); - - // Compute the maximum X coordinate for the current screen. MenuButtons - // use this to make sure a menu is never shown off screen. - int GetMaximumScreenXCoordinate(); - - // We use a time object in order to keep track of when the menu was closed. - // The time is used for simulating menu behavior for the menu button; that - // is, if the menu is shown and the button is pressed, we need to close the - // menu. There is no clean way to get the second click event because the - // menu is displayed using a modal loop and, unlike regular menus in Windows, - // the button is not part of the displayed menu. - base::TimeTicks menu_closed_time_; - - // Our listener. Not owned. - MenuButtonListener* listener_; - - // Whether or not we're showing a drop marker. - bool show_menu_marker_; - - // The down arrow used to differentiate the menu button from normal buttons. - const gfx::ImageSkia* menu_marker_; - - // The current number of "pressed" locks this button has. - int pressed_lock_count_ = 0; - - // Used to let Activate() know if IncrementPressedLocked() was called. - bool* increment_pressed_lock_called_ = nullptr; - - // True if the button was in a disabled state when a menu was run, and should - // return to it once the press is complete. This can happen if, e.g., we - // programmatically show a menu on a disabled button. - bool should_disable_after_press_ = false; - - base::WeakPtrFactory<MenuButton> weak_factory_; + // All events get sent to this handler to be processed. + // TODO (cyan): This will be generalized into a ButtonEventHandler and moved + // into the Button class. + MenuButtonEventHandler menu_button_event_handler_; DISALLOW_COPY_AND_ASSIGN(MenuButton); }; diff --git a/chromium/ui/views/controls/button/menu_button_event_handler.cc b/chromium/ui/views/controls/button/menu_button_event_handler.cc new file mode 100644 index 00000000000..2960da32944 --- /dev/null +++ b/chromium/ui/views/controls/button/menu_button_event_handler.cc @@ -0,0 +1,384 @@ +// Copyright 2018 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "ui/views/controls/button/menu_button_event_handler.h" + +#include "ui/accessibility/ax_node_data.h" +#include "ui/base/dragdrop/drag_drop_types.h" +#include "ui/events/event_constants.h" +#include "ui/views/animation/ink_drop.h" +#include "ui/views/controls/button/menu_button.h" +#include "ui/views/controls/button/menu_button_listener.h" +#include "ui/views/mouse_constants.h" +#include "ui/views/widget/root_view.h" +#include "ui/views/widget/widget.h" + +using base::TimeTicks; +using base::TimeDelta; + +namespace views { + +//////////////////////////////////////////////////////////////////////////////// +// +// MenuButtonEventHandler::PressedLock +// +//////////////////////////////////////////////////////////////////////////////// + +MenuButtonEventHandler::PressedLock::PressedLock( + MenuButtonEventHandler* menu_button_event_handler) + : PressedLock(menu_button_event_handler, false, nullptr) {} + +MenuButtonEventHandler::PressedLock::PressedLock( + MenuButtonEventHandler* menu_button_event_handler, + bool is_sibling_menu_show, + const ui::LocatedEvent* event) + : menu_button_event_handler_( + menu_button_event_handler->weak_factory_.GetWeakPtr()) { + menu_button_event_handler_->IncrementPressedLocked(is_sibling_menu_show, + event); +} + +std::unique_ptr<MenuButtonEventHandler::PressedLock> +MenuButtonEventHandler::TakeLock() { + return TakeLock(false, nullptr); +} + +std::unique_ptr<MenuButtonEventHandler::PressedLock> +MenuButtonEventHandler::TakeLock(bool is_sibling_menu_show, + const ui::LocatedEvent* event) { + return std::make_unique<MenuButtonEventHandler::PressedLock>( + this, is_sibling_menu_show, event); +} + +MenuButtonEventHandler::PressedLock::~PressedLock() { + if (menu_button_event_handler_) + menu_button_event_handler_->DecrementPressedLocked(); +} + +//////////////////////////////////////////////////////////////////////////////// +// +// MenuButtonEventHandler +// +//////////////////////////////////////////////////////////////////////////////// + +MenuButtonEventHandler::MenuButtonEventHandler( + MenuButton* menu_button_parent, + MenuButtonListener* menu_button_listener) + : menu_button_parent_(menu_button_parent), listener_(menu_button_listener) { + menu_button_parent_->AddPreTargetHandler(this); +} + +MenuButtonEventHandler::~MenuButtonEventHandler() { + menu_button_parent_->RemovePreTargetHandler(this); +} + +bool MenuButtonEventHandler::OnMousePressed(const ui::MouseEvent& event) { + if (menu_button_parent_->request_focus_on_press()) + menu_button_parent_->RequestFocus(); + if (menu_button_parent_->state() != Button::STATE_DISABLED && + menu_button_parent_->HitTestPoint(event.location()) && + menu_button_parent_->IsTriggerableEventType(event)) { + if (menu_button_parent_->IsTriggerableEvent(event)) + return Activate(&event); + } + return true; +} + +void MenuButtonEventHandler::OnMouseReleased(const ui::MouseEvent& event) { + if (menu_button_parent_->state() != Button::STATE_DISABLED && + menu_button_parent_->IsTriggerableEvent(event) && + menu_button_parent_->HitTestPoint(event.location()) && + !menu_button_parent_->InDrag()) { + Activate(&event); + } else { + menu_button_parent_->AnimateInkDrop(InkDropState::HIDDEN, &event); + menu_button_parent_->LabelButton::OnMouseReleased(event); + } +} + +void MenuButtonEventHandler::OnMouseEntered(const ui::MouseEvent& event) { + if (pressed_lock_count_ == 0) // Ignore mouse movement if state is locked. + menu_button_parent_->LabelButton::OnMouseEntered(event); +} + +void MenuButtonEventHandler::OnMouseExited(const ui::MouseEvent& event) { + if (pressed_lock_count_ == 0) // Ignore mouse movement if state is locked. + menu_button_parent_->LabelButton::OnMouseExited(event); +} + +void MenuButtonEventHandler::OnMouseMoved(const ui::MouseEvent& event) { + if (pressed_lock_count_ == 0) // Ignore mouse movement if state is locked. + menu_button_parent_->LabelButton::OnMouseMoved(event); +} + +void MenuButtonEventHandler::OnGestureEvent(ui::GestureEvent* event) { + if (menu_button_parent_->state() != Button::STATE_DISABLED) { + auto ref = weak_factory_.GetWeakPtr(); + if (menu_button_parent_->IsTriggerableEvent(*event) && !Activate(event)) { + if (!ref) + return; + // When |Activate()| returns |false|, it means the click was handled by + // a button listener and has handled the gesture event. So, there is no + // need to further process the gesture event here. However, if the + // listener didn't run menu code, we should make sure to reset our state. + if (menu_button_parent_->state() == Button::STATE_HOVERED) + menu_button_parent_->SetState(Button::STATE_NORMAL); + return; + } + if (event->type() == ui::ET_GESTURE_TAP_DOWN) { + event->SetHandled(); + if (pressed_lock_count_ == 0) + menu_button_parent_->SetState(Button::STATE_HOVERED); + } else if (menu_button_parent_->state() == Button::STATE_HOVERED && + (event->type() == ui::ET_GESTURE_TAP_CANCEL || + event->type() == ui::ET_GESTURE_END) && + pressed_lock_count_ == 0) { + menu_button_parent_->SetState(Button::STATE_NORMAL); + } + } + menu_button_parent_->LabelButton::OnGestureEvent(event); +} + +bool MenuButtonEventHandler::OnKeyPressed(const ui::KeyEvent& event) { + switch (event.key_code()) { + case ui::VKEY_SPACE: + // Alt-space on windows should show the window menu. + if (event.IsAltDown()) + break; + FALLTHROUGH; + case ui::VKEY_RETURN: + case ui::VKEY_UP: + case ui::VKEY_DOWN: { + // WARNING: we may have been deleted by the time Activate returns. + Activate(&event); + // This is to prevent the keyboard event from being dispatched twice. If + // the keyboard event is not handled, we pass it to the default handler + // which dispatches the event back to us causing the menu to get displayed + // again. Return true to prevent this. + return true; + } + default: + break; + } + return false; +} + +bool MenuButtonEventHandler::OnKeyReleased(const ui::KeyEvent& event) { + // A MenuButton always activates the menu on key press. + return false; +} + +void MenuButtonEventHandler::GetAccessibleNodeData(ui::AXNodeData* node_data) { + menu_button_parent_->Button::GetAccessibleNodeData(node_data); + node_data->role = ax::mojom::Role::kPopUpButton; + node_data->SetHasPopup(ax::mojom::HasPopup::kMenu); + if (menu_button_parent_->enabled()) + node_data->SetDefaultActionVerb(ax::mojom::DefaultActionVerb::kOpen); +} + +bool MenuButtonEventHandler::ShouldEnterPushedState(const ui::Event& event) { + return menu_button_parent_->IsTriggerableEventType(event); +} + +void MenuButtonEventHandler::StateChanged(LabelButton::ButtonState old_state) { + if (pressed_lock_count_ != 0) { + // The button's state was changed while it was supposed to be locked in a + // pressed state. This shouldn't happen, but conceivably could if a caller + // tries to switch from enabled to disabled or vice versa while the button + // is pressed. + if (menu_button_parent_->state() == Button::STATE_NORMAL) + should_disable_after_press_ = false; + else if (menu_button_parent_->state() == Button::STATE_DISABLED) + should_disable_after_press_ = true; + } else { + menu_button_parent_->LabelButtonStateChanged(old_state); + } +} + +void MenuButtonEventHandler::NotifyClick(const ui::Event& event) { + // We don't forward events to the normal button listener, instead using the + // MenuButtonListener. + Activate(&event); +} + +void MenuButtonEventHandler::IncrementPressedLocked( + bool snap_ink_drop_to_activated, + const ui::LocatedEvent* event) { + ++pressed_lock_count_; + if (increment_pressed_lock_called_) + *(increment_pressed_lock_called_) = true; + should_disable_after_press_ = + menu_button_parent_->state() == Button::STATE_DISABLED; + if (menu_button_parent_->state() != Button::STATE_PRESSED) { + if (snap_ink_drop_to_activated) { + menu_button_parent_->GetInkDrop()->SnapToActivated(); + } else + menu_button_parent_->AnimateInkDrop(InkDropState::ACTIVATED, event); + } + menu_button_parent_->SetState(Button::STATE_PRESSED); + menu_button_parent_->GetInkDrop()->SetHovered(false); +} + +void MenuButtonEventHandler::DecrementPressedLocked() { + --pressed_lock_count_; + DCHECK_GE(pressed_lock_count_, 0); + + // If this was the last lock, manually reset state to the desired state. + if (pressed_lock_count_ == 0) { + menu_closed_time_ = TimeTicks::Now(); + LabelButton::ButtonState desired_state = Button::STATE_NORMAL; + if (should_disable_after_press_) { + desired_state = Button::STATE_DISABLED; + should_disable_after_press_ = false; + } else if (menu_button_parent_->GetWidget() && + !menu_button_parent_->GetWidget()->dragged_view() && + menu_button_parent_->ShouldEnterHoveredState()) { + desired_state = Button::STATE_HOVERED; + menu_button_parent_->GetInkDrop()->SetHovered(true); + } + menu_button_parent_->SetState(desired_state); + // The widget may be null during shutdown. If so, it doesn't make sense to + // try to add an ink drop effect. + if (menu_button_parent_->GetWidget() && + menu_button_parent_->state() != Button::STATE_PRESSED) + menu_button_parent_->AnimateInkDrop(InkDropState::DEACTIVATED, + nullptr /* event */); + } +} + +int MenuButtonEventHandler::GetMaximumScreenXCoordinate() { + if (!menu_button_parent_->GetWidget()) { + NOTREACHED(); + return 0; + } + + gfx::Rect monitor_bounds = + menu_button_parent_->GetWidget()->GetWorkAreaBoundsInScreen(); + return monitor_bounds.right() - 1; +} + +bool MenuButtonEventHandler::Activate(const ui::Event* event) { + if (listener_) { + gfx::Rect lb = menu_button_parent_->GetLocalBounds(); + + // Offset of the associated menu position. + constexpr gfx::Vector2d kMenuOffset{-2, -4}; + + // The position of the menu depends on whether or not the locale is + // right-to-left. + gfx::Point menu_position(lb.right(), lb.bottom()); + if (base::i18n::IsRTL()) + menu_position.set_x(lb.x()); + + View::ConvertPointToScreen(menu_button_parent_, &menu_position); + if (base::i18n::IsRTL()) + menu_position.Offset(-kMenuOffset.x(), kMenuOffset.y()); + else + menu_position += kMenuOffset; + + int max_x_coordinate = GetMaximumScreenXCoordinate(); + if (max_x_coordinate && max_x_coordinate <= menu_position.x()) + menu_position.set_x(max_x_coordinate - 1); + + // We're about to show the menu from a mouse press. By showing from the + // mouse press event we block RootView in mouse dispatching. This also + // appears to cause RootView to get a mouse pressed BEFORE the mouse + // release is seen, which means RootView sends us another mouse press no + // matter where the user pressed. To force RootView to recalculate the + // mouse target during the mouse press we explicitly set the mouse handler + // to NULL. + static_cast<internal::RootView*>( + menu_button_parent_->GetWidget()->GetRootView()) + ->SetMouseHandler(nullptr); + + DCHECK(increment_pressed_lock_called_ == nullptr); + // Observe if IncrementPressedLocked() was called so we can trigger the + // correct ink drop animations. + bool increment_pressed_lock_called = false; + increment_pressed_lock_called_ = &increment_pressed_lock_called; + + // Allow for OnMenuButtonClicked() to delete this. + auto ref = weak_factory_.GetWeakPtr(); + + // We don't set our state here. It's handled in the MenuController code or + // by our click listener. + listener_->OnMenuButtonClicked(menu_button_parent_, menu_position, event); + + if (!ref) { + // The menu was deleted while showing. Don't attempt any processing. + return false; + } + + increment_pressed_lock_called_ = nullptr; + + if (!increment_pressed_lock_called && pressed_lock_count_ == 0) { + menu_button_parent_->AnimateInkDrop(InkDropState::ACTION_TRIGGERED, + ui::LocatedEvent::FromIfValid(event)); + } + + // We must return false here so that the RootView does not get stuck + // sending all mouse pressed events to us instead of the appropriate + // target. + return false; + } + + menu_button_parent_->AnimateInkDrop(InkDropState::HIDDEN, + ui::LocatedEvent::FromIfValid(event)); + return true; +} + +bool MenuButtonEventHandler::IsTriggerableEventType(const ui::Event& event) { + if (event.IsMouseEvent()) { + const ui::MouseEvent& mouseev = static_cast<const ui::MouseEvent&>(event); + // Active on left mouse button only, to prevent a menu from being activated + // when a right-click would also activate a context menu. + if (!mouseev.IsOnlyLeftMouseButton()) + return false; + // If dragging is supported activate on release, otherwise activate on + // pressed. + ui::EventType active_on = + menu_button_parent_->GetDragOperations(mouseev.location()) == + ui::DragDropTypes::DRAG_NONE + ? ui::ET_MOUSE_PRESSED + : ui::ET_MOUSE_RELEASED; + return event.type() == active_on; + } + + return event.type() == ui::ET_GESTURE_TAP; +} + +// TODO (cyan): Move all of the OnMouse.* methods here. +// This gets more tricky because certain parts are still expected to be called +// in the View::base class. +void MenuButtonEventHandler::OnMouseEvent(ui::MouseEvent* event) { + switch (event->type()) { + case ui::ET_MOUSE_MOVED: + if ((event->flags() & + (ui::EF_LEFT_MOUSE_BUTTON | ui::EF_RIGHT_MOUSE_BUTTON | + ui::EF_MIDDLE_MOUSE_BUTTON)) == 0) { + OnMouseMoved(*event); + return; + } + FALLTHROUGH; + case ui::ET_MOUSE_ENTERED: + if (event->flags() & ui::EF_TOUCH_ACCESSIBILITY) + menu_button_parent_->NotifyAccessibilityEvent(ax::mojom::Event::kHover, + true); + OnMouseEntered(*event); + break; + case ui::ET_MOUSE_EXITED: + OnMouseExited(*event); + break; + default: + return; + } +} + +bool MenuButtonEventHandler::IsTriggerableEvent(const ui::Event& event) { + return menu_button_parent_->IsTriggerableEventType(event) && + (TimeTicks::Now() - menu_closed_time_).InMilliseconds() >= + kMinimumMsBetweenButtonClicks; +} + +} // namespace views diff --git a/chromium/ui/views/controls/button/menu_button_event_handler.h b/chromium/ui/views/controls/button/menu_button_event_handler.h new file mode 100644 index 00000000000..af44fa7b0a9 --- /dev/null +++ b/chromium/ui/views/controls/button/menu_button_event_handler.h @@ -0,0 +1,137 @@ +// Copyright 2018 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef UI_VIEWS_CONTROLS_BUTTON_MENU_BUTTON_EVENT_HANDLER_H_ +#define UI_VIEWS_CONTROLS_BUTTON_MENU_BUTTON_EVENT_HANDLER_H_ + +#include "base/macros.h" +#include "base/memory/weak_ptr.h" +#include "base/time/time.h" +#include "ui/events/event.h" +#include "ui/views/controls/button/button.h" + +namespace ui { +struct AXNodeData; +} +namespace views { +class MenuButton; +class MenuButtonListener; + +// An EventHandler that contains the event behavior of a MenuButton. +// TODO (cyan): This will eventually be available to any Button derivative class +// that wants MenuButton behavior. +class VIEWS_EXPORT MenuButtonEventHandler : public ui::EventHandler { + public: + // A scoped lock for keeping the MenuButton in STATE_PRESSED e.g., while a + // menu is running. These are cumulative. + class VIEWS_EXPORT PressedLock { + public: + explicit PressedLock(MenuButtonEventHandler* menu_button_event_handler); + + // |event| is the event that caused the button to be pressed. May be null. + PressedLock(MenuButtonEventHandler* menu_button_event_handler, + bool is_sibling_menu_show, + const ui::LocatedEvent* event); + + ~PressedLock(); + + private: + base::WeakPtr<MenuButtonEventHandler> menu_button_event_handler_; + + DISALLOW_COPY_AND_ASSIGN(PressedLock); + }; + + // TODO (cyan): MenuButtonEventHandler should take in a Button. + MenuButtonEventHandler(MenuButton* menu_button_parent, + MenuButtonListener* menu_button_listener); + ~MenuButtonEventHandler() override; + + // TODO (cyan): Methods in this block should override EventHandler methods. + // These are currently called by the corresponding method from view::Views. + bool OnMousePressed(const ui::MouseEvent& event); + void OnMouseReleased(const ui::MouseEvent& event); + void OnMouseEntered(const ui::MouseEvent& event); + void OnMouseExited(const ui::MouseEvent& event); + void OnMouseMoved(const ui::MouseEvent& event); + bool OnKeyPressed(const ui::KeyEvent& event); + bool OnKeyReleased(const ui::KeyEvent& event); + void GetAccessibleNodeData(ui::AXNodeData* node_data); + + // ui::EventHandler: + void OnGestureEvent(ui::GestureEvent* event) override; + + // Calls TakeLock with is_sibling_menu_show as false and a nullptr to the + // event. + std::unique_ptr<PressedLock> TakeLock(); + + // Convenience method to increment the lock count on a button to keep the + // button in a PRESSED state when a menu is showing. + std::unique_ptr<PressedLock> TakeLock(bool is_sibling_menu_show, + const ui::LocatedEvent* event); + + bool IsTriggerableEvent(const ui::Event& event); + + bool ShouldEnterPushedState(const ui::Event& event); + void StateChanged(Button::ButtonState old_state); + void NotifyClick(const ui::Event& event); + + // Activate the button (called when the button is pressed). |event| is the + // event triggering the activation, if any. + bool Activate(const ui::Event* event); + + // Returns true if the event is of the proper type to potentially trigger an + // action. Since MenuButtons have properties other than event type (like + // last menu open time) to determine if an event is valid to activate the + // menu, this is distinct from IsTriggerableEvent(). + bool IsTriggerableEventType(const ui::Event& event); + + void OnMouseEvent(ui::MouseEvent* event) override; + + private: + // Increment/decrement the number of "pressed" locks this button has, and + // set the state accordingly. The ink drop is snapped to the final ACTIVATED + // state if |snap_ink_drop_to_activated| is true, otherwise the ink drop + // will be animated to the ACTIVATED node_data. The ink drop is animated at + // the location of |event| if non-null, otherwise at the default location. + void IncrementPressedLocked(bool snap_ink_drop_to_activated, + const ui::LocatedEvent* event); + + void DecrementPressedLocked(); + + // Compute the maximum X coordinate for the current screen. MenuButtons + // use this to make sure a menu is never shown off screen. + int GetMaximumScreenXCoordinate(); + + MenuButton* const menu_button_parent_; + + // Our listener. Not owned. + MenuButtonListener* const listener_; + + // We use a time object in order to keep track of when the menu was closed. + // The time is used for simulating menu behavior for the menu button; that + // is, if the menu is shown and the button is pressed, we need to close the + // menu. There is no clean way to get the second click event because the + // menu is displayed using a modal loop and, unlike regular menus in + // Windows, the button is not part of the displayed menu. + base::TimeTicks menu_closed_time_; + + // The current number of "pressed" locks this button has. + int pressed_lock_count_ = 0; + + // Used to let Activate() know if IncrementPressedLocked() was called. + bool* increment_pressed_lock_called_ = nullptr; + + // True if the button was in a disabled state when a menu was run, and + // should return to it once the press is complete. This can happen if, e.g., + // we programmatically show a menu on a disabled button. + bool should_disable_after_press_ = false; + + base::WeakPtrFactory<MenuButtonEventHandler> weak_factory_{this}; + + DISALLOW_COPY_AND_ASSIGN(MenuButtonEventHandler); +}; + +} // namespace views + +#endif // UI_VIEWS_CONTROLS_BUTTON_MENU_BUTTON_EVENT_HANDLER_H_
\ No newline at end of file diff --git a/chromium/ui/views/controls/button/menu_button_unittest.cc b/chromium/ui/views/controls/button/menu_button_unittest.cc index 969b67c86c5..823940021dc 100644 --- a/chromium/ui/views/controls/button/menu_button_unittest.cc +++ b/chromium/ui/views/controls/button/menu_button_unittest.cc @@ -15,9 +15,11 @@ #include "ui/events/test/event_generator.h" #include "ui/views/animation/test/ink_drop_host_view_test_api.h" #include "ui/views/animation/test/test_ink_drop.h" +#include "ui/views/controls/button/menu_button_event_handler.h" #include "ui/views/controls/button/menu_button_listener.h" #include "ui/views/drag_controller.h" #include "ui/views/test/views_test_base.h" +#include "ui/views/widget/widget_utils.h" #if defined(USE_AURA) #include "ui/aura/client/drag_drop_client.h" @@ -38,8 +40,7 @@ class TestMenuButton : public MenuButton { public: explicit TestMenuButton(MenuButtonListener* menu_button_listener) : MenuButton(base::string16(ASCIIToUTF16("button")), - menu_button_listener, - false) {} + menu_button_listener) {} ~TestMenuButton() override {} @@ -88,10 +89,11 @@ class MenuButtonTest : public ViewsTestBase { void CreateMenuButton(MenuButtonListener* menu_button_listener) { CreateWidget(); - generator_.reset(new ui::test::EventGenerator(widget_->GetNativeWindow())); + generator_ = + std::make_unique<ui::test::EventGenerator>(GetRootWindow(widget_)); // Set initial mouse location in a consistent way so that the menu button we // are about to create initializes its hover state in a consistent manner. - generator_->set_current_location(gfx::Point(10, 10)); + generator_->set_current_screen_location(gfx::Point(10, 10)); button_ = new TestMenuButton(menu_button_listener); button_->SetBoundsRect(gfx::Rect(0, 0, 200, 20)); @@ -191,7 +193,7 @@ class PressStateMenuButtonListener : public MenuButtonListener { void OnMenuButtonClicked(MenuButton* source, const gfx::Point& point, const ui::Event* event) override { - pressed_lock_.reset(new MenuButton::PressedLock(menu_button_)); + pressed_lock_ = menu_button_->menu_button_event_handler()->TakeLock(); if (release_lock_) pressed_lock_.reset(); } @@ -201,7 +203,7 @@ class PressStateMenuButtonListener : public MenuButtonListener { private: MenuButton* menu_button_; - std::unique_ptr<MenuButton::PressedLock> pressed_lock_; + std::unique_ptr<MenuButtonEventHandler::PressedLock> pressed_lock_; // The |pressed_lock_| will be released when true. bool release_lock_; @@ -371,7 +373,8 @@ TEST_F(MenuButtonTest, InkDropCenterSetFromClickWithPressedLock) { gfx::Point click_point(11, 7); ui::MouseEvent click_event(ui::EventType::ET_MOUSE_PRESSED, click_point, click_point, base::TimeTicks(), 0, 0); - MenuButton::PressedLock pressed_lock(button(), false, &click_event); + MenuButtonEventHandler::PressedLock pressed_lock( + button()->menu_button_event_handler(), false, &click_event); EXPECT_EQ(Button::STATE_PRESSED, button()->state()); EXPECT_EQ( @@ -394,8 +397,9 @@ TEST_F(MenuButtonTest, ButtonStateForMenuButtonsWithPressedLocks) { EXPECT_EQ(Button::STATE_HOVERED, button()->state()); // Introduce a PressedLock, which should make the button pressed. - std::unique_ptr<MenuButton::PressedLock> pressed_lock1( - new MenuButton::PressedLock(button())); + std::unique_ptr<MenuButtonEventHandler::PressedLock> pressed_lock1( + new MenuButtonEventHandler::PressedLock( + button()->menu_button_event_handler())); EXPECT_EQ(Button::STATE_PRESSED, button()->state()); // Even if we move the mouse outside of the button, it should remain pressed. @@ -403,8 +407,9 @@ TEST_F(MenuButtonTest, ButtonStateForMenuButtonsWithPressedLocks) { EXPECT_EQ(Button::STATE_PRESSED, button()->state()); // Creating a new lock should obviously keep the button pressed. - std::unique_ptr<MenuButton::PressedLock> pressed_lock2( - new MenuButton::PressedLock(button())); + std::unique_ptr<MenuButtonEventHandler::PressedLock> pressed_lock2( + new MenuButtonEventHandler::PressedLock( + button()->menu_button_event_handler())); EXPECT_EQ(Button::STATE_PRESSED, button()->state()); // The button should remain pressed while any locks are active. @@ -421,7 +426,7 @@ TEST_F(MenuButtonTest, ButtonStateForMenuButtonsWithPressedLocks) { // Test that the button returns to the appropriate state after the press; if // the mouse ends over the button, the button should be hovered. - pressed_lock1.reset(new MenuButton::PressedLock(button())); + pressed_lock1 = button()->menu_button_event_handler()->TakeLock(); EXPECT_EQ(Button::STATE_PRESSED, button()->state()); pressed_lock1.reset(); EXPECT_EQ(Button::STATE_HOVERED, button()->state()); @@ -429,7 +434,7 @@ TEST_F(MenuButtonTest, ButtonStateForMenuButtonsWithPressedLocks) { // If the button is disabled before the pressed lock, it should be disabled // after the pressed lock. button()->SetState(Button::STATE_DISABLED); - pressed_lock1.reset(new MenuButton::PressedLock(button())); + pressed_lock1 = button()->menu_button_event_handler()->TakeLock(); EXPECT_EQ(Button::STATE_PRESSED, button()->state()); pressed_lock1.reset(); EXPECT_EQ(Button::STATE_DISABLED, button()->state()); @@ -438,7 +443,7 @@ TEST_F(MenuButtonTest, ButtonStateForMenuButtonsWithPressedLocks) { // Edge case: the button is disabled, a pressed lock is added, and then the // button is re-enabled. It should be enabled after the lock is removed. - pressed_lock1.reset(new MenuButton::PressedLock(button())); + pressed_lock1 = button()->menu_button_event_handler()->TakeLock(); EXPECT_EQ(Button::STATE_PRESSED, button()->state()); button()->SetState(Button::STATE_NORMAL); pressed_lock1.reset(); @@ -516,13 +521,15 @@ TEST_F(MenuButtonTest, TEST_F(MenuButtonTest, InkDropStateForMenuButtonsWithPressedLocks) { CreateMenuButtonWithNoListener(); - std::unique_ptr<MenuButton::PressedLock> pressed_lock1( - new MenuButton::PressedLock(button())); + std::unique_ptr<MenuButtonEventHandler::PressedLock> pressed_lock1( + new MenuButtonEventHandler::PressedLock( + button()->menu_button_event_handler())); EXPECT_EQ(InkDropState::ACTIVATED, ink_drop()->GetTargetInkDropState()); - std::unique_ptr<MenuButton::PressedLock> pressed_lock2( - new MenuButton::PressedLock(button())); + std::unique_ptr<MenuButtonEventHandler::PressedLock> pressed_lock2( + new MenuButtonEventHandler::PressedLock( + button()->menu_button_event_handler())); EXPECT_EQ(InkDropState::ACTIVATED, ink_drop()->GetTargetInkDropState()); @@ -538,14 +545,16 @@ TEST_F(MenuButtonTest, InkDropStateForMenuButtonsWithPressedLocks) { TEST_F(MenuButtonTest, OneInkDropAnimationForReentrantPressedLocks) { CreateMenuButtonWithNoListener(); - std::unique_ptr<MenuButton::PressedLock> pressed_lock1( - new MenuButton::PressedLock(button())); + std::unique_ptr<MenuButtonEventHandler::PressedLock> pressed_lock1( + new MenuButtonEventHandler::PressedLock( + button()->menu_button_event_handler())); EXPECT_EQ(InkDropState::ACTIVATED, ink_drop()->GetTargetInkDropState()); ink_drop()->AnimateToState(InkDropState::ACTION_PENDING); - std::unique_ptr<MenuButton::PressedLock> pressed_lock2( - new MenuButton::PressedLock(button())); + std::unique_ptr<MenuButtonEventHandler::PressedLock> pressed_lock2( + new MenuButtonEventHandler::PressedLock( + button()->menu_button_event_handler())); EXPECT_EQ(InkDropState::ACTION_PENDING, ink_drop()->GetTargetInkDropState()); } @@ -556,7 +565,8 @@ TEST_F(MenuButtonTest, InkDropStateForMenuButtonWithPressedLockBeforeActivation) { TestMenuButtonListener menu_button_listener; CreateMenuButtonWithMenuButtonListener(&menu_button_listener); - MenuButton::PressedLock lock(button()); + MenuButtonEventHandler::PressedLock lock( + button()->menu_button_event_handler()); button()->Activate(nullptr); @@ -585,6 +595,7 @@ TEST_F(MenuButtonTest, DraggableMenuButtonDoesNotActivateOnDrag) { generator()->DragMouseBy(10, 0); EXPECT_EQ(nullptr, menu_button_listener.last_source()); EXPECT_EQ(Button::STATE_NORMAL, menu_button_listener.last_source_state()); + button()->RemovePreTargetHandler(&drag_client); } #endif // USE_AURA @@ -692,7 +703,7 @@ TEST_F(MenuButtonTest, class DestroyButtonInGestureListener : public MenuButtonListener { public: DestroyButtonInGestureListener() { - menu_button_ = std::make_unique<MenuButton>(base::string16(), this, true); + menu_button_ = std::make_unique<MenuButton>(base::string16(), this); } ~DestroyButtonInGestureListener() override = default; diff --git a/chromium/ui/views/controls/button/toggle_button_unittest.cc b/chromium/ui/views/controls/button/toggle_button_unittest.cc index 2315f7935b7..1b77d087c29 100644 --- a/chromium/ui/views/controls/button/toggle_button_unittest.cc +++ b/chromium/ui/views/controls/button/toggle_button_unittest.cc @@ -10,6 +10,7 @@ #include "ui/events/event_utils.h" #include "ui/events/test/event_generator.h" #include "ui/views/test/views_test_base.h" +#include "ui/views/widget/widget_utils.h" namespace views { @@ -108,7 +109,7 @@ TEST_F(ToggleButtonTest, ShutdownWithFocus) { // Verify that ToggleButton::accepts_events_ works as expected. TEST_F(ToggleButtonTest, AcceptEvents) { EXPECT_FALSE(button()->is_on()); - ui::test::EventGenerator generator(widget()->GetNativeWindow()); + ui::test::EventGenerator generator(GetRootWindow(widget())); // Clicking toggles. generator.ClickLeftButton(); diff --git a/chromium/ui/views/controls/combobox/combobox.cc b/chromium/ui/views/controls/combobox/combobox.cc index f900b99d012..41a5ac2b1b3 100644 --- a/chromium/ui/views/controls/combobox/combobox.cc +++ b/chromium/ui/views/controls/combobox/combobox.cc @@ -4,93 +4,41 @@ #include "ui/views/controls/combobox/combobox.h" -#include <stddef.h> - -#include <utility> - #include "base/logging.h" -#include "base/macros.h" #include "build/build_config.h" #include "ui/accessibility/ax_action_data.h" #include "ui/accessibility/ax_node_data.h" -#include "ui/base/default_style.h" #include "ui/base/ime/input_method.h" +#include "ui/base/models/combobox_model.h" #include "ui/base/models/combobox_model_observer.h" -#include "ui/base/resource/resource_bundle.h" +#include "ui/base/models/menu_model.h" #include "ui/events/event.h" -#include "ui/gfx/animation/throb_animation.h" #include "ui/gfx/canvas.h" #include "ui/gfx/color_palette.h" #include "ui/gfx/scoped_canvas.h" #include "ui/gfx/text_utils.h" -#include "ui/native_theme/common_theme.h" #include "ui/native_theme/native_theme.h" -#include "ui/native_theme/native_theme_aura.h" -#include "ui/resources/grit/ui_resources.h" #include "ui/views/animation/flood_fill_ink_drop_ripple.h" -#include "ui/views/animation/ink_drop_highlight.h" #include "ui/views/animation/ink_drop_impl.h" #include "ui/views/background.h" -#include "ui/views/controls/button/button.h" -#include "ui/views/controls/button/label_button.h" #include "ui/views/controls/combobox/combobox_listener.h" #include "ui/views/controls/focus_ring.h" #include "ui/views/controls/focusable_border.h" #include "ui/views/controls/menu/menu_config.h" #include "ui/views/controls/menu/menu_runner.h" #include "ui/views/controls/prefix_selector.h" -#include "ui/views/controls/textfield/textfield.h" #include "ui/views/layout/layout_provider.h" #include "ui/views/mouse_constants.h" -#include "ui/views/painter.h" -#include "ui/views/resources/grit/views_resources.h" #include "ui/views/style/platform_style.h" +#include "ui/views/style/typography.h" #include "ui/views/widget/widget.h" namespace views { namespace { -// STYLE_ACTION arrow container padding widths. -const int kActionLeftPadding = 12; -const int kActionRightPadding = 11; - -// Menu border widths -const int kMenuBorderWidthLeft = 1; -const int kMenuBorderWidthTop = 1; -const int kMenuBorderWidthRight = 1; - -// Limit how small a combobox can be. -const int kMinComboboxWidth = 25; - -// Define the id of the first item in the menu (since it needs to be > 0) -const int kFirstMenuItemId = 1000; - // Used to indicate that no item is currently selected by the user. -const int kNoSelection = -1; - -const int kBodyButtonImages[] = IMAGE_GRID(IDR_COMBOBOX_BUTTON); -const int kHoveredBodyButtonImages[] = IMAGE_GRID(IDR_COMBOBOX_BUTTON_H); -const int kPressedBodyButtonImages[] = IMAGE_GRID(IDR_COMBOBOX_BUTTON_P); -const int kFocusedBodyButtonImages[] = IMAGE_GRID(IDR_COMBOBOX_BUTTON_F); -const int kFocusedHoveredBodyButtonImages[] = - IMAGE_GRID(IDR_COMBOBOX_BUTTON_F_H); -const int kFocusedPressedBodyButtonImages[] = - IMAGE_GRID(IDR_COMBOBOX_BUTTON_F_P); - -#define MENU_IMAGE_GRID(x) { \ - x ## _MENU_TOP, x ## _MENU_CENTER, x ## _MENU_BOTTOM, } - -const int kMenuButtonImages[] = MENU_IMAGE_GRID(IDR_COMBOBOX_BUTTON); -const int kHoveredMenuButtonImages[] = MENU_IMAGE_GRID(IDR_COMBOBOX_BUTTON_H); -const int kPressedMenuButtonImages[] = MENU_IMAGE_GRID(IDR_COMBOBOX_BUTTON_P); -const int kFocusedMenuButtonImages[] = MENU_IMAGE_GRID(IDR_COMBOBOX_BUTTON_F); -const int kFocusedHoveredMenuButtonImages[] = - MENU_IMAGE_GRID(IDR_COMBOBOX_BUTTON_F_H); -const int kFocusedPressedMenuButtonImages[] = - MENU_IMAGE_GRID(IDR_COMBOBOX_BUTTON_F_P); - -#undef MENU_IMAGE_GRID +constexpr int kNoSelection = -1; SkColor GetTextColorForEnableState(const Combobox& combobox, bool enabled) { return style::GetColor( @@ -98,30 +46,10 @@ SkColor GetTextColorForEnableState(const Combobox& combobox, bool enabled) { enabled ? style::STYLE_PRIMARY : style::STYLE_DISABLED); } -gfx::Rect PositionArrowWithinContainer(const gfx::Rect& container_bounds, - const gfx::Size& arrow_size, - Combobox::Style style) { - gfx::Rect bounds(container_bounds); - if (style == Combobox::STYLE_ACTION) { - // This positions the arrow horizontally. The later call to - // ClampToCenteredSize will position it vertically without touching the - // horizontal position. - bounds.Inset(kActionLeftPadding, 0, kActionRightPadding, 0); - DCHECK_EQ(bounds.width(), arrow_size.width()); - } - - bounds.ClampToCenteredSize(arrow_size); - return bounds; -} - // The transparent button which holds a button state but is not rendered. class TransparentButton : public Button { public: - TransparentButton(ButtonListener* listener, bool animate_state_change) - : Button(listener) { - set_animate_on_state_change(animate_state_change); - if (animate_state_change) - SetAnimationDuration(LabelButton::kHoverAnimationDurationMs); + explicit TransparentButton(ButtonListener* listener) : Button(listener) { SetFocusBehavior(FocusBehavior::NEVER); set_notify_action(PlatformStyle::kMenuNotifyActivationAction); @@ -180,103 +108,6 @@ int GetAdjacentIndex(ui::ComboboxModel* model, int increment, int index) { } #endif -// Returns the image resource ids of an array for the body button. -// -// TODO(hajimehoshi): This function should return the images for the 'disabled' -// status. (crbug/270052) -const int* GetBodyButtonImageIds(bool focused, - Button::ButtonState state, - size_t* num) { - DCHECK(num); - *num = 9; - switch (state) { - case Button::STATE_DISABLED: - return focused ? kFocusedBodyButtonImages : kBodyButtonImages; - case Button::STATE_NORMAL: - return focused ? kFocusedBodyButtonImages : kBodyButtonImages; - case Button::STATE_HOVERED: - return focused ? - kFocusedHoveredBodyButtonImages : kHoveredBodyButtonImages; - case Button::STATE_PRESSED: - return focused ? - kFocusedPressedBodyButtonImages : kPressedBodyButtonImages; - default: - NOTREACHED(); - } - return NULL; -} - -// Returns the image resource ids of an array for the menu button. -const int* GetMenuButtonImageIds(bool focused, - Button::ButtonState state, - size_t* num) { - DCHECK(num); - *num = 3; - switch (state) { - case Button::STATE_DISABLED: - return focused ? kFocusedMenuButtonImages : kMenuButtonImages; - case Button::STATE_NORMAL: - return focused ? kFocusedMenuButtonImages : kMenuButtonImages; - case Button::STATE_HOVERED: - return focused ? - kFocusedHoveredMenuButtonImages : kHoveredMenuButtonImages; - case Button::STATE_PRESSED: - return focused ? - kFocusedPressedMenuButtonImages : kPressedMenuButtonImages; - default: - NOTREACHED(); - } - return NULL; -} - -// Returns the images for the menu buttons. -std::vector<const gfx::ImageSkia*> GetMenuButtonImages( - bool focused, - Button::ButtonState state) { - const int* ids; - size_t num_ids; - ids = GetMenuButtonImageIds(focused, state, &num_ids); - std::vector<const gfx::ImageSkia*> images; - images.reserve(num_ids); - ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance(); - for (size_t i = 0; i < num_ids; i++) - images.push_back(rb.GetImageSkiaNamed(ids[i])); - return images; -} - -// Paints three images in a column at the given location. The center image is -// stretched so as to fit the given height. -void PaintImagesVertically(gfx::Canvas* canvas, - const gfx::ImageSkia& top_image, - const gfx::ImageSkia& center_image, - const gfx::ImageSkia& bottom_image, - int x, int y, int width, int height) { - canvas->DrawImageInt(top_image, - 0, 0, top_image.width(), top_image.height(), - x, y, width, top_image.height(), false); - y += top_image.height(); - int center_height = height - top_image.height() - bottom_image.height(); - canvas->DrawImageInt(center_image, - 0, 0, center_image.width(), center_image.height(), - x, y, width, center_height, false); - y += center_height; - canvas->DrawImageInt(bottom_image, - 0, 0, bottom_image.width(), bottom_image.height(), - x, y, width, bottom_image.height(), false); -} - -// Paints the arrow button. -void PaintArrowButton( - gfx::Canvas* canvas, - const std::vector<const gfx::ImageSkia*>& arrow_button_images, - int x, int height) { - PaintImagesVertically(canvas, - *arrow_button_images[0], - *arrow_button_images[1], - *arrow_button_images[2], - x, 0, arrow_button_images[0]->width(), height); -} - } // namespace // static @@ -295,8 +126,7 @@ class Combobox::ComboboxMenuModel : public ui::MenuModel, private: bool UseCheckmarks() const { - return owner_->style_ != STYLE_ACTION && - MenuConfig::instance().check_selected_combobox_item; + return MenuConfig::instance().check_selected_combobox_item; } // Overridden from MenuModel: @@ -305,12 +135,8 @@ class Combobox::ComboboxMenuModel : public ui::MenuModel, int GetItemCount() const override { return model_->GetItemCount(); } ItemType GetTypeAt(int index) const override { - if (model_->IsItemSeparatorAt(index)) { - // In action menus, disallow <item>, <separator>, ... since that would put - // a separator at the top of the menu. - DCHECK(index != 1 || owner_->style_ != STYLE_ACTION); + if (model_->IsItemSeparatorAt(index)) return TYPE_SEPARATOR; - } return UseCheckmarks() ? TYPE_CHECK : TYPE_COMMAND; } @@ -319,6 +145,8 @@ class Combobox::ComboboxMenuModel : public ui::MenuModel, } int GetCommandIdAt(int index) const override { + // Define the id of the first item in the menu (since it needs to be > 0) + constexpr int kFirstMenuItemId = 1000; return index + kFirstMenuItemId; } @@ -357,13 +185,6 @@ class Combobox::ComboboxMenuModel : public ui::MenuModel, return model_->IsItemEnabledAt(index); } - bool IsVisibleAt(int index) const override { - // When STYLE_ACTION is used, the first item is not added to the menu. It is - // assumed that the first item is always selected and rendered on the top of - // the action button. - return index > 0 || owner_->style_ != STYLE_ACTION; - } - void HighlightChangedTo(int index) override {} void ActivatedAt(int index) override { @@ -396,22 +217,19 @@ class Combobox::ComboboxMenuModel : public ui::MenuModel, //////////////////////////////////////////////////////////////////////////////// // Combobox, public: -Combobox::Combobox(std::unique_ptr<ui::ComboboxModel> model, Style style) - : Combobox(model.get(), style) { +Combobox::Combobox(std::unique_ptr<ui::ComboboxModel> model) + : Combobox(model.get()) { owned_model_ = std::move(model); } -Combobox::Combobox(ui::ComboboxModel* model, Style style) +Combobox::Combobox(ui::ComboboxModel* model) : model_(model), - style_(style), listener_(nullptr), - selected_index_(style == STYLE_ACTION ? 0 : model_->GetDefaultIndex()), + selected_index_(model_->GetDefaultIndex()), invalid_(false), menu_model_(new ComboboxMenuModel(this, model)), - text_button_(new TransparentButton(this, style_ == STYLE_ACTION)), - arrow_button_(new TransparentButton(this, style_ == STYLE_ACTION)), - size_to_largest_label_(style_ == STYLE_NORMAL), - weak_ptr_factory_(this) { + arrow_button_(new TransparentButton(this)), + size_to_largest_label_(true) { ModelChanged(); #if defined(OS_MACOSX) SetFocusBehavior(FocusBehavior::ACCESSIBLE_ONLY); @@ -421,29 +239,7 @@ Combobox::Combobox(ui::ComboboxModel* model, Style style) UpdateBorder(); - // Initialize the button images. - Button::ButtonState button_states[] = { - Button::STATE_DISABLED, - Button::STATE_NORMAL, - Button::STATE_HOVERED, - Button::STATE_PRESSED, - }; - for (int i = 0; i < 2; i++) { - for (size_t state_index = 0; state_index < arraysize(button_states); - state_index++) { - Button::ButtonState state = button_states[state_index]; - size_t num; - bool focused = !!i; - const int* ids = GetBodyButtonImageIds(focused, state, &num); - body_button_painters_[focused][state] = - Painter::CreateImageGridPainter(ids); - menu_button_images_[focused][state] = GetMenuButtonImages(focused, state); - } - } - - text_button_->SetVisible(true); arrow_button_->SetVisible(true); - AddChildView(text_button_); AddChildView(arrow_button_); // A layer is applied to make sure that canvas bounds are snapped to pixel @@ -463,8 +259,7 @@ Combobox::~Combobox() { // static const gfx::FontList& Combobox::GetFontList() { - ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance(); - return rb.GetFontListWithDelta(ui::kLabelFontSizeDelta); + return style::GetFont(style::CONTEXT_BUTTON, style::STYLE_PRIMARY); } void Combobox::ModelChanged() { @@ -482,9 +277,6 @@ void Combobox::ModelChanged() { } void Combobox::SetSelectedIndex(int index) { - if (style_ == STYLE_ACTION) - return; - selected_index_ = index; if (size_to_largest_label_) { SchedulePaint(); @@ -495,9 +287,6 @@ void Combobox::SetSelectedIndex(int index) { } bool Combobox::SelectValue(const base::string16& value) { - if (style_ == STYLE_ACTION) - return false; - for (int i = 0; i < model()->GetItemCount(); ++i) { if (value == model()->GetItemAt(i)) { SetSelectedIndex(i); @@ -532,25 +321,7 @@ void Combobox::SetInvalid(bool invalid) { void Combobox::Layout() { View::Layout(); - - int text_button_width = 0; - int arrow_button_width = 0; - - switch (style_) { - case STYLE_NORMAL: { - arrow_button_width = width(); - break; - } - case STYLE_ACTION: { - arrow_button_width = GetArrowContainerWidth(); - text_button_width = width() - arrow_button_width; - break; - } - } - - int arrow_button_x = std::max(0, text_button_width); - text_button_->SetBounds(0, 0, std::max(0, text_button_width), height()); - arrow_button_->SetBounds(arrow_button_x, 0, arrow_button_width, height()); + arrow_button_->SetBounds(0, 0, width(), height()); } void Combobox::OnNativeThemeChanged(const ui::NativeTheme* theme) { @@ -585,6 +356,9 @@ base::string16 Combobox::GetTextForRow(int row) { // Combobox, View overrides: gfx::Size Combobox::CalculatePreferredSize() const { + // Limit how small a combobox can be. + constexpr int kMinComboboxWidth = 25; + // The preferred size will drive the local bounds which in turn is used to set // the minimum width for the dropdown list. gfx::Insets insets = GetInsets(); @@ -664,21 +438,9 @@ bool Combobox::OnKeyPressed(const ui::KeyEvent& e) { new_index = GetAdjacentIndex(model(), -1, selected_index_); break; - case ui::VKEY_SPACE: - if (style_ == STYLE_ACTION) { - // When pressing space, the click event will be raised after the key is - // released. - text_button_->SetState(Button::STATE_PRESSED); - } else { - show_menu = true; - } - break; - case ui::VKEY_RETURN: - if (style_ == STYLE_ACTION) - OnPerformAction(); - else - show_menu = true; + case ui::VKEY_SPACE: + show_menu = true; break; #endif // OS_MACOSX default: @@ -687,8 +449,7 @@ bool Combobox::OnKeyPressed(const ui::KeyEvent& e) { if (show_menu) { ShowDropDownMenu(ui::MENU_SOURCE_KEYBOARD); - } else if (new_index != selected_index_ && new_index != kNoSelection && - style_ != STYLE_ACTION) { + } else if (new_index != selected_index_ && new_index != kNoSelection) { DCHECK(!model()->IsItemSeparatorAt(new_index)); selected_index_ = new_index; OnPerformAction(); @@ -697,31 +458,10 @@ bool Combobox::OnKeyPressed(const ui::KeyEvent& e) { return true; } -bool Combobox::OnKeyReleased(const ui::KeyEvent& e) { - if (style_ != STYLE_ACTION) - return false; // crbug.com/127520 - - if (e.key_code() == ui::VKEY_SPACE && style_ == STYLE_ACTION && - text_button_->state() == Button::STATE_PRESSED) - OnPerformAction(); - - return false; -} - void Combobox::OnPaint(gfx::Canvas* canvas) { - switch (style_) { - case STYLE_NORMAL: { - OnPaintBackground(canvas); - PaintText(canvas); - OnPaintBorder(canvas); - break; - } - case STYLE_ACTION: { - PaintButtons(canvas); - PaintText(canvas); - break; - } - } + OnPaintBackground(canvas); + PaintText(canvas); + OnPaintBorder(canvas); } void Combobox::OnFocus() { @@ -779,29 +519,22 @@ void Combobox::ButtonPressed(Button* sender, const ui::Event& event) { if (!enabled()) return; - if (sender == text_button_) { - OnPerformAction(); - } else { - DCHECK_EQ(arrow_button_, sender); - // TODO(hajimehoshi): Fix the problem that the arrow button blinks when - // cliking this while the dropdown menu is opened. - const base::TimeDelta delta = base::Time::Now() - closed_time_; - if (delta.InMilliseconds() <= kMinimumMsBetweenButtonClicks) - return; + // TODO(hajimehoshi): Fix the problem that the arrow button blinks when + // cliking this while the dropdown menu is opened. + const base::TimeDelta delta = base::Time::Now() - closed_time_; + if (delta.InMilliseconds() <= kMinimumMsBetweenButtonClicks) + return; - ui::MenuSourceType source_type = ui::MENU_SOURCE_MOUSE; - if (event.IsKeyEvent()) - source_type = ui::MENU_SOURCE_KEYBOARD; - else if (event.IsGestureEvent() || event.IsTouchEvent()) - source_type = ui::MENU_SOURCE_TOUCH; - ShowDropDownMenu(source_type); - } + ui::MenuSourceType source_type = ui::MENU_SOURCE_MOUSE; + if (event.IsKeyEvent()) + source_type = ui::MENU_SOURCE_KEYBOARD; + else if (event.IsGestureEvent() || event.IsTouchEvent()) + source_type = ui::MENU_SOURCE_TOUCH; + ShowDropDownMenu(source_type); } void Combobox::UpdateBorder() { std::unique_ptr<FocusableBorder> border(new FocusableBorder()); - if (style_ == STYLE_ACTION) - border->SetInsets(5, 10, 5, 10); if (invalid_) border->SetColorId(ui::NativeTheme::kColorId_AlertSeverityHigh); SetBorder(std::move(border)); @@ -842,8 +575,7 @@ void Combobox::PaintText(gfx::Canvas* canvas) { gfx::Rect arrow_bounds(disclosure_arrow_offset, 0, GetArrowContainerWidth(), height()); - arrow_bounds = - PositionArrowWithinContainer(arrow_bounds, ArrowSize(), style_); + arrow_bounds.ClampToCenteredSize(ArrowSize()); AdjustBoundsForRTLUI(&arrow_bounds); { @@ -874,82 +606,28 @@ void Combobox::PaintText(gfx::Canvas* canvas) { } } -void Combobox::PaintButtons(gfx::Canvas* canvas) { - DCHECK(style_ == STYLE_ACTION); - - gfx::ScopedRTLFlipCanvas scoped_canvas(canvas, width()); - - bool focused = HasFocus(); - const std::vector<const gfx::ImageSkia*>& arrow_button_images = - menu_button_images_[focused][ - arrow_button_->state() == Button::STATE_HOVERED ? - Button::STATE_NORMAL : arrow_button_->state()]; - - int text_button_hover_alpha = - text_button_->state() == Button::STATE_PRESSED ? 0 : - static_cast<int>(static_cast<TransparentButton*>(text_button_)-> - GetAnimationValue() * 255); - if (text_button_hover_alpha < 255) { - canvas->SaveLayerAlpha(255 - text_button_hover_alpha); - Painter* text_button_painter = - body_button_painters_[focused][ - text_button_->state() == Button::STATE_HOVERED ? - Button::STATE_NORMAL : text_button_->state()].get(); - Painter::PaintPainterAt(canvas, text_button_painter, - gfx::Rect(0, 0, text_button_->width(), height())); - canvas->Restore(); - } - if (0 < text_button_hover_alpha) { - canvas->SaveLayerAlpha(text_button_hover_alpha); - Painter* text_button_hovered_painter = - body_button_painters_[focused][Button::STATE_HOVERED].get(); - Painter::PaintPainterAt(canvas, text_button_hovered_painter, - gfx::Rect(0, 0, text_button_->width(), height())); - canvas->Restore(); - } - - int arrow_button_hover_alpha = - arrow_button_->state() == Button::STATE_PRESSED ? 0 : - static_cast<int>(static_cast<TransparentButton*>(arrow_button_)-> - GetAnimationValue() * 255); - if (arrow_button_hover_alpha < 255) { - canvas->SaveLayerAlpha(255 - arrow_button_hover_alpha); - PaintArrowButton(canvas, arrow_button_images, arrow_button_->x(), height()); - canvas->Restore(); - } - if (0 < arrow_button_hover_alpha) { - canvas->SaveLayerAlpha(arrow_button_hover_alpha); - const std::vector<const gfx::ImageSkia*>& arrow_button_hovered_images = - menu_button_images_[focused][Button::STATE_HOVERED]; - PaintArrowButton(canvas, arrow_button_hovered_images, - arrow_button_->x(), height()); - canvas->Restore(); - } -} - void Combobox::ShowDropDownMenu(ui::MenuSourceType source_type) { + // Menu border widths. + constexpr int kMenuBorderWidthLeft = 1; + constexpr int kMenuBorderWidthTop = 1; + constexpr int kMenuBorderWidthRight = 1; + gfx::Rect lb = GetLocalBounds(); gfx::Point menu_position(lb.origin()); - if (style_ == STYLE_NORMAL) { - // Inset the menu's requested position so the border of the menu lines up - // with the border of the combobox. - menu_position.set_x(menu_position.x() + kMenuBorderWidthLeft); - menu_position.set_y(menu_position.y() + kMenuBorderWidthTop); - } + // Inset the menu's requested position so the border of the menu lines up + // with the border of the combobox. + menu_position.set_x(menu_position.x() + kMenuBorderWidthLeft); + menu_position.set_y(menu_position.y() + kMenuBorderWidthTop); + lb.set_width(lb.width() - (kMenuBorderWidthLeft + kMenuBorderWidthRight)); View::ConvertPointToScreen(this, &menu_position); gfx::Rect bounds(menu_position, lb.size()); - Button::ButtonState original_state = Button::STATE_NORMAL; - if (arrow_button_) { - original_state = arrow_button_->state(); - arrow_button_->SetState(Button::STATE_PRESSED); - } - MenuAnchorPosition anchor_position = - style_ == STYLE_ACTION ? MENU_ANCHOR_TOPRIGHT : MENU_ANCHOR_TOPLEFT; + Button::ButtonState original_state = arrow_button_->state(); + arrow_button_->SetState(Button::STATE_PRESSED); // Allow |menu_runner_| to be set by the testing API, but if this method is // ever invoked recursively, ensure the old menu is closed. @@ -959,14 +637,13 @@ void Combobox::ShowDropDownMenu(ui::MenuSourceType source_type) { base::Bind(&Combobox::OnMenuClosed, base::Unretained(this), original_state))); } - menu_runner_->RunMenuAt(GetWidget(), nullptr, bounds, anchor_position, + menu_runner_->RunMenuAt(GetWidget(), nullptr, bounds, MENU_ANCHOR_TOPLEFT, source_type); } void Combobox::OnMenuClosed(Button::ButtonState original_button_state) { menu_runner_.reset(); - if (arrow_button_) - arrow_button_->SetState(original_button_state); + arrow_button_->SetState(original_button_state); closed_time_ = base::Time::Now(); } @@ -974,13 +651,10 @@ void Combobox::OnPerformAction() { NotifyAccessibilityEvent(ax::mojom::Event::kValueChanged, true); SchedulePaint(); - // This combobox may be deleted by the listener. - base::WeakPtr<Combobox> weak_ptr = weak_ptr_factory_.GetWeakPtr(); if (listener_) listener_->OnPerformAction(this); - if (weak_ptr && style_ == STYLE_ACTION) - selected_index_ = 0; + // Note |this| may be deleted by |listener_|. } gfx::Size Combobox::ArrowSize() const { @@ -1011,10 +685,7 @@ PrefixSelector* Combobox::GetPrefixSelector() { int Combobox::GetArrowContainerWidth() const { constexpr int kPaddingWidth = 8; - int padding = style_ == STYLE_NORMAL - ? kPaddingWidth * 2 - : kActionLeftPadding + kActionRightPadding; - return ArrowSize().width() + padding; + return ArrowSize().width() + kPaddingWidth * 2; } } // namespace views diff --git a/chromium/ui/views/controls/combobox/combobox.h b/chromium/ui/views/controls/combobox/combobox.h index d6132543edd..9f86b4619e5 100644 --- a/chromium/ui/views/controls/combobox/combobox.h +++ b/chromium/ui/views/controls/combobox/combobox.h @@ -6,12 +6,9 @@ #define UI_VIEWS_CONTROLS_COMBOBOX_COMBOBOX_H_ #include "base/macros.h" -#include "base/memory/weak_ptr.h" #include "base/strings/string16.h" #include "base/time/time.h" -#include "ui/base/models/combobox_model.h" #include "ui/views/controls/button/button.h" -#include "ui/views/controls/focus_ring.h" #include "ui/views/controls/prefix_delegate.h" namespace gfx { @@ -19,6 +16,7 @@ class FontList; } namespace ui { +class ComboboxModel; class MenuModel; } @@ -28,40 +26,23 @@ class ComboboxTestApi; } class ComboboxListener; -class Button; +class FocusRing; class MenuRunner; -class Painter; class PrefixSelector; // A non-editable combobox (aka a drop-down list or selector). -// Combobox has two distinct parts, the drop down arrow and the text. Combobox -// offers two distinct behaviors: -// * STYLE_NORMAL: typical combobox, clicking on the text and/or button shows -// the drop down, arrow keys change selection or show the menu depending on -// the platform, selected index can be changed by the user to something other -// than the first item. -// * STYLE_ACTION: clicking on the text notifies the listener. The menu can be -// shown only by clicking on the arrow, except on Mac where it can be shown -// through the keyboard. The selected index is always reverted to 0 after the -// listener is notified. +// Combobox has two distinct parts, the drop down arrow and the text. class VIEWS_EXPORT Combobox : public View, public PrefixDelegate, public ButtonListener { public: - // The style of the combobox. - enum Style { - STYLE_NORMAL, - STYLE_ACTION, - }; - // The combobox's class name. static const char kViewClassName[]; // |model| is owned by the combobox when using this constructor. - explicit Combobox(std::unique_ptr<ui::ComboboxModel> model, - Style style = STYLE_NORMAL); + explicit Combobox(std::unique_ptr<ui::ComboboxModel> model); // |model| is not owned by the combobox when using this constructor. - explicit Combobox(ui::ComboboxModel* model, Style style = STYLE_NORMAL); + explicit Combobox(ui::ComboboxModel* model); ~Combobox() override; static const gfx::FontList& GetFontList(); @@ -99,7 +80,6 @@ class VIEWS_EXPORT Combobox : public View, const char* GetClassName() const override; bool SkipDefaultKeyEventProcessing(const ui::KeyEvent& e) override; bool OnKeyPressed(const ui::KeyEvent& e) override; - bool OnKeyReleased(const ui::KeyEvent& e) override; void OnPaint(gfx::Canvas* canvas) override; void OnFocus() override; void OnBlur() override; @@ -136,9 +116,6 @@ class VIEWS_EXPORT Combobox : public View, // Draws the selected value of the drop down list void PaintText(gfx::Canvas* canvas); - // Draws the button images. - void PaintButtons(gfx::Canvas* canvas); - // Show the drop down list void ShowDropDownMenu(ui::MenuSourceType source_type); @@ -148,14 +125,10 @@ class VIEWS_EXPORT Combobox : public View, // Called when the selection is changed by the user. void OnPerformAction(); - int GetDisclosureArrowLeftPadding() const; - int GetDisclosureArrowRightPadding() const; - // Returns the size of the disclosure arrow. gfx::Size ArrowSize() const; - // Finds the size of the largest menu label or, for STYLE_ACTION, the size of - // the selected label. + // Finds the size of the largest menu label. gfx::Size GetContentSize() const; // Handles the clicking event. @@ -176,9 +149,6 @@ class VIEWS_EXPORT Combobox : public View, // Reference to our model, which may be owned or not. ui::ComboboxModel* model_; - // The visual style of this combobox. - const Style style_; - // Our listener. Not owned. Notified when the selected index change. ComboboxListener* listener_; @@ -208,29 +178,15 @@ class VIEWS_EXPORT Combobox : public View, // The maximum dimensions of the content in the dropdown. gfx::Size content_size_; - // The painters or images that are used when |style_| is STYLE_BUTTONS. The - // first index means the state of unfocused or focused. - // The images are owned by ResourceBundle. - std::unique_ptr<Painter> body_button_painters_[2][Button::STATE_COUNT]; - std::vector<const gfx::ImageSkia*> - menu_button_images_[2][Button::STATE_COUNT]; - - // The transparent buttons to handle events and render buttons. These are - // placed on top of this combobox as child views, accept event and manage the - // button states. These are not rendered but when |style_| is - // STYLE_NOTIFY_ON_CLICK, a Combobox renders the button images according to - // these button states. - // The base View takes the ownerships of these as child views. - Button* text_button_; + // A transparent button that handles events and holds button state. Placed on + // top of the combobox as a child view. Doesn't paint itself, but serves as a + // host for inkdrops. Button* arrow_button_; // Set while the dropdown is showing. Ensures the menu is closed if |this| is // destroyed. std::unique_ptr<MenuRunner> menu_runner_; - // The image to be drawn for this combobox's arrow. - gfx::ImageSkia arrow_image_; - // When true, the size of contents is defined by the selected label. // Otherwise, it's defined by the widest label in the menu. If this is set to // true, the parent view must relayout in ChildPreferredSizeChanged(). @@ -239,9 +195,6 @@ class VIEWS_EXPORT Combobox : public View, // The focus ring for this Combobox. std::unique_ptr<FocusRing> focus_ring_; - // Used for making calbacks. - base::WeakPtrFactory<Combobox> weak_ptr_factory_; - DISALLOW_COPY_AND_ASSIGN(Combobox); }; diff --git a/chromium/ui/views/controls/combobox/combobox_listener.h b/chromium/ui/views/controls/combobox/combobox_listener.h index 2522e03c7d8..71442cd1c4e 100644 --- a/chromium/ui/views/controls/combobox/combobox_listener.h +++ b/chromium/ui/views/controls/combobox/combobox_listener.h @@ -16,12 +16,11 @@ class Combobox; class VIEWS_EXPORT ComboboxListener { public: // Invoked when the user does the appropriate gesture that some action should - // be performed. For both STYLE_NORMAL and STYLE_ACTION this is invoked if the - // user clicks on the menu button and then clicks an item. For STYLE_NORMAL - // this is also invoked when the menu is not showing and the does a gesture to - // change the selection (for example, presses the home or end keys). This is - // not invoked when the menu is shown and the user changes the selection - // without closing the menu. + // be performed. This is invoked if the user clicks on the menu button and + // then clicks an item, and also when the menu is not showing and the does a + // gesture to change the selection (for example, presses the home or end + // keys). This is not invoked when the menu is shown and the user changes the + // selection without closing the menu. virtual void OnPerformAction(Combobox* combobox) = 0; protected: diff --git a/chromium/ui/views/controls/combobox/combobox_unittest.cc b/chromium/ui/views/controls/combobox/combobox_unittest.cc index 5c32debbaf9..f1c486f26d1 100644 --- a/chromium/ui/views/controls/combobox/combobox_unittest.cc +++ b/chromium/ui/views/controls/combobox/combobox_unittest.cc @@ -25,6 +25,7 @@ #include "ui/views/test/combobox_test_api.h" #include "ui/views/test/views_test_base.h" #include "ui/views/widget/widget.h" +#include "ui/views/widget/widget_utils.h" using base::ASCIIToUTF16; @@ -38,8 +39,8 @@ namespace { // OnKeyReleased() methods. class TestCombobox : public Combobox { public: - TestCombobox(ui::ComboboxModel* model, Combobox::Style style) - : Combobox(model, style), key_handled_(false), key_received_(false) {} + explicit TestCombobox(ui::ComboboxModel* model) + : Combobox(model), key_handled_(false), key_received_(false) {} bool OnKeyPressed(const ui::KeyEvent& e) override { key_received_ = true; @@ -196,14 +197,14 @@ class ComboboxTest : public ViewsTestBase { ViewsTestBase::TearDown(); } - void InitCombobox(const std::set<int>* separators, Combobox::Style style) { + void InitCombobox(const std::set<int>* separators) { model_.reset(new TestComboboxModel()); if (separators) model_->SetSeparators(*separators); ASSERT_FALSE(combobox_); - combobox_ = new TestCombobox(model_.get(), style); + combobox_ = new TestCombobox(model_.get()); test_api_.reset(new ComboboxTestApi(combobox_)); test_api_->InstallTestMenuRunner(&menu_show_count_); combobox_->set_id(1); @@ -222,7 +223,7 @@ class ComboboxTest : public ViewsTestBase { combobox_->SizeToPreferredSize(); event_generator_ = - std::make_unique<ui::test::EventGenerator>(widget_->GetNativeWindow()); + std::make_unique<ui::test::EventGenerator>(GetRootWindow(widget_)); event_generator_->set_target(ui::test::EventGenerator::Target::WINDOW); } @@ -282,7 +283,7 @@ class ComboboxTest : public ViewsTestBase { // Tests whether the various Mac specific keyboard shortcuts invoke the dropdown // menu or not. TEST_F(ComboboxTest, KeyTestMac) { - InitCombobox(nullptr, Combobox::STYLE_NORMAL); + InitCombobox(nullptr); PressKey(ui::VKEY_END); EXPECT_EQ(0, combobox_->selected_index()); EXPECT_EQ(1, menu_show_count_); @@ -331,7 +332,7 @@ TEST_F(ComboboxTest, DisabilityTest) { model_.reset(new TestComboboxModel()); ASSERT_FALSE(combobox_); - combobox_ = new TestCombobox(model_.get(), Combobox::STYLE_NORMAL); + combobox_ = new TestCombobox(model_.get()); combobox_->SetEnabled(false); widget_ = new Widget; @@ -352,7 +353,7 @@ TEST_F(ComboboxTest, DisabilityTest) { // Tests the behavior of various keyboard shortcuts on the currently selected // index. TEST_F(ComboboxTest, KeyTest) { - InitCombobox(nullptr, Combobox::STYLE_NORMAL); + InitCombobox(nullptr); PressKey(ui::VKEY_END); EXPECT_EQ(model_->GetItemCount(), combobox_->selected_index() + 1); PressKey(ui::VKEY_HOME); @@ -377,7 +378,7 @@ TEST_F(ComboboxTest, KeyTest) { TEST_F(ComboboxTest, SkipSeparatorSimple) { std::set<int> separators; separators.insert(2); - InitCombobox(&separators, Combobox::STYLE_NORMAL); + InitCombobox(&separators); EXPECT_EQ(0, combobox_->selected_index()); PressKey(ui::VKEY_DOWN); EXPECT_EQ(1, combobox_->selected_index()); @@ -398,7 +399,7 @@ TEST_F(ComboboxTest, SkipSeparatorSimple) { TEST_F(ComboboxTest, SkipSeparatorBeginning) { std::set<int> separators; separators.insert(0); - InitCombobox(&separators, Combobox::STYLE_NORMAL); + InitCombobox(&separators); EXPECT_EQ(1, combobox_->selected_index()); PressKey(ui::VKEY_DOWN); EXPECT_EQ(2, combobox_->selected_index()); @@ -419,7 +420,7 @@ TEST_F(ComboboxTest, SkipSeparatorBeginning) { TEST_F(ComboboxTest, SkipSeparatorEnd) { std::set<int> separators; separators.insert(TestComboboxModel::kItemCount - 1); - InitCombobox(&separators, Combobox::STYLE_NORMAL); + InitCombobox(&separators); combobox_->SetSelectedIndex(8); PressKey(ui::VKEY_DOWN); EXPECT_EQ(8, combobox_->selected_index()); @@ -437,7 +438,7 @@ TEST_F(ComboboxTest, SkipMultipleSeparatorsAtBeginning) { separators.insert(0); separators.insert(1); separators.insert(2); - InitCombobox(&separators, Combobox::STYLE_NORMAL); + InitCombobox(&separators); EXPECT_EQ(3, combobox_->selected_index()); PressKey(ui::VKEY_DOWN); EXPECT_EQ(4, combobox_->selected_index()); @@ -461,7 +462,7 @@ TEST_F(ComboboxTest, SkipMultipleAdjacentSeparatorsAtMiddle) { separators.insert(4); separators.insert(5); separators.insert(6); - InitCombobox(&separators, Combobox::STYLE_NORMAL); + InitCombobox(&separators); combobox_->SetSelectedIndex(3); PressKey(ui::VKEY_DOWN); EXPECT_EQ(7, combobox_->selected_index()); @@ -477,7 +478,7 @@ TEST_F(ComboboxTest, SkipMultipleSeparatorsAtEnd) { separators.insert(7); separators.insert(8); separators.insert(9); - InitCombobox(&separators, Combobox::STYLE_NORMAL); + InitCombobox(&separators); combobox_->SetSelectedIndex(6); PressKey(ui::VKEY_DOWN); EXPECT_EQ(6, combobox_->selected_index()); @@ -499,7 +500,7 @@ TEST_F(ComboboxTest, GetTextForRowTest) { separators.insert(0); separators.insert(1); separators.insert(9); - InitCombobox(&separators, Combobox::STYLE_NORMAL); + InitCombobox(&separators); for (int i = 0; i < combobox_->GetRowCount(); ++i) { if (separators.count(i) != 0) { EXPECT_TRUE(combobox_->GetTextForRow(i).empty()) << i; @@ -512,7 +513,7 @@ TEST_F(ComboboxTest, GetTextForRowTest) { // Verifies selecting the first matching value (and returning whether found). TEST_F(ComboboxTest, SelectValue) { - InitCombobox(nullptr, Combobox::STYLE_NORMAL); + InitCombobox(nullptr); ASSERT_EQ(model_->GetDefaultIndex(), combobox_->selected_index()); EXPECT_TRUE(combobox_->SelectValue(ASCIIToUTF16("PEANUT BUTTER"))); EXPECT_EQ(0, combobox_->selected_index()); @@ -522,50 +523,19 @@ TEST_F(ComboboxTest, SelectValue) { EXPECT_EQ(1, combobox_->selected_index()); } -TEST_F(ComboboxTest, SelectValueActionStyle) { - // With the action style, the selected index is always 0. - InitCombobox(nullptr, Combobox::STYLE_ACTION); - EXPECT_FALSE(combobox_->SelectValue(ASCIIToUTF16("PEANUT BUTTER"))); - EXPECT_EQ(0, combobox_->selected_index()); - EXPECT_FALSE(combobox_->SelectValue(ASCIIToUTF16("JELLY"))); - EXPECT_EQ(0, combobox_->selected_index()); - EXPECT_FALSE(combobox_->SelectValue(ASCIIToUTF16("BANANAS"))); - EXPECT_EQ(0, combobox_->selected_index()); -} - -TEST_F(ComboboxTest, SelectIndexActionStyle) { - InitCombobox(nullptr, Combobox::STYLE_ACTION); - - // With the action style, the selected index is always 0. - combobox_->SetSelectedIndex(1); - EXPECT_EQ(0, combobox_->selected_index()); - combobox_->SetSelectedIndex(2); - EXPECT_EQ(0, combobox_->selected_index()); - combobox_->SetSelectedIndex(3); - EXPECT_EQ(0, combobox_->selected_index()); -} - TEST_F(ComboboxTest, ListenerHandlesDelete) { TestComboboxModel model; // |combobox| will be deleted on change. - TestCombobox* combobox = new TestCombobox(&model, Combobox::STYLE_NORMAL); + TestCombobox* combobox = new TestCombobox(&model); std::unique_ptr<EvilListener> evil_listener(new EvilListener()); combobox->set_listener(evil_listener.get()); ASSERT_NO_FATAL_FAILURE(ComboboxTestApi(combobox).PerformActionAt(2)); EXPECT_TRUE(evil_listener->deleted()); - - // With STYLE_ACTION - // |combobox| will be deleted on change. - combobox = new TestCombobox(&model, Combobox::STYLE_ACTION); - evil_listener.reset(new EvilListener()); - combobox->set_listener(evil_listener.get()); - ASSERT_NO_FATAL_FAILURE(ComboboxTestApi(combobox).PerformActionAt(2)); - EXPECT_TRUE(evil_listener->deleted()); } TEST_F(ComboboxTest, Click) { - InitCombobox(nullptr, Combobox::STYLE_NORMAL); + InitCombobox(nullptr); TestComboboxListener listener; combobox_->set_listener(&listener); @@ -580,7 +550,7 @@ TEST_F(ComboboxTest, Click) { } TEST_F(ComboboxTest, ClickButDisabled) { - InitCombobox(nullptr, Combobox::STYLE_NORMAL); + InitCombobox(nullptr); TestComboboxListener listener; combobox_->set_listener(&listener); @@ -596,44 +566,25 @@ TEST_F(ComboboxTest, ClickButDisabled) { } TEST_F(ComboboxTest, NotifyOnClickWithReturnKey) { - InitCombobox(nullptr, Combobox::STYLE_NORMAL); + InitCombobox(nullptr); TestComboboxListener listener; combobox_->set_listener(&listener); - // With STYLE_NORMAL, the click event is ignored. Instead the menu is shown. + // The click event is ignored. Instead the menu is shown. PressKey(ui::VKEY_RETURN); EXPECT_EQ(PlatformStyle::kReturnClicksFocusedControl ? 1 : 0, menu_show_count_); EXPECT_FALSE(listener.on_perform_action_called()); } -TEST_F(ComboboxTest, NotifyOnClickWithReturnKeyActionStyle) { - InitCombobox(nullptr, Combobox::STYLE_ACTION); - - TestComboboxListener listener; - combobox_->set_listener(&listener); - - // With STYLE_ACTION, the click event is notified and the menu is not shown. - PressKey(ui::VKEY_RETURN); - EXPECT_EQ(0, menu_show_count_); - - if (PlatformStyle::kReturnClicksFocusedControl) { - EXPECT_TRUE(listener.on_perform_action_called()); - EXPECT_EQ(0, listener.perform_action_index()); - } else { - EXPECT_FALSE(listener.on_perform_action_called()); - EXPECT_EQ(-1, listener.perform_action_index()); - } -} - TEST_F(ComboboxTest, NotifyOnClickWithSpaceKey) { - InitCombobox(nullptr, Combobox::STYLE_NORMAL); + InitCombobox(nullptr); TestComboboxListener listener; combobox_->set_listener(&listener); - // With STYLE_NORMAL, the click event is ignored. Instead the menu is shwon. + // The click event is ignored. Instead the menu is shwon. PressKey(ui::VKEY_SPACE); EXPECT_EQ(1, menu_show_count_); EXPECT_FALSE(listener.on_perform_action_called()); @@ -645,7 +596,7 @@ TEST_F(ComboboxTest, NotifyOnClickWithSpaceKey) { // Test that accessibility action events show the combobox dropdown. TEST_F(ComboboxTest, ShowViaAccessibleAction) { - InitCombobox(nullptr, Combobox::STYLE_NORMAL); + InitCombobox(nullptr); ui::AXActionData data; data.action = ax::mojom::Action::kDoDefault; @@ -673,37 +624,8 @@ TEST_F(ComboboxTest, ShowViaAccessibleAction) { EXPECT_EQ(1, menu_show_count_); // No change. } -TEST_F(ComboboxTest, NotifyOnClickWithSpaceKeyActionStyle) { - InitCombobox(nullptr, Combobox::STYLE_ACTION); - - TestComboboxListener listener; - combobox_->set_listener(&listener); - - // With STYLE_ACTION, the click event is notified after releasing and the menu - // is not shown. On Mac, the menu should be shown. - PressKey(ui::VKEY_SPACE); -#if defined(OS_MACOSX) - EXPECT_EQ(1, menu_show_count_); -#else - EXPECT_EQ(0, menu_show_count_); -#endif - EXPECT_FALSE(listener.on_perform_action_called()); - EXPECT_EQ(-1, listener.perform_action_index()); - - ReleaseKey(ui::VKEY_SPACE); -#if defined(OS_MACOSX) - EXPECT_EQ(1, menu_show_count_); - EXPECT_FALSE(listener.on_perform_action_called()); - EXPECT_EQ(-1, listener.perform_action_index()); -#else - EXPECT_EQ(0, menu_show_count_); - EXPECT_TRUE(listener.on_perform_action_called()); - EXPECT_EQ(0, listener.perform_action_index()); -#endif -} - TEST_F(ComboboxTest, NotifyOnClickWithMouse) { - InitCombobox(nullptr, Combobox::STYLE_ACTION); + InitCombobox(nullptr); TestComboboxListener listener; combobox_->set_listener(&listener); @@ -735,12 +657,13 @@ TEST_F(ComboboxTest, NotifyOnClickWithMouse) { PerformMousePress(left_point); PerformMouseRelease(left_point); - EXPECT_EQ(1, menu_show_count_); // Unchanged. - EXPECT_EQ(0, listener.perform_action_index()); + // Both the text and the arrow may toggle the menu. + EXPECT_EQ(2, menu_show_count_); + EXPECT_EQ(-1, listener.perform_action_index()); // Nothing selected. } TEST_F(ComboboxTest, ConsumingPressKeyEvents) { - InitCombobox(nullptr, Combobox::STYLE_NORMAL); + InitCombobox(nullptr); EXPECT_TRUE(combobox_->OnKeyPressed( ui::KeyEvent(ui::ET_KEY_PRESSED, ui::VKEY_SPACE, ui::EF_NONE))); @@ -756,33 +679,11 @@ TEST_F(ComboboxTest, ConsumingPressKeyEvents) { } } -TEST_F(ComboboxTest, ConsumingKeyPressEventsActionStyle) { - // When the combobox's style is STYLE_ACTION, pressing events of a space key - // or an enter key will be consumed and the menu is not shown. However, on - // Mac, space will show the menu. - InitCombobox(nullptr, Combobox::STYLE_ACTION); - - EXPECT_EQ(PlatformStyle::kReturnClicksFocusedControl, - combobox_->OnKeyPressed(ui::KeyEvent( - ui::ET_KEY_PRESSED, ui::VKEY_RETURN, ui::EF_NONE))); - EXPECT_EQ(0, menu_show_count_); - - EXPECT_TRUE(combobox_->OnKeyPressed( - ui::KeyEvent(ui::ET_KEY_PRESSED, ui::VKEY_SPACE, ui::EF_NONE))); -#if defined(OS_MACOSX) - EXPECT_EQ(1, menu_show_count_); -#else - EXPECT_EQ(0, menu_show_count_); -#endif -} - TEST_F(ComboboxTest, ContentWidth) { std::vector<std::string> values; VectorComboboxModel model(&values); - TestCombobox combobox(&model, Combobox::STYLE_NORMAL); - TestCombobox action_combobox(&model, Combobox::STYLE_ACTION); + TestCombobox combobox(&model); ComboboxTestApi test_api(&combobox); - ComboboxTestApi action_test_api(&action_combobox); std::string long_item = "this is the long item"; std::string short_item = "s"; @@ -790,13 +691,11 @@ TEST_F(ComboboxTest, ContentWidth) { values.resize(1); values[0] = long_item; combobox.ModelChanged(); - action_combobox.ModelChanged(); const int long_item_width = test_api.content_size().width(); values[0] = short_item; combobox.ModelChanged(); - action_combobox.ModelChanged(); const int short_item_width = test_api.content_size().width(); @@ -804,20 +703,16 @@ TEST_F(ComboboxTest, ContentWidth) { values[0] = short_item; values[1] = long_item; combobox.ModelChanged(); - action_combobox.ModelChanged(); - // When the style is STYLE_NORMAL, the width will fit with the longest item. + // The width will fit with the longest item. EXPECT_EQ(long_item_width, test_api.content_size().width()); - - // When the style is STYLE_ACTION, the width will fit with the selected item's - // width. - EXPECT_EQ(short_item_width, action_test_api.content_size().width()); + EXPECT_LT(short_item_width, test_api.content_size().width()); } // Test that model updates preserve the selected index, so long as it is in // range. TEST_F(ComboboxTest, ModelChanged) { - InitCombobox(nullptr, Combobox::STYLE_NORMAL); + InitCombobox(nullptr); EXPECT_EQ(0, combobox_->GetSelectedRow()); EXPECT_EQ(10, combobox_->GetRowCount()); @@ -859,7 +754,7 @@ TEST_F(ComboboxTest, ModelChanged) { } TEST_F(ComboboxTest, TypingPrefixNotifiesListener) { - InitCombobox(nullptr, Combobox::STYLE_NORMAL); + InitCombobox(nullptr); TestComboboxListener listener; combobox_->set_listener(&listener); @@ -902,7 +797,7 @@ TEST_F(ComboboxTest, MenuModel) { const int kSeparatorIndex = 3; std::set<int> separators; separators.insert(kSeparatorIndex); - InitCombobox(&separators, Combobox::STYLE_NORMAL); + InitCombobox(&separators); ui::MenuModel* menu_model = test_api_->menu_model(); @@ -931,26 +826,4 @@ TEST_F(ComboboxTest, MenuModel) { EXPECT_TRUE(menu_model->IsVisibleAt(0)); } -// Check that with STYLE_ACTION, the first item (only) is not shown. -TEST_F(ComboboxTest, MenuModelActionStyleMac) { - const int kSeparatorIndex = 3; - std::set<int> separators; - separators.insert(kSeparatorIndex); - InitCombobox(&separators, Combobox::STYLE_ACTION); - - ui::MenuModel* menu_model = test_api_->menu_model(); - - EXPECT_EQ(TestComboboxModel::kItemCount, menu_model->GetItemCount()); - EXPECT_EQ(ui::MenuModel::TYPE_SEPARATOR, - menu_model->GetTypeAt(kSeparatorIndex)); - - EXPECT_EQ(ui::MenuModel::TYPE_COMMAND, menu_model->GetTypeAt(0)); - EXPECT_EQ(ui::MenuModel::TYPE_COMMAND, menu_model->GetTypeAt(1)); - - EXPECT_EQ(ASCIIToUTF16("PEANUT BUTTER"), menu_model->GetLabelAt(0)); - EXPECT_EQ(ASCIIToUTF16("JELLY"), menu_model->GetLabelAt(1)); - EXPECT_FALSE(menu_model->IsVisibleAt(0)); - EXPECT_TRUE(menu_model->IsVisibleAt(1)); -} - } // namespace views diff --git a/chromium/ui/views/controls/focus_ring.cc b/chromium/ui/views/controls/focus_ring.cc index bc9597596ce..f5fbb33ec88 100644 --- a/chromium/ui/views/controls/focus_ring.cc +++ b/chromium/ui/views/controls/focus_ring.cc @@ -81,7 +81,7 @@ void FocusRing::OnPaint(gfx::Canvas* canvas) { SkPath path = path_; if (path.isEmpty()) { - gfx::Path* highlight_path = parent()->GetProperty(kHighlightPathKey); + SkPath* highlight_path = parent()->GetProperty(kHighlightPathKey); if (highlight_path) path = *highlight_path; } diff --git a/chromium/ui/views/controls/image_view_base.cc b/chromium/ui/views/controls/image_view_base.cc index d898798239f..9c15144910c 100644 --- a/chromium/ui/views/controls/image_view_base.cc +++ b/chromium/ui/views/controls/image_view_base.cc @@ -32,7 +32,7 @@ void ImageViewBase::ResetImageSize() { void ImageViewBase::GetAccessibleNodeData(ui::AXNodeData* node_data) { node_data->role = ax::mojom::Role::kImage; - node_data->SetName(accessible_name_); + node_data->SetName(GetAccessibleName()); } void ImageViewBase::SetHorizontalAlignment(Alignment alignment) { @@ -63,19 +63,8 @@ void ImageViewBase::SetAccessibleName(const base::string16& accessible_name) { accessible_name_ = accessible_name; } -base::string16 ImageViewBase::GetAccessibleName() const { - return accessible_name_; -} - -// TODO(crbug.com/890465): Update the duplicate code here and in views::Button. -void ImageViewBase::SetTooltipText(const base::string16& tooltip) { - tooltip_text_ = tooltip; - if (accessible_name_.empty()) - accessible_name_ = tooltip_text_; -} - -base::string16 ImageViewBase::GetTooltipText() const { - return tooltip_text_; +const base::string16& ImageViewBase::GetAccessibleName() const { + return accessible_name_.empty() ? tooltip_text_ : accessible_name_; } bool ImageViewBase::GetTooltipText(const gfx::Point& p, @@ -83,7 +72,7 @@ bool ImageViewBase::GetTooltipText(const gfx::Point& p, if (tooltip_text_.empty()) return false; - *tooltip = GetTooltipText(); + *tooltip = tooltip_text(); return true; } diff --git a/chromium/ui/views/controls/image_view_base.h b/chromium/ui/views/controls/image_view_base.h index 4764f2f4c47..8046584de4a 100644 --- a/chromium/ui/views/controls/image_view_base.h +++ b/chromium/ui/views/controls/image_view_base.h @@ -40,12 +40,14 @@ class VIEWS_EXPORT ImageViewBase : public View { Alignment GetVerticalAlignment() const; // Set / Get the tooltip text. - void SetTooltipText(const base::string16& tooltip); - base::string16 GetTooltipText() const; + void set_tooltip_text(const base::string16& tooltip) { + tooltip_text_ = tooltip; + } + const base::string16& tooltip_text() const { return tooltip_text_; } // Set / Get the accessible name text. void SetAccessibleName(const base::string16& name); - base::string16 GetAccessibleName() const; + const base::string16& GetAccessibleName() const; // Overridden from View: void OnPaint(gfx::Canvas* canvas) override = 0; diff --git a/chromium/ui/views/controls/label_unittest.cc b/chromium/ui/views/controls/label_unittest.cc index 1ee2b444c42..aec4a0a0d3d 100644 --- a/chromium/ui/views/controls/label_unittest.cc +++ b/chromium/ui/views/controls/label_unittest.cc @@ -27,6 +27,7 @@ #include "ui/views/test/focus_manager_test.h" #include "ui/views/test/views_test_base.h" #include "ui/views/widget/widget.h" +#include "ui/views/widget/widget_utils.h" using base::ASCIIToUTF16; using base::WideToUTF16; @@ -164,7 +165,7 @@ class LabelSelectionTest : public LabelTest { void SetUp() override { LabelTest::SetUp(); event_generator_ = - std::make_unique<ui::test::EventGenerator>(widget()->GetNativeWindow()); + std::make_unique<ui::test::EventGenerator>(GetRootWindow(widget())); } protected: diff --git a/chromium/ui/views/controls/menu/menu_closure_animation_mac.mm b/chromium/ui/views/controls/menu/menu_closure_animation_mac.mm index 32127db42d9..fabb1a34680 100644 --- a/chromium/ui/views/controls/menu/menu_closure_animation_mac.mm +++ b/chromium/ui/views/controls/menu/menu_closure_animation_mac.mm @@ -86,7 +86,7 @@ void MenuClosureAnimationMac::DisableAnimationsForTesting() { void MenuClosureAnimationMac::AnimationProgressed( const gfx::Animation* animation) { - NSWindow* window = menu_->GetWidget()->GetNativeWindow(); + NSWindow* window = menu_->GetWidget()->GetNativeWindow().GetNativeNSWindow(); [window setAlphaValue:animation->CurrentValueBetween(1.0, 0.0)]; } diff --git a/chromium/ui/views/controls/menu/menu_controller.cc b/chromium/ui/views/controls/menu/menu_controller.cc index 9e56ca54813..237a19181f2 100644 --- a/chromium/ui/views/controls/menu/menu_controller.cc +++ b/chromium/ui/views/controls/menu/menu_controller.cc @@ -480,8 +480,8 @@ void MenuController::Run(Widget* parent, SetSelection(root, SELECTION_OPEN_SUBMENU | SELECTION_UPDATE_IMMEDIATELY); if (button) { - pressed_lock_ = std::make_unique<MenuButton::PressedLock>( - button, false, ui::LocatedEvent::FromIfValid(event)); + pressed_lock_ = button->menu_button_event_handler()->TakeLock( + false, ui::LocatedEvent::FromIfValid(event)); } if (for_drop_) { @@ -1118,7 +1118,23 @@ ui::PostDispatchAction MenuController::OnWillDispatchKeyEvent( if (event->type() == ui::ET_KEY_PRESSED) { base::WeakPtr<MenuController> this_ref = AsWeakPtr(); +#if defined(OS_MACOSX) + // Special handling for Option-Up and Option-Down, which should behave like + // Home and End respectively in menus. + if ((event->flags() & ui::EF_ALT_DOWN)) { + if (event->key_code() == ui::VKEY_UP) { + OnKeyDown(ui::VKEY_HOME); + } else if (event->key_code() == ui::VKEY_DOWN) { + OnKeyDown(ui::VKEY_END); + } else { + OnKeyDown(event->key_code()); + } + } else { + OnKeyDown(event->key_code()); + } +#else OnKeyDown(event->key_code()); +#endif // Key events can lead to this being deleted. if (!this_ref) return ui::POST_DISPATCH_NONE; @@ -1348,6 +1364,14 @@ void MenuController::OnKeyDown(ui::KeyboardCode key_code) { return; switch (key_code) { + case ui::VKEY_HOME: + MoveSelectionToFirstOrLastItem(INCREMENT_SELECTION_DOWN); + break; + + case ui::VKEY_END: + MoveSelectionToFirstOrLastItem(INCREMENT_SELECTION_UP); + break; + case ui::VKEY_UP: IncrementSelection(INCREMENT_SELECTION_UP); break; @@ -1601,7 +1625,7 @@ bool MenuController::ShowSiblingMenu(SubmenuView* source, // There is a sibling menu, update the button state, hide the current menu // and show the new one. - pressed_lock_.reset(new MenuButton::PressedLock(button, true, nullptr)); + pressed_lock_ = button->menu_button_event_handler()->TakeLock(true, nullptr); // Need to reset capture when we show the menu again, otherwise we aren't // going to get any events. @@ -2155,7 +2179,7 @@ gfx::Rect MenuController::CalculateMenuBounds(MenuItemView* item, // Menu fits above anchor bounds. menu_bounds.set_y(above_anchor); item->set_actual_menu_position(MenuItemView::POSITION_ABOVE_BOUNDS); - } else { + } else if (item->GetDelegate()->ShouldTryPositioningBesideAnchor()) { const int left_of_anchor = anchor_bounds.x() - menu_bounds.width(); const int right_of_anchor = anchor_bounds.right(); @@ -2173,6 +2197,11 @@ gfx::Rect MenuController::CalculateMenuBounds(MenuItemView* item, if (menu_bounds.x() < monitor_bounds.x()) menu_bounds.set_x(right_of_anchor); } + } else { + // The delegate doesn't want the menu repositioned to the side, and it + // doesn't fit on the screen in any orientation - just clip the menu to + // the screen and let the scrolling arrows appear. + menu_bounds.Intersect(monitor_bounds); } } @@ -2412,6 +2441,27 @@ void MenuController::IncrementSelection( } } +void MenuController::MoveSelectionToFirstOrLastItem( + SelectionIncrementDirectionType direction) { + MenuItemView* item = pending_state_.item; + DCHECK(item); + MenuItemView* submenu = nullptr; + + if (pending_state_.submenu_open && item->SubmenuIsShowing()) { + if (!item->GetSubmenu()->GetMenuItemCount()) + return; + + // A menu is selected and open, but none of its children are selected, + // select the first or last menu item that is visible and enabled. + submenu = item; + } else { + submenu = item->GetParentMenuItem(); + } + + MenuItemView* to_select = FindInitialSelectableMenuItem(submenu, direction); + SetInitialHotTrackedView(to_select, direction); +} + MenuItemView* MenuController::FindInitialSelectableMenuItem( MenuItemView* parent, SelectionIncrementDirectionType direction) { @@ -2788,7 +2838,7 @@ MenuItemView* MenuController::ExitTopMostMenu() { } #endif - std::unique_ptr<MenuButton::PressedLock> nested_pressed_lock; + std::unique_ptr<MenuButtonEventHandler::PressedLock> nested_pressed_lock; bool nested_menu = !menu_stack_.empty(); if (nested_menu) { DCHECK(!menu_stack_.empty()); diff --git a/chromium/ui/views/controls/menu/menu_controller.h b/chromium/ui/views/controls/menu/menu_controller.h index 588612b1edc..94538e47b29 100644 --- a/chromium/ui/views/controls/menu/menu_controller.h +++ b/chromium/ui/views/controls/menu/menu_controller.h @@ -22,6 +22,7 @@ #include "ui/events/event_constants.h" #include "ui/events/platform/platform_event_dispatcher.h" #include "ui/views/controls/button/menu_button.h" +#include "ui/views/controls/button/menu_button_event_handler.h" #include "ui/views/controls/menu/menu_config.h" #include "ui/views/controls/menu/menu_delegate.h" #include "ui/views/widget/widget_observer.h" @@ -486,6 +487,10 @@ class VIEWS_EXPORT MenuController // Selects the next or previous (depending on |direction|) menu item. void IncrementSelection(SelectionIncrementDirectionType direction); + // Selects the first or last (depending on |direction|) menu item. + void MoveSelectionToFirstOrLastItem( + SelectionIncrementDirectionType direction); + // Returns the first (|direction| == NAVIGATE_SELECTION_DOWN) or the last // (|direction| == INCREMENT_SELECTION_UP) selectable child menu item of // |parent|. If there are no selectable items returns NULL. @@ -623,7 +628,7 @@ class VIEWS_EXPORT MenuController // Run, the current state (state_) is pushed onto menu_stack_. This allows // MenuController to restore the state when the nested run returns. using NestedState = - std::pair<State, std::unique_ptr<MenuButton::PressedLock>>; + std::pair<State, std::unique_ptr<MenuButtonEventHandler::PressedLock>>; std::list<NestedState> menu_stack_; // When Run is invoked during an active Run, it may be called from a separate @@ -677,7 +682,7 @@ class VIEWS_EXPORT MenuController std::unique_ptr<MenuScrollTask> scroll_task_; // The lock to keep the menu button pressed while a menu is visible. - std::unique_ptr<MenuButton::PressedLock> pressed_lock_; + std::unique_ptr<MenuButtonEventHandler::PressedLock> pressed_lock_; // ViewTracker used to store the View mouse drag events are forwarded to. See // UpdateActiveMouseView() for details. diff --git a/chromium/ui/views/controls/menu/menu_controller_unittest.cc b/chromium/ui/views/controls/menu/menu_controller_unittest.cc index ad8f6732a80..8cce8de8e18 100644 --- a/chromium/ui/views/controls/menu/menu_controller_unittest.cc +++ b/chromium/ui/views/controls/menu/menu_controller_unittest.cc @@ -30,6 +30,7 @@ #include "ui/views/test/test_views_delegate.h" #include "ui/views/test/views_test_base.h" #include "ui/views/widget/root_view.h" +#include "ui/views/widget/widget_utils.h" #if defined(USE_AURA) #include "ui/aura/client/drag_drop_client.h" @@ -45,6 +46,10 @@ #include "ui/gfx/x/x11.h" #endif +#if defined(OS_CHROMEOS) +#include "ui/base/ui_base_features.h" +#endif + namespace views { namespace test { @@ -324,7 +329,7 @@ class MenuControllerTest : public ViewsTestBase { set_views_delegate(std::move(views_delegate)); ViewsTestBase::SetUp(); Init(); - ASSERT_TRUE(base::MessageLoopForUI::IsCurrent()); + ASSERT_TRUE(base::MessageLoopCurrentForUI::IsSet()); } void TearDown() override { @@ -604,8 +609,8 @@ class MenuControllerTest : public ViewsTestBase { Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP); params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET; owner_->Init(params); - event_generator_.reset( - new ui::test::EventGenerator(owner_->GetNativeWindow())); + event_generator_ = + std::make_unique<ui::test::EventGenerator>(GetRootWindow(owner())); owner_->Show(); SetupMenuItem(); @@ -654,8 +659,7 @@ TEST_F(MenuControllerTest, EventTargeter) { // With the aura::NullWindowTargeter instantiated and assigned we expect // the menu to not handle the key event. aura::ScopedWindowTargeter scoped_targeter( - owner()->GetNativeWindow()->GetRootWindow(), - std::make_unique<aura::NullWindowTargeter>()); + GetRootWindow(owner()), std::make_unique<aura::NullWindowTargeter>()); PressKey(ui::VKEY_ESCAPE); EXPECT_EQ(MenuController::EXIT_NONE, menu_exit_type()); } @@ -672,8 +676,7 @@ TEST_F(MenuControllerTest, EventTargeter) { // details. When the ids aren't managed correctly, we get stuck down touches. TEST_F(MenuControllerTest, TouchIdsReleasedCorrectly) { TestEventHandler test_event_handler; - owner()->GetNativeWindow()->GetRootWindow()->AddPreTargetHandler( - &test_event_handler); + GetRootWindow(owner())->AddPreTargetHandler(&test_event_handler); std::vector<int> devices; devices.push_back(1); @@ -692,8 +695,7 @@ TEST_F(MenuControllerTest, TouchIdsReleasedCorrectly) { EXPECT_EQ(MenuController::EXIT_ALL, menu_exit_type()); EXPECT_EQ(0, test_event_handler.outstanding_touches()); - owner()->GetNativeWindow()->GetRootWindow()->RemovePreTargetHandler( - &test_event_handler); + GetRootWindow(owner())->RemovePreTargetHandler(&test_event_handler); } #endif // defined(USE_X11) @@ -777,6 +779,66 @@ TEST_F(MenuControllerTest, InitialSelectedItem) { ResetSelection(); } +// Tests that opening the menu and pressing 'Home' selects the first menu item. +TEST_F(MenuControllerTest, FirstSelectedItem) { + SetPendingStateItem(menu_item()->GetSubmenu()->GetMenuItemAt(0)); + EXPECT_EQ(1, pending_state_item()->GetCommand()); + + // Select the first menu item. + DispatchKey(ui::VKEY_HOME); + EXPECT_EQ(1, pending_state_item()->GetCommand()); + + // Fake initial root item selection and submenu showing. + SetPendingStateItem(menu_item()); + EXPECT_EQ(0, pending_state_item()->GetCommand()); + + // Select the first menu item. + DispatchKey(ui::VKEY_HOME); + EXPECT_EQ(1, pending_state_item()->GetCommand()); + + // Select the last item. + SetPendingStateItem(menu_item()->GetSubmenu()->GetMenuItemAt(3)); + EXPECT_EQ(4, pending_state_item()->GetCommand()); + + // Select the first menu item. + DispatchKey(ui::VKEY_HOME); + EXPECT_EQ(1, pending_state_item()->GetCommand()); + + // Clear references in menu controller to the menu item that is going away. + ResetSelection(); +} + +// Tests that opening the menu and pressing 'End' selects the last enabled menu +// item. +TEST_F(MenuControllerTest, LastSelectedItem) { + // Fake initial root item selection and submenu showing. + SetPendingStateItem(menu_item()); + EXPECT_EQ(0, pending_state_item()->GetCommand()); + + // Select the last menu item. + DispatchKey(ui::VKEY_END); + EXPECT_EQ(4, pending_state_item()->GetCommand()); + + // Select the last item. + SetPendingStateItem(menu_item()->GetSubmenu()->GetMenuItemAt(3)); + EXPECT_EQ(4, pending_state_item()->GetCommand()); + + // Select the last menu item. + DispatchKey(ui::VKEY_END); + EXPECT_EQ(4, pending_state_item()->GetCommand()); + + // Select the first item. + SetPendingStateItem(menu_item()->GetSubmenu()->GetMenuItemAt(0)); + EXPECT_EQ(1, pending_state_item()->GetCommand()); + + // Select the last menu item. + DispatchKey(ui::VKEY_END); + EXPECT_EQ(4, pending_state_item()->GetCommand()); + + // Clear references in menu controller to the menu item that is going away. + ResetSelection(); +} + // Tests that opening menu and pressing 'Down' and 'Up' iterates over enabled // items. TEST_F(MenuControllerTest, NextSelectedItem) { @@ -1669,7 +1731,7 @@ TEST_F(MenuControllerTest, MouseAtMenuItemOnShow) { // and show the menu. gfx::Size item_size = first_item->CalculatePreferredSize(); gfx::Point location(item_size.width() / 2, item_size.height() / 2); - owner()->GetNativeWindow()->GetRootWindow()->MoveCursorTo(location); + GetRootWindow(owner())->MoveCursorTo(location); menu_controller()->Run(owner(), nullptr, menu_item.get(), gfx::Rect(), MENU_ANCHOR_TOPLEFT, false, false); @@ -1704,9 +1766,15 @@ TEST_F(MenuControllerTest, AsynchronousCancelEvent) { EXPECT_EQ(MenuController::EXIT_ALL, controller->exit_type()); } -// Tests that if a menu is ran without a widget, that MenuPreTargetHandler does -// not cause a crash. +// Tests that menus without parent widgets do not crash in MenuPreTargetHandler. +// This is generally true, except on Chrome OS running with the window service. +// In that case, a DCHECK fires to ensure menus can consume parents' key events. TEST_F(MenuControllerTest, RunWithoutWidgetDoesntCrash) { +#if defined(OS_CHROMEOS) + if (features::IsUsingWindowService()) + return; +#endif // OS_CHROMEOS + ExitMenuRun(); MenuController* controller = menu_controller(); controller->Run(nullptr, nullptr, menu_item(), gfx::Rect(), @@ -1723,12 +1791,8 @@ TEST_F(MenuControllerTest, MenuControllerReplacedDuringDrag) { TestDragDropClient drag_drop_client( base::Bind(&MenuControllerTest::TestMenuControllerReplacementDuringDrag, base::Unretained(this))); - aura::client::SetDragDropClient(menu_item() - ->GetSubmenu() - ->GetWidget() - ->GetNativeWindow() - ->GetRootWindow(), - &drag_drop_client); + aura::client::SetDragDropClient( + GetRootWindow(menu_item()->GetSubmenu()->GetWidget()), &drag_drop_client); StartDrag(); } @@ -1741,12 +1805,8 @@ TEST_F(MenuControllerTest, CancelAllDuringDrag) { AddButtonMenuItems(); TestDragDropClient drag_drop_client(base::Bind( &MenuControllerTest::TestCancelAllDuringDrag, base::Unretained(this))); - aura::client::SetDragDropClient(menu_item() - ->GetSubmenu() - ->GetWidget() - ->GetNativeWindow() - ->GetRootWindow(), - &drag_drop_client); + aura::client::SetDragDropClient( + GetRootWindow(menu_item()->GetSubmenu()->GetWidget()), &drag_drop_client); StartDrag(); } diff --git a/chromium/ui/views/controls/menu/menu_delegate.cc b/chromium/ui/views/controls/menu/menu_delegate.cc index 56c3c51ca94..f8a91bcd5c7 100644 --- a/chromium/ui/views/controls/menu/menu_delegate.cc +++ b/chromium/ui/views/controls/menu/menu_delegate.cc @@ -152,4 +152,8 @@ bool MenuDelegate::ShouldReserveSpaceForSubmenuIndicator() const { return true; } +bool MenuDelegate::ShouldTryPositioningBesideAnchor() const { + return true; +} + } // namespace views diff --git a/chromium/ui/views/controls/menu/menu_delegate.h b/chromium/ui/views/controls/menu/menu_delegate.h index 706605182f9..69a8ed510b6 100644 --- a/chromium/ui/views/controls/menu/menu_delegate.h +++ b/chromium/ui/views/controls/menu/menu_delegate.h @@ -233,6 +233,11 @@ class VIEWS_EXPORT MenuDelegate { // Returns true if the labels should reserve additional spacing for e.g. // submenu indicators at the end of the line. virtual bool ShouldReserveSpaceForSubmenuIndicator() const; + + // Returns true if menus should fall back to positioning beside the anchor, + // rather than directly above or below it, when the menu is too tall to fit + // within the screen. + virtual bool ShouldTryPositioningBesideAnchor() const; }; } // namespace views diff --git a/chromium/ui/views/controls/menu/menu_host.cc b/chromium/ui/views/controls/menu/menu_host.cc index 780a0f9c27b..91889d70bf7 100644 --- a/chromium/ui/views/controls/menu/menu_host.cc +++ b/chromium/ui/views/controls/menu/menu_host.cc @@ -124,7 +124,7 @@ void MenuHost::InitMenuHost(Widget* parent, params.opacity = (bubble_border || rounded_border) ? Widget::InitParams::TRANSLUCENT_WINDOW : Widget::InitParams::OPAQUE_WINDOW; - params.parent = parent ? parent->GetNativeView() : NULL; + params.parent = parent ? parent->GetNativeView() : gfx::kNullNativeView; params.bounds = bounds; // If MenuHost has no parent widget, it needs to be marked // Activatable, so that calling Show in ShowMenuHost will diff --git a/chromium/ui/views/controls/menu/menu_image_util.cc b/chromium/ui/views/controls/menu/menu_image_util.cc index f5aa8df39aa..d1cc32bc757 100644 --- a/chromium/ui/views/controls/menu/menu_image_util.cc +++ b/chromium/ui/views/controls/menu/menu_image_util.cc @@ -4,7 +4,6 @@ #include "ui/views/controls/menu/menu_image_util.h" -#include "ui/base/material_design/material_design_controller.h" #include "ui/gfx/color_palette.h" #include "ui/gfx/paint_vector_icon.h" #include "ui/gfx/vector_icon_types.h" diff --git a/chromium/ui/views/controls/menu/menu_item_view.cc b/chromium/ui/views/controls/menu/menu_item_view.cc index 7b17d2662ec..c26f578b8c6 100644 --- a/chromium/ui/views/controls/menu/menu_item_view.cc +++ b/chromium/ui/views/controls/menu/menu_item_view.cc @@ -196,6 +196,7 @@ void MenuItemView::GetAccessibleNodeData(ui::AXNodeData* node_data) { case NORMAL: case SEPARATOR: case EMPTY: + case HIGHLIGHTED: // No additional accessibility states currently for these menu states. break; } @@ -692,6 +693,12 @@ void MenuItemView::SetForcedVisualSelection(bool selected) { SchedulePaint(); } +void MenuItemView::SetCornerRadius(int radius) { + DCHECK_EQ(GetType(), HIGHLIGHTED); + corner_radius_ = radius; + invalidate_dimensions(); // Triggers preferred size recalculation. +} + MenuItemView::MenuItemView(MenuItemView* parent, int command, MenuItemView::Type type) @@ -781,6 +788,7 @@ void MenuItemView::Init(MenuItemView* parent, submenu_arrow_image_view_ = nullptr; vertical_separator_ = nullptr; show_mnemonics_ = false; + corner_radius_ = 0; // Assign our ID, this allows SubmenuItemView to find MenuItemViews. set_id(kMenuItemViewID); has_icons_ = false; @@ -925,26 +933,7 @@ void MenuItemView::PaintButton(gfx::Canvas* canvas, PaintButtonMode mode) { // Render the background. As MenuScrollViewContainer draws the background, we // only need the background when we want it to look different, as when we're // selected. - ui::NativeTheme* native_theme = GetNativeTheme(); - if (render_selection) { - gfx::Rect item_bounds(0, 0, width(), height()); - if (type_ == ACTIONABLE_SUBMENU) { - if (submenu_area_of_actionable_submenu_selected_) { - item_bounds = GetSubmenuAreaOfActionableSubmenu(); - } else { - item_bounds = gfx::Rect(gfx::Size( - width() - MenuConfig::instance().actionable_submenu_width - 1, - height())); - } - } - AdjustBoundsForRTLUI(&item_bounds); - - native_theme->Paint(canvas->sk_canvas(), - ui::NativeTheme::kMenuItemBackground, - ui::NativeTheme::kHovered, - item_bounds, - ui::NativeTheme::ExtraParams()); - } + PaintBackground(canvas, mode, render_selection); const int top_margin = GetTopMargin(); const int bottom_margin = GetBottomMargin(); @@ -999,6 +988,48 @@ void MenuItemView::PaintButton(gfx::Canvas* canvas, PaintButtonMode mode) { submenu_arrow_image_view_->SetImage(GetSubmenuArrowImage(icon_color)); } +void MenuItemView::PaintBackground(gfx::Canvas* canvas, + PaintButtonMode mode, + bool render_selection) { + if (GetType() == HIGHLIGHTED) { + // Highligted items always have a different-colored background, and ignore + // system theme. + ui::NativeTheme::ColorId color_id = + render_selection + ? ui::NativeTheme:: + kColorId_FocusedHighlightedMenuItemBackgroundColor + : ui::NativeTheme::kColorId_HighlightedMenuItemBackgroundColor; + cc::PaintFlags flags; + flags.setAntiAlias(true); + flags.setStyle(cc::PaintFlags::kFill_Style); + flags.setColor(GetNativeTheme()->GetSystemColor(color_id)); + // Draw a rounded rect that spills outside of the clipping area, so that the + // rounded corners only show in the bottom 2 corners. Note that + // |corner_radius_| should only be set when the highlighted item is at the + // end of the menu. + gfx::RectF spilling_rect(GetLocalBounds()); + spilling_rect.set_y(spilling_rect.y() - corner_radius_); + spilling_rect.set_height(spilling_rect.height() + corner_radius_); + canvas->DrawRoundRect(spilling_rect, corner_radius_, flags); + } else if (render_selection) { + gfx::Rect item_bounds = GetLocalBounds(); + if (type_ == ACTIONABLE_SUBMENU) { + if (submenu_area_of_actionable_submenu_selected_) { + item_bounds = GetSubmenuAreaOfActionableSubmenu(); + } else { + item_bounds.set_width(item_bounds.width() - + MenuConfig::instance().actionable_submenu_width - + 1); + } + } + AdjustBoundsForRTLUI(&item_bounds); + + GetNativeTheme()->Paint( + canvas->sk_canvas(), ui::NativeTheme::kMenuItemBackground, + ui::NativeTheme::kHovered, item_bounds, ui::NativeTheme::ExtraParams()); + } +} + void MenuItemView::PaintMinorIconAndText( gfx::Canvas* canvas, const MenuDelegate::LabelStyle& style) { @@ -1059,6 +1090,9 @@ SkColor MenuItemView::GetTextColor(bool minor, bool render_selection) const { if (GetMenuController() && GetMenuController()->use_touchable_layout()) color_id = ui::NativeTheme::kColorId_TouchableMenuItemLabelColor; + if (GetType() == HIGHLIGHTED) + color_id = ui::NativeTheme::kColorId_HighlightedMenuItemForegroundColor; + return GetNativeTheme()->GetSystemColor(color_id); } @@ -1074,23 +1108,28 @@ void MenuItemView::DestroyAllMenuHosts() { } int MenuItemView::GetTopMargin() const { - if (top_margin_ >= 0) - return top_margin_; - - const MenuItemView* root = GetRootMenuItem(); - return root && root->has_icons_ - ? MenuConfig::instance().item_top_margin - : MenuConfig::instance().item_no_icon_top_margin; + int margin = top_margin_; + if (margin < 0) { + const MenuItemView* root = GetRootMenuItem(); + margin = root && root->has_icons_ + ? MenuConfig::instance().item_top_margin + : MenuConfig::instance().item_no_icon_top_margin; + } + return margin + corner_radius_ / 2; } int MenuItemView::GetBottomMargin() const { - if (bottom_margin_ >= 0) - return bottom_margin_; - - const MenuItemView* root = GetRootMenuItem(); - return root && root->has_icons_ - ? MenuConfig::instance().item_bottom_margin - : MenuConfig::instance().item_no_icon_bottom_margin; + int margin = bottom_margin_; + if (margin < 0) { + const MenuItemView* root = GetRootMenuItem(); + margin = root && root->has_icons_ + ? MenuConfig::instance().item_bottom_margin + : MenuConfig::instance().item_no_icon_bottom_margin; + } + // Add half of |corner_radius_| in both GetTopMargin() and GetBottomMargin(), + // so that they add up to exactly |corner_radius_|. When |corner_radius_| is + // odd, we need to add 1 here to avoid the height being off by 1. + return margin + corner_radius_ / 2 + (corner_radius_ % 2); } gfx::Size MenuItemView::GetChildPreferredSize() const { diff --git a/chromium/ui/views/controls/menu/menu_item_view.h b/chromium/ui/views/controls/menu/menu_item_view.h index 160a0069256..f0e80ee12e6 100644 --- a/chromium/ui/views/controls/menu/menu_item_view.h +++ b/chromium/ui/views/controls/menu/menu_item_view.h @@ -91,6 +91,9 @@ class VIEWS_EXPORT MenuItemView : public View { CHECKBOX, // Can be selected/checked to toggle a boolean state. RADIO, // Can be selected/checked among a group of choices. SEPARATOR, // Shows a horizontal line separator. + HIGHLIGHTED, // Performs an action when selected, and has a + // different colored background that merges with the + // menu's rounded corners when placed at the bottom. EMPTY, // EMPTY is a special type for empty menus that is only used // internally. }; @@ -359,6 +362,11 @@ class VIEWS_EXPORT MenuItemView : public View { // there's no way to unset it for this MenuItemView! void SetForcedVisualSelection(bool selected); + // For items of type HIGHLIGHTED only: sets the radius of the item's + // background. This makes the menu item's background fit its container's + // border radius, if they are both the same value. + void SetCornerRadius(int radius); + protected: // Creates a MenuItemView. This is used by the various AddXXX methods. MenuItemView(MenuItemView* parent, int command, Type type); @@ -424,6 +432,12 @@ class VIEWS_EXPORT MenuItemView : public View { // are not rendered. void PaintButton(gfx::Canvas* canvas, PaintButtonMode mode); + // Helper function for PaintButton(), draws the background for the button if + // appropriate. + void PaintBackground(gfx::Canvas* canvas, + PaintButtonMode mode, + bool render_selection); + // Paints the right-side icon and text. void PaintMinorIconAndText(gfx::Canvas* canvas, const MenuDelegate::LabelStyle& style); @@ -568,6 +582,9 @@ class VIEWS_EXPORT MenuItemView : public View { int top_margin_; int bottom_margin_; + // Corner radius in pixels, for HIGHLIGHTED items placed at the end of a menu. + int corner_radius_; + // Horizontal icon margins in pixels, which can differ between MenuItems. // These values will be set in the layout process. mutable int left_icon_margin_; diff --git a/chromium/ui/views/controls/menu/menu_model_adapter.cc b/chromium/ui/views/controls/menu/menu_model_adapter.cc index cbccee16e4c..08132c522c1 100644 --- a/chromium/ui/views/controls/menu/menu_model_adapter.cc +++ b/chromium/ui/views/controls/menu/menu_model_adapter.cc @@ -83,6 +83,9 @@ MenuItemView* MenuModelAdapter::AddMenuItemFromModelAt(ui::MenuModel* model, case ui::MenuModel::TYPE_ACTIONABLE_SUBMENU: type = MenuItemView::ACTIONABLE_SUBMENU; break; + case ui::MenuModel::TYPE_HIGHLIGHTED: + type = MenuItemView::HIGHLIGHTED; + break; } if (*type == MenuItemView::SEPARATOR) { diff --git a/chromium/ui/views/controls/menu/menu_model_adapter_unittest.cc b/chromium/ui/views/controls/menu/menu_model_adapter_unittest.cc index 00da8e3ebcc..392f6c3ed34 100644 --- a/chromium/ui/views/controls/menu/menu_model_adapter_unittest.cc +++ b/chromium/ui/views/controls/menu/menu_model_adapter_unittest.cc @@ -241,6 +241,9 @@ void CheckSubmenu(const RootModel& model, case ui::MenuModel::TYPE_ACTIONABLE_SUBMENU: EXPECT_EQ(views::MenuItemView::ACTIONABLE_SUBMENU, item->GetType()); break; + case ui::MenuModel::TYPE_HIGHLIGHTED: + EXPECT_EQ(views::MenuItemView::HIGHLIGHTED, item->GetType()); + break; } // Check enabled state. @@ -312,6 +315,9 @@ TEST_F(MenuModelAdapterTest, BasicTest) { case ui::MenuModel::TYPE_ACTIONABLE_SUBMENU: EXPECT_EQ(views::MenuItemView::ACTIONABLE_SUBMENU, item->GetType()); break; + case ui::MenuModel::TYPE_HIGHLIGHTED: + EXPECT_EQ(views::MenuItemView::HIGHLIGHTED, item->GetType()); + break; } // Check enabled state. diff --git a/chromium/ui/views/controls/menu/menu_pre_target_handler_aura.cc b/chromium/ui/views/controls/menu/menu_pre_target_handler_aura.cc index b38b0d49d53..459347bf9ee 100644 --- a/chromium/ui/views/controls/menu/menu_pre_target_handler_aura.cc +++ b/chromium/ui/views/controls/menu/menu_pre_target_handler_aura.cc @@ -25,26 +25,23 @@ MenuPreTargetHandlerAura::MenuPreTargetHandlerAura(MenuController* controller, Widget* owner) : controller_(controller), root_(GetOwnerRootWindow(owner)) { if (root_) { - root_->env()->AddPreTargetHandler(this, ui::EventTarget::Priority::kSystem); + aura_env_ = root_->env(); wm::GetActivationClient(root_)->AddObserver(this); root_->AddObserver(this); } else { - // TODO(mukai): check if this code path can run in ChromeOS and find the - // solution for SingleProcessMash. - if (features::IsUsingWindowService()) { - LOG(WARNING) << "MenuPreTargetHandlerAura is created without owner " - << "widget. This may not work well in SingleProcessMash."; - } - aura::Env::GetInstance()->AddPreTargetHandler( - this, ui::EventTarget::Priority::kSystem); + // This should only happen in cases like when context menus are shown for + // Windows OS system tray items and there is no parent window. This should + // not be hit on Chrome OS, where Window Service clients need to install a + // pre-target handler on the aura::Env associated with their app window. + DCHECK(!features::IsUsingWindowService()) + << "MenuPreTargetHandlerAura may not work correctly without an owner."; + aura_env_ = aura::Env::GetInstance(); } + aura_env_->AddPreTargetHandler(this, ui::EventTarget::Priority::kSystem); } MenuPreTargetHandlerAura::~MenuPreTargetHandlerAura() { - if (root_) - root_->env()->RemovePreTargetHandler(this); - else - aura::Env::GetInstance()->RemovePreTargetHandler(this); + aura_env_->RemovePreTargetHandler(this); Cleanup(); } diff --git a/chromium/ui/views/controls/menu/menu_pre_target_handler_aura.h b/chromium/ui/views/controls/menu/menu_pre_target_handler_aura.h index 13fc79cbde2..f1e494e494f 100644 --- a/chromium/ui/views/controls/menu/menu_pre_target_handler_aura.h +++ b/chromium/ui/views/controls/menu/menu_pre_target_handler_aura.h @@ -12,6 +12,7 @@ #include "ui/wm/public/activation_change_observer.h" namespace aura { +class Env; class Window; } // namespace aura @@ -48,6 +49,7 @@ class VIEWS_EXPORT MenuPreTargetHandlerAura private: void Cleanup(); + aura::Env* aura_env_ = nullptr; MenuController* controller_; aura::Window* root_; diff --git a/chromium/ui/views/controls/menu/menu_pre_target_handler_mac.h b/chromium/ui/views/controls/menu/menu_pre_target_handler_mac.h index d2250979d3d..76a78f7c308 100644 --- a/chromium/ui/views/controls/menu/menu_pre_target_handler_mac.h +++ b/chromium/ui/views/controls/menu/menu_pre_target_handler_mac.h @@ -5,24 +5,23 @@ #ifndef UI_VIEWS_CONTROLS_MENU_MENU_PRE_TARGET_HANDLER_MAC_H_ #define UI_VIEWS_CONTROLS_MENU_MENU_PRE_TARGET_HANDLER_MAC_H_ -#include "ui/events/event_handler.h" #include "ui/views/controls/menu/menu_pre_target_handler.h" -#include "ui/views/event_monitor_mac.h" + +#include "ui/base/cocoa/weak_ptr_nsobject.h" namespace views { -class MenuPreTargetHandlerMac : public MenuPreTargetHandler, - public ui::EventHandler { +// Stops dispatch of key events when they are handled by MenuController. +// While similar to EventMonitorMac, that class does not allow dispatch changes. +class MenuPreTargetHandlerMac : public MenuPreTargetHandler { public: MenuPreTargetHandlerMac(MenuController* controller, Widget* widget); ~MenuPreTargetHandlerMac() override; - // ui::EventHandler: - void OnKeyEvent(ui::KeyEvent* event) override; - private: MenuController* controller_; // Weak. Owns |this|. - std::unique_ptr<EventMonitorMac> event_monitor_; + id monitor_; + ui::WeakPtrNSObjectFactory<MenuPreTargetHandlerMac> factory_; DISALLOW_COPY_AND_ASSIGN(MenuPreTargetHandlerMac); }; diff --git a/chromium/ui/views/controls/menu/menu_pre_target_handler_mac.mm b/chromium/ui/views/controls/menu/menu_pre_target_handler_mac.mm index 81ce0dafa79..4308b8cdf3a 100644 --- a/chromium/ui/views/controls/menu/menu_pre_target_handler_mac.mm +++ b/chromium/ui/views/controls/menu/menu_pre_target_handler_mac.mm @@ -4,6 +4,10 @@ #include "ui/views/controls/menu/menu_pre_target_handler_mac.h" +#import <Cocoa/Cocoa.h> + +#include "ui/events/event.h" +#include "ui/events/event_utils.h" #include "ui/views/controls/menu/menu_controller.h" #include "ui/views/widget/widget.h" @@ -11,17 +15,35 @@ namespace views { MenuPreTargetHandlerMac::MenuPreTargetHandlerMac(MenuController* controller, Widget* widget) - : controller_(controller), - event_monitor_( - std::make_unique<EventMonitorMac>(this, widget->GetNativeWindow())) {} - -MenuPreTargetHandlerMac::~MenuPreTargetHandlerMac() {} + : controller_(controller), factory_(this) { + gfx::NativeWindow target_window = widget->GetNativeWindow(); + + // Capture a WeakPtr via NSObject. This allows the block to detect another + // event monitor for the same event deleting |this|. + WeakPtrNSObject* handle = factory_.handle(); + + auto block = ^NSEvent*(NSEvent* event) { + if (!ui::WeakPtrNSObjectFactory<MenuPreTargetHandlerMac>::Get(handle)) + return event; + + if (!target_window || [event window] == target_window.GetNativeNSWindow()) { + std::unique_ptr<ui::Event> ui_event = ui::EventFromNative(event); + if (ui_event && ui_event->IsKeyEvent() && + controller_->OnWillDispatchKeyEvent(ui_event->AsKeyEvent()) != + ui::POST_DISPATCH_PERFORM_DEFAULT) { + // Return nil so the event will not proceed through normal dispatch. + return nil; + } + } + return event; + }; + + monitor_ = [NSEvent addLocalMonitorForEventsMatchingMask:NSKeyDownMask + handler:block]; +} -void MenuPreTargetHandlerMac::OnKeyEvent(ui::KeyEvent* event) { - if (controller_->OnWillDispatchKeyEvent(event) != - ui::POST_DISPATCH_PERFORM_DEFAULT) { - event->SetHandled(); - } +MenuPreTargetHandlerMac::~MenuPreTargetHandlerMac() { + [NSEvent removeMonitor:monitor_]; } // static diff --git a/chromium/ui/views/controls/menu/menu_runner_cocoa_unittest.mm b/chromium/ui/views/controls/menu/menu_runner_cocoa_unittest.mm index 7acfc99f907..557e7e372ce 100644 --- a/chromium/ui/views/controls/menu/menu_runner_cocoa_unittest.mm +++ b/chromium/ui/views/controls/menu/menu_runner_cocoa_unittest.mm @@ -106,7 +106,8 @@ class MenuRunnerCocoaTest : public ViewsTestBase, gfx::Rect(kWindowOffset, kWindowOffset, kWindowWidth, kWindowHeight)); parent_->Show(); - native_view_subview_count_ = [[parent_->GetNativeView() subviews] count]; + native_view_subview_count_ = + [[parent_->GetNativeView().GetNativeNSView() subviews] count]; base::Closure on_close = base::Bind(&MenuRunnerCocoaTest::MenuCloseCallback, base::Unretained(this)); @@ -119,7 +120,7 @@ class MenuRunnerCocoaTest : public ViewsTestBase, void TearDown() override { EXPECT_EQ(native_view_subview_count_, - [[parent_->GetNativeView() subviews] count]); + [[parent_->GetNativeView().GetNativeNSView() subviews] count]); if (runner_) { runner_->Release(); @@ -252,12 +253,11 @@ class MenuRunnerCocoaTest : public ViewsTestBase, } void ComboboxRunMenuAtCallback() { - NSArray* subviews = [parent_->GetNativeView() subviews]; + NSArray* subviews = [parent_->GetNativeView().GetNativeNSView() subviews]; // An anchor view should only be added for Native menus. if (GetParam() == MenuType::NATIVE) { ASSERT_EQ(native_view_subview_count_ + 1, [subviews count]); - last_anchor_frame_ = - [[subviews objectAtIndex:native_view_subview_count_] frame]; + last_anchor_frame_ = [subviews[native_view_subview_count_] frame]; } else { EXPECT_EQ(native_view_subview_count_, [subviews count]); } diff --git a/chromium/ui/views/controls/menu/menu_runner_impl_cocoa.mm b/chromium/ui/views/controls/menu/menu_runner_impl_cocoa.mm index f0ea47108de..f67cdcbd9f9 100644 --- a/chromium/ui/views/controls/menu/menu_runner_impl_cocoa.mm +++ b/chromium/ui/views/controls/menu/menu_runner_impl_cocoa.mm @@ -176,11 +176,12 @@ void MenuRunnerImplCocoa::RunMenuAt(Widget* parent, // Ensure the UI can update while the menu is fading out. base::ScopedPumpMessagesInPrivateModes pump_private; - NSWindow* window = parent->GetNativeWindow(); + NSWindow* window = parent->GetNativeWindow().GetNativeNSWindow(); + NSView* view = parent->GetNativeView().GetNativeNSView(); if (run_types & MenuRunner::CONTEXT_MENU) { [NSMenu popUpContextMenu:[menu_controller_ menu] withEvent:EventForPositioningContextMenu(bounds, window) - forView:parent->GetNativeView()]; + forView:view]; } else if (run_types & MenuRunner::COMBOBOX) { NSMenuItem* checked_item = FirstCheckedItem(menu_controller_); NSMenu* menu = [menu_controller_ menu]; diff --git a/chromium/ui/views/controls/menu/menu_runner_unittest.cc b/chromium/ui/views/controls/menu/menu_runner_unittest.cc index bb3961c9af0..577c4cceb03 100644 --- a/chromium/ui/views/controls/menu/menu_runner_unittest.cc +++ b/chromium/ui/views/controls/menu/menu_runner_unittest.cc @@ -27,6 +27,7 @@ #include "ui/views/test/views_test_base.h" #include "ui/views/widget/native_widget_private.h" #include "ui/views/widget/widget.h" +#include "ui/views/widget/widget_utils.h" namespace { @@ -369,7 +370,7 @@ class MenuRunnerWidgetTest : public MenuRunnerTest { std::unique_ptr<ui::test::EventGenerator> EventGeneratorForWidget( Widget* widget) { return std::make_unique<ui::test::EventGenerator>( - IsMus() ? widget->GetNativeWindow() : GetContext(), + IsMus() ? GetRootWindow(widget) : GetContext(), widget->GetNativeWindow()); } @@ -413,8 +414,9 @@ class MenuRunnerWidgetTest : public MenuRunnerTest { TEST_F(MenuRunnerWidgetTest, WidgetDoesntTakeCapture) { AddMenuLauncherEventHandler(owner()); - EXPECT_EQ(nullptr, internal::NativeWidgetPrivate::GetGlobalCapture( - widget()->GetNativeView())); + EXPECT_EQ(gfx::kNullNativeView, + internal::NativeWidgetPrivate::GetGlobalCapture( + widget()->GetNativeView())); auto generator(EventGeneratorForWidget(widget())); // Implicit capture should not be held by |widget|. generator->PressLeftButton(); diff --git a/chromium/ui/views/controls/menu/menu_scroll_view_container.cc b/chromium/ui/views/controls/menu/menu_scroll_view_container.cc index ecbd5d773b9..0ad986a75c3 100644 --- a/chromium/ui/views/controls/menu/menu_scroll_view_container.cc +++ b/chromium/ui/views/controls/menu/menu_scroll_view_container.cc @@ -172,9 +172,7 @@ class MenuScrollViewContainer::MenuScrollView : public View { // MenuScrollViewContainer ---------------------------------------------------- MenuScrollViewContainer::MenuScrollViewContainer(SubmenuView* content_view) - : content_view_(content_view), - arrow_(BubbleBorder::NONE), - bubble_border_(NULL) { + : content_view_(content_view) { scroll_up_button_ = new MenuScrollButton(content_view, true); scroll_down_button_ = new MenuScrollButton(content_view, false); AddChildView(scroll_up_button_); @@ -186,10 +184,7 @@ MenuScrollViewContainer::MenuScrollViewContainer(SubmenuView* content_view) arrow_ = BubbleBorderTypeFromAnchor( content_view_->GetMenuItem()->GetMenuController()->GetAnchorPosition()); - if (arrow_ != BubbleBorder::NONE) - CreateBubbleBorder(); - else - CreateDefaultBorder(); + CreateBorder(); } bool MenuScrollViewContainer::HasBubbleBorder() { @@ -201,10 +196,19 @@ void MenuScrollViewContainer::SetBubbleArrowOffset(int offset) { bubble_border_->set_arrow_offset(offset); } +MenuItemView* MenuScrollViewContainer::GetFootnote() const { + MenuItemView* footnote = content_view_->GetLastItem(); + if (!footnote || footnote->GetType() != MenuItemView::HIGHLIGHTED) + return nullptr; + return footnote; +} + gfx::Size MenuScrollViewContainer::CalculatePreferredSize() const { gfx::Size prefsize = scroll_view_->GetContents()->GetPreferredSize(); gfx::Insets insets = GetInsets(); prefsize.Enlarge(insets.width(), insets.height()); + if (GetFootnote() && bubble_border_) + prefsize.Enlarge(0, bubble_border_->GetBorderCornerRadius()); return prefsize; } @@ -214,12 +218,19 @@ void MenuScrollViewContainer::Layout() { int y = insets.top(); int width = View::width() - insets.width(); int content_height = height() - insets.height(); + MenuItemView* footnote = GetFootnote(); if (!scroll_up_button_->visible()) { + if (footnote && bubble_border_) + footnote->SetCornerRadius(bubble_border_->GetBorderCornerRadius()); scroll_view_->SetBounds(x, y, width, content_height); scroll_view_->Layout(); return; } + // Don't round the footnote when the scroll button is visible. + if (footnote) + footnote->SetCornerRadius(0); + gfx::Size pref = scroll_up_button_->GetPreferredSize(); scroll_up_button_->SetBounds(x, y, width, pref.height()); content_height -= pref.height(); @@ -237,7 +248,7 @@ void MenuScrollViewContainer::Layout() { void MenuScrollViewContainer::OnNativeThemeChanged( const ui::NativeTheme* theme) { - if (arrow_ == BubbleBorder::NONE) + if (!HasBubbleBorder()) CreateDefaultBorder(); } @@ -270,6 +281,13 @@ void MenuScrollViewContainer::OnBoundsChanged( Layout(); } +void MenuScrollViewContainer::CreateBorder() { + if (HasBubbleBorder()) + CreateBubbleBorder(); + else + CreateDefaultBorder(); +} + void MenuScrollViewContainer::CreateDefaultBorder() { DCHECK_EQ(arrow_, BubbleBorder::NONE); bubble_border_ = nullptr; @@ -292,6 +310,8 @@ void MenuScrollViewContainer::CreateDefaultBorder() { const int horizontal_inset = menu_config.menu_horizontal_border_size + padding; + int bottom_inset = GetFootnote() ? 0 : vertical_inset; + if (use_outer_border) { SkColor color = GetNativeTheme() ? GetNativeTheme()->GetSystemColor( @@ -299,10 +319,11 @@ void MenuScrollViewContainer::CreateDefaultBorder() { : gfx::kPlaceholderColor; SetBorder(views::CreateBorderPainter( std::make_unique<views::RoundRectPainter>(color, corner_radius), - gfx::Insets(vertical_inset, horizontal_inset))); + gfx::Insets(vertical_inset, horizontal_inset, bottom_inset, + horizontal_inset))); } else { - SetBorder(CreateEmptyBorder(vertical_inset, horizontal_inset, - vertical_inset, horizontal_inset)); + SetBorder(CreateEmptyBorder(vertical_inset, horizontal_inset, bottom_inset, + horizontal_inset)); } } @@ -316,8 +337,10 @@ void MenuScrollViewContainer::CreateBubbleBorder() { bubble_border_->SetCornerRadius(menu_config.touchable_corner_radius); bubble_border_->set_md_shadow_elevation( menu_config.touchable_menu_shadow_elevation); - scroll_view_->GetContents()->SetBorder(CreateEmptyBorder( - gfx::Insets(menu_config.vertical_touchable_menu_item_padding, 0))); + gfx::Insets insets(menu_config.vertical_touchable_menu_item_padding, 0); + if (GetFootnote()) + insets.Set(menu_config.vertical_touchable_menu_item_padding, 0, 0, 0); + scroll_view_->GetContents()->SetBorder(CreateEmptyBorder(insets)); } SetBorder(std::unique_ptr<Border>(bubble_border_)); diff --git a/chromium/ui/views/controls/menu/menu_scroll_view_container.h b/chromium/ui/views/controls/menu/menu_scroll_view_container.h index f08341929cb..290fe81bb33 100644 --- a/chromium/ui/views/controls/menu/menu_scroll_view_container.h +++ b/chromium/ui/views/controls/menu/menu_scroll_view_container.h @@ -12,6 +12,7 @@ namespace views { +class MenuItemView; class SubmenuView; // MenuScrollViewContainer contains the SubmenuView (through a MenuScrollView) @@ -31,6 +32,8 @@ class MenuScrollViewContainer : public View { // Offsets the Arrow from the default location. void SetBubbleArrowOffset(int offset); + void SetFootnoteView(View* view); + // View overrides. gfx::Size CalculatePreferredSize() const override; void Layout() override; @@ -43,6 +46,9 @@ class MenuScrollViewContainer : public View { void OnBoundsChanged(const gfx::Rect& previous_bounds) override; private: + // Create a default border or bubble border, as appropriate. + void CreateBorder(); + // Create the default border. void CreateDefaultBorder(); @@ -51,6 +57,9 @@ class MenuScrollViewContainer : public View { BubbleBorder::Arrow BubbleBorderTypeFromAnchor(MenuAnchorPosition anchor); + // Returns the last item in the menu if it is of type HIGHLIGHTED. + MenuItemView* GetFootnote() const; + class MenuScrollView; // The scroll buttons. @@ -64,10 +73,10 @@ class MenuScrollViewContainer : public View { SubmenuView* content_view_; // If set the currently set border is a bubble border. - BubbleBorder::Arrow arrow_; + BubbleBorder::Arrow arrow_ = BubbleBorder::NONE; // Weak reference to the currently set border. - BubbleBorder* bubble_border_; + BubbleBorder* bubble_border_ = nullptr; DISALLOW_COPY_AND_ASSIGN(MenuScrollViewContainer); }; diff --git a/chromium/ui/views/controls/menu/submenu_view.cc b/chromium/ui/views/controls/menu/submenu_view.cc index 36865a59316..04c38f5c364 100644 --- a/chromium/ui/views/controls/menu/submenu_view.cc +++ b/chromium/ui/views/controls/menu/submenu_view.cc @@ -475,6 +475,14 @@ MenuScrollViewContainer* SubmenuView::GetScrollViewContainer() { return scroll_view_container_; } +MenuItemView* SubmenuView::GetLastItem() { + for (int i = child_count() - 1; i >= 0; i--) { + if (child_at(i)->id() == MenuItemView::kMenuItemViewID) + return static_cast<MenuItemView*>(child_at(i)); + } + return nullptr; +} + void SubmenuView::MenuHostDestroyed() { host_ = NULL; MenuController* controller = GetMenuItem()->GetMenuController(); diff --git a/chromium/ui/views/controls/menu/submenu_view.h b/chromium/ui/views/controls/menu/submenu_view.h index 4374790035e..37056577fe9 100644 --- a/chromium/ui/views/controls/menu/submenu_view.h +++ b/chromium/ui/views/controls/menu/submenu_view.h @@ -147,6 +147,9 @@ class VIEWS_EXPORT SubmenuView : public View, // Returns the container for the SubmenuView. MenuScrollViewContainer* GetScrollViewContainer(); + // Returns the last MenuItemView in this submenu. + MenuItemView* GetLastItem(); + // Invoked if the menu is prematurely destroyed. This can happen if the window // closes while the menu is shown. If invoked the SubmenuView must drop all // references to the MenuHost as the MenuHost is about to be deleted. diff --git a/chromium/ui/views/controls/menu/submenu_view_unittest.cc b/chromium/ui/views/controls/menu/submenu_view_unittest.cc new file mode 100644 index 00000000000..9b2458799c9 --- /dev/null +++ b/chromium/ui/views/controls/menu/submenu_view_unittest.cc @@ -0,0 +1,36 @@ +// Copyright 2018 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "ui/views/controls/menu/submenu_view.h" + +#include "testing/gtest/include/gtest/gtest.h" +#include "ui/views/controls/menu/menu_item_view.h" +#include "ui/views/controls/menu/menu_runner.h" +#include "ui/views/controls/menu/submenu_view.h" + +namespace views { + +TEST(SubmenuViewTest, GetLastItem) { + MenuItemView* parent = new MenuItemView(nullptr); + MenuRunner menu_runner(parent, 0); + + SubmenuView* submenu = parent->CreateSubmenu(); + EXPECT_EQ(nullptr, submenu->GetLastItem()); + + submenu->AddChildView(new View()); + EXPECT_EQ(nullptr, submenu->GetLastItem()); + + MenuItemView* first = new MenuItemView(nullptr); + submenu->AddChildView(first); + EXPECT_EQ(first, submenu->GetLastItem()); + + submenu->AddChildView(new View()); + EXPECT_EQ(first, submenu->GetLastItem()); + + MenuItemView* second = new MenuItemView(nullptr); + submenu->AddChildView(second); + EXPECT_EQ(second, submenu->GetLastItem()); +} + +} // namespace views diff --git a/chromium/ui/views/controls/native/native_view_host.cc b/chromium/ui/views/controls/native/native_view_host.cc index 5240d2bf72d..270f22b0ece 100644 --- a/chromium/ui/views/controls/native/native_view_host.cc +++ b/chromium/ui/views/controls/native/native_view_host.cc @@ -176,7 +176,7 @@ void NativeViewHost::ViewHierarchyChanged( if (!native_wrapper_.get()) native_wrapper_.reset(NativeViewHostWrapper::CreateWrapper(this)); native_wrapper_->AddedToWidget(); - } else if (!details.is_add) { + } else if (!details.is_add && native_wrapper_) { native_wrapper_->RemovedFromWidget(); } } @@ -206,6 +206,11 @@ gfx::NativeCursor NativeViewHost::GetCursor(const ui::MouseEvent& event) { return native_wrapper_->GetCursor(event.x(), event.y()); } +void NativeViewHost::SetVisible(bool visible) { + native_wrapper_->SetVisible(visible); + View::SetVisible(visible); +} + //////////////////////////////////////////////////////////////////////////////// // NativeViewHost, private: diff --git a/chromium/ui/views/controls/native/native_view_host.h b/chromium/ui/views/controls/native/native_view_host.h index ec0b8e9f18a..5c47f29d258 100644 --- a/chromium/ui/views/controls/native/native_view_host.h +++ b/chromium/ui/views/controls/native/native_view_host.h @@ -98,6 +98,7 @@ class VIEWS_EXPORT NativeViewHost : public View { void OnFocus() override; gfx::NativeViewAccessible GetNativeViewAccessible() override; gfx::NativeCursor GetCursor(const ui::MouseEvent& event) override; + void SetVisible(bool visible) override; protected: bool GetNeedsNotificationWhenVisibleBoundsChange() const override; diff --git a/chromium/ui/views/controls/native/native_view_host_aura.cc b/chromium/ui/views/controls/native/native_view_host_aura.cc index 3be7f536eb1..dabae673dd7 100644 --- a/chromium/ui/views/controls/native/native_view_host_aura.cc +++ b/chromium/ui/views/controls/native/native_view_host_aura.cc @@ -134,8 +134,12 @@ void NativeViewHostAura::AddedToWidget() { void NativeViewHostAura::RemovedFromWidget() { if (host_->native_view()) { - host_->native_view()->Hide(); + // Clear kHostWindowKey before Hide() because it could be accessed during + // the call. In MUS aura, the hosting window could be destroyed at this + // point. host_->native_view()->ClearProperty(aura::client::kHostWindowKey); + + host_->native_view()->Hide(); if (host_->native_view()->parent()) host_->native_view()->parent()->RemoveChild(host_->native_view()); RemoveClippingWindow(); @@ -229,6 +233,13 @@ gfx::NativeCursor NativeViewHostAura::GetCursor(int x, int y) { return gfx::kNullCursor; } +void NativeViewHostAura::SetVisible(bool visible) { + if (!visible) + host_->native_view()->Hide(); + else + host_->native_view()->Show(); +} + void NativeViewHostAura::OnWindowBoundsChanged( aura::Window* window, const gfx::Rect& old_bounds, diff --git a/chromium/ui/views/controls/native/native_view_host_aura.h b/chromium/ui/views/controls/native/native_view_host_aura.h index 0adf8fb3aec..1e24714575b 100644 --- a/chromium/ui/views/controls/native/native_view_host_aura.h +++ b/chromium/ui/views/controls/native/native_view_host_aura.h @@ -44,6 +44,7 @@ class NativeViewHostAura : public NativeViewHostWrapper, gfx::NativeView GetNativeViewContainer() const override; gfx::NativeViewAccessible GetNativeViewAccessible() override; gfx::NativeCursor GetCursor(int x, int y) override; + void SetVisible(bool visible) override; private: friend class NativeViewHostAuraTest; diff --git a/chromium/ui/views/controls/native/native_view_host_mac.h b/chromium/ui/views/controls/native/native_view_host_mac.h index f46e575e442..08a0ce36806 100644 --- a/chromium/ui/views/controls/native/native_view_host_mac.h +++ b/chromium/ui/views/controls/native/native_view_host_mac.h @@ -51,6 +51,7 @@ class NativeViewHostMac : public NativeViewHostWrapper, gfx::NativeView GetNativeViewContainer() const override; gfx::NativeViewAccessible GetNativeViewAccessible() override; gfx::NativeCursor GetCursor(int x, int y) override; + void SetVisible(bool visible) override; private: // Return the BridgedNativeWidgetHostImpl for this hosted view. diff --git a/chromium/ui/views/controls/native/native_view_host_mac.mm b/chromium/ui/views/controls/native/native_view_host_mac.mm index d98e6e1456c..7ce36315bfa 100644 --- a/chromium/ui/views/controls/native/native_view_host_mac.mm +++ b/chromium/ui/views/controls/native/native_view_host_mac.mm @@ -68,7 +68,10 @@ uint64_t NativeViewHostMac::GetViewsFactoryHostId() const { auto* bridge_host = GetBridgedNativeWidgetHost(); if (bridge_host && bridge_host->bridge_factory_host()) return bridge_host->bridge_factory_host()->GetHostId(); - return 0; + // This matches content::NSViewBridgeFactoryHost::kLocalDirectHostId, + // indicating that this is a local window. + constexpr uint64_t kLocalDirectHostId = -1; + return kLocalDirectHostId; } uint64_t NativeViewHostMac::GetNSViewId() const { @@ -111,12 +114,14 @@ void NativeViewHostMac::OnHostableViewDestroying() { void NativeViewHostMac::AttachNativeView() { DCHECK(host_->native_view()); DCHECK(!native_view_); - native_view_.reset([host_->native_view() retain]); + native_view_.reset([host_->native_view().GetNativeNSView() retain]); EnsureNativeViewHasNoChildWidgets(native_view_); auto* bridge_host = GetBridgedNativeWidgetHost(); DCHECK(bridge_host); - [bridge_host->native_widget_mac()->GetNativeView() addSubview:native_view_]; + NSView* superview = + bridge_host->native_widget_mac()->GetNativeView().GetNativeNSView(); + [superview addSubview:native_view_]; bridge_host->SetAssociationForView(host_, native_view_); if ([native_view_ conformsToProtocol:@protocol(ViewsHostable)]) { @@ -134,21 +139,22 @@ void NativeViewHostMac::NativeViewDetaching(bool destroyed) { // |native_view_| can be nil here if RemovedFromWidget() is called before // NativeViewHost::Detach(). + NSView* host_native_view = host_->native_view().GetNativeNSView(); if (!native_view_) { - DCHECK(![host_->native_view() superview]); + DCHECK(![host_native_view superview]); return; } - DCHECK(native_view_ == host_->native_view()); - [host_->native_view() setHidden:YES]; - [host_->native_view() removeFromSuperview]; + DCHECK(native_view_ == host_native_view); + [native_view_ setHidden:YES]; + [native_view_ removeFromSuperview]; if (native_view_hostable_) { native_view_hostable_->OnViewsHostableDetached(); native_view_hostable_ = nullptr; } - EnsureNativeViewHasNoChildWidgets(host_->native_view()); + EnsureNativeViewHasNoChildWidgets(native_view_); auto* bridge_host = GetBridgedNativeWidgetHost(); // BridgedNativeWidgetImpl can be null when Widget is closing. if (bridge_host) @@ -195,8 +201,7 @@ void NativeViewHostMac::ShowWidget(int x, int h, int native_w, int native_h) { - if (host_->fast_resize()) - NOTIMPLEMENTED(); + // TODO(https://crbug.com/415024): Implement host_->fast_resize(). // Coordinates will be from the top left of the parent Widget. The NativeView // is already in the same NSWindow, so just flip to get Cooca coordinates and @@ -210,24 +215,24 @@ void NativeViewHostMac::ShowWidget(int x, // Convert window coordinates to the hosted view's superview, since that's how // coordinates of the hosted view's frame is based. NSRect container_rect = - [[host_->native_view() superview] convertRect:window_rect fromView:nil]; - [host_->native_view() setFrame:container_rect]; - [host_->native_view() setHidden:NO]; + [[native_view_ superview] convertRect:window_rect fromView:nil]; + [native_view_ setFrame:container_rect]; + [native_view_ setHidden:NO]; if (native_view_hostable_) native_view_hostable_->OnViewsHostableShow(gfx::Rect(x, y, w, h)); } void NativeViewHostMac::HideWidget() { - [host_->native_view() setHidden:YES]; + [native_view_ setHidden:YES]; if (native_view_hostable_) native_view_hostable_->OnViewsHostableHide(); } void NativeViewHostMac::SetFocus() { - if ([host_->native_view() acceptsFirstResponder]) - [[host_->native_view() window] makeFirstResponder:host_->native_view()]; + if ([native_view_ acceptsFirstResponder]) + [[native_view_ window] makeFirstResponder:native_view_]; if (native_view_hostable_) native_view_hostable_->OnViewsHostableMakeFirstResponder(); @@ -256,6 +261,10 @@ gfx::NativeCursor NativeViewHostMac::GetCursor(int x, int y) { return gfx::kNullCursor; } +void NativeViewHostMac::SetVisible(bool visible) { + [native_view_ setHidden:!visible]; +} + // static NativeViewHostWrapper* NativeViewHostWrapper::CreateWrapper( NativeViewHost* host) { diff --git a/chromium/ui/views/controls/native/native_view_host_mac_unittest.mm b/chromium/ui/views/controls/native/native_view_host_mac_unittest.mm index 85373e5b977..3de36c65a96 100644 --- a/chromium/ui/views/controls/native/native_view_host_mac_unittest.mm +++ b/chromium/ui/views/controls/native/native_view_host_mac_unittest.mm @@ -76,7 +76,7 @@ class NativeViewHostMacTest : public test::NativeViewHostTestBase { toplevel()->GetRootView()->AddChildView(host()); EXPECT_TRUE(native_host()); - host()->Attach(native_view_); + host()->Attach(native_view_.get()); } protected: @@ -118,7 +118,7 @@ TEST_F(NativeViewHostMacTest, Attach) { EXPECT_FALSE([native_view_ window]); EXPECT_NSEQ(NSZeroRect, [native_view_ frame]); - host()->Attach(native_view_); + host()->Attach(native_view_.get()); EXPECT_TRUE([native_view_ superview]); EXPECT_TRUE([native_view_ window]); @@ -141,7 +141,7 @@ TEST_F(NativeViewHostMacTest, AccessibilityParent) { TestViewsHostable views_hostable; [view setViewsHostableView:&views_hostable]; - host()->Attach(view); + host()->Attach(view.get()); EXPECT_NSEQ(views_hostable.parent_accessibility_element(), toplevel()->GetRootView()->GetNativeViewAccessible()); @@ -186,14 +186,14 @@ TEST_F(NativeViewHostMacTest, NativeViewHidden) { host()->SetVisible(false); EXPECT_FALSE([native_view_ isHidden]); // Stays visible. - host()->Attach(native_view_); + host()->Attach(native_view_.get()); EXPECT_TRUE([native_view_ isHidden]); // Hidden when attached. host()->Detach(); [native_view_ setHidden:YES]; host()->SetVisible(true); EXPECT_TRUE([native_view_ isHidden]); // Stays hidden. - host()->Attach(native_view_); + host()->Attach(native_view_.get()); EXPECT_FALSE([native_view_ isHidden]); // Made visible when attached. EXPECT_TRUE([native_view_ superview]); diff --git a/chromium/ui/views/controls/native/native_view_host_wrapper.h b/chromium/ui/views/controls/native/native_view_host_wrapper.h index 07cf9f1fea0..acd9b2fbd5d 100644 --- a/chromium/ui/views/controls/native/native_view_host_wrapper.h +++ b/chromium/ui/views/controls/native/native_view_host_wrapper.h @@ -90,6 +90,11 @@ class NativeViewHostWrapper { // in the native view. virtual gfx::NativeCursor GetCursor(int x, int y) = 0; + // Sets the visibility of the gfx::NativeView. This differs from + // {Show,Hide}Widget because it doesn't affect the placement, size, + // or clipping of the view. + virtual void SetVisible(bool visible) = 0; + // Creates a platform-specific instance of an object implementing this // interface. static NativeViewHostWrapper* CreateWrapper(NativeViewHost* host); diff --git a/chromium/ui/views/controls/resize_area_unittest.cc b/chromium/ui/views/controls/resize_area_unittest.cc index 8d11df5e8ea..f5e0db232c5 100644 --- a/chromium/ui/views/controls/resize_area_unittest.cc +++ b/chromium/ui/views/controls/resize_area_unittest.cc @@ -13,6 +13,7 @@ #include "ui/views/test/views_test_base.h" #include "ui/views/view.h" #include "ui/views/widget/widget.h" +#include "ui/views/widget/widget_utils.h" namespace { // Constants used by the ResizeAreaTest.SuccessfulGestureDrag test to simulate @@ -130,8 +131,8 @@ void ResizeAreaTest::SetUp() { widget_->SetContentsView(resize_area_); widget_->Show(); - event_generator_.reset( - new ui::test::EventGenerator(widget_->GetNativeWindow())); + event_generator_ = + std::make_unique<ui::test::EventGenerator>(GetRootWindow(widget_)); } void ResizeAreaTest::TearDown() { diff --git a/chromium/ui/views/controls/scroll_view_unittest.cc b/chromium/ui/views/controls/scroll_view_unittest.cc index 4166c97c0bc..cb3a4c981e8 100644 --- a/chromium/ui/views/controls/scroll_view_unittest.cc +++ b/chromium/ui/views/controls/scroll_view_unittest.cc @@ -1025,8 +1025,9 @@ TEST_F(WidgetScrollViewTest, ScrollersOnRest) { const float y_offset = 3; const int kSteps = 1; const int kNnumFingers = 2; - generator.ScrollSequence(generator.current_location(), base::TimeDelta(), 0, - y_offset, kSteps, kNnumFingers); + generator.ScrollSequence(generator.current_screen_location(), + base::TimeDelta(), 0, y_offset, kSteps, + kNnumFingers); // Horizontal scroller should start fading out immediately. EXPECT_EQ(kMaxOpacity, bar[HORIZONTAL]->layer()->opacity()); @@ -1044,8 +1045,9 @@ TEST_F(WidgetScrollViewTest, ScrollersOnRest) { // Then, scrolling horizontally should show the horizontal scroller. The // vertical scroller should still be visible, running its hide timer. const float x_offset = 5; - generator.ScrollSequence(generator.current_location(), base::TimeDelta(), - x_offset, 0, kSteps, kNnumFingers); + generator.ScrollSequence(generator.current_screen_location(), + base::TimeDelta(), x_offset, 0, kSteps, + kNnumFingers); for (ScrollBarOrientation orientation : {HORIZONTAL, VERTICAL}) { EXPECT_EQ(kMaxOpacity, bar[orientation]->layer()->opacity()); EXPECT_EQ(kMaxOpacity, bar[orientation]->layer()->GetTargetOpacity()); diff --git a/chromium/ui/views/controls/scrollbar/base_scroll_bar.cc b/chromium/ui/views/controls/scrollbar/base_scroll_bar.cc index 0801c66123a..26ddd900913 100644 --- a/chromium/ui/views/controls/scrollbar/base_scroll_bar.cc +++ b/chromium/ui/views/controls/scrollbar/base_scroll_bar.cc @@ -84,8 +84,8 @@ void BaseScrollBar::ScrollByAmount(ScrollAmount amount) { void BaseScrollBar::ScrollToThumbPosition(int thumb_position, bool scroll_to_middle) { - contents_scroll_offset_ = - CalculateContentsOffset(thumb_position, scroll_to_middle); + contents_scroll_offset_ = CalculateContentsOffset( + static_cast<float>(thumb_position), scroll_to_middle); if (contents_scroll_offset_ < GetMinPosition()) { contents_scroll_offset_ = GetMinPosition(); } else if (contents_scroll_offset_ > GetMaxPosition()) { @@ -460,16 +460,17 @@ int BaseScrollBar::CalculateThumbPosition(int contents_scroll_offset) const { (contents_size_ - viewport_size_); } -int BaseScrollBar::CalculateContentsOffset(int thumb_position, +int BaseScrollBar::CalculateContentsOffset(float thumb_position, bool scroll_to_middle) const { - int thumb_size = thumb_->GetSize(); + float thumb_size = static_cast<float>(thumb_->GetSize()); int track_size = GetTrackSize(); if (track_size == thumb_size) return 0; if (scroll_to_middle) thumb_position = thumb_position - (thumb_size / 2); - return (thumb_position * (contents_size_ - viewport_size_)) / - (track_size - thumb_size); + float result = (thumb_position * (contents_size_ - viewport_size_)) / + (track_size - thumb_size); + return static_cast<int>(result); } } // namespace views diff --git a/chromium/ui/views/controls/scrollbar/base_scroll_bar.h b/chromium/ui/views/controls/scrollbar/base_scroll_bar.h index 9b447add2ee..f2c0a7d5c48 100644 --- a/chromium/ui/views/controls/scrollbar/base_scroll_bar.h +++ b/chromium/ui/views/controls/scrollbar/base_scroll_bar.h @@ -135,7 +135,7 @@ class VIEWS_EXPORT BaseScrollBar : public ScrollBar, // Calculates the current value of the contents offset (contents coordinates) // based on the current thumb position (thumb track coordinates). See // |ScrollToThumbPosition| for an explanation of |scroll_to_middle|. - int CalculateContentsOffset(int thumb_position, + int CalculateContentsOffset(float thumb_position, bool scroll_to_middle) const; // Called when the state of the thumb track changes (e.g. by the user diff --git a/chromium/ui/views/controls/scrollbar/scrollbar_unittest.cc b/chromium/ui/views/controls/scrollbar/scrollbar_unittest.cc index 75196bdef82..913a1e5669b 100644 --- a/chromium/ui/views/controls/scrollbar/scrollbar_unittest.cc +++ b/chromium/ui/views/controls/scrollbar/scrollbar_unittest.cc @@ -87,17 +87,7 @@ class ScrollBarViewsTest : public ViewsTestBase { std::unique_ptr<TestScrollBarController> controller_; }; -// TODO(dnicoara) Can't run the test on Windows since the scrollbar |Part| -// isn't handled in NativeTheme. -#if defined(OS_WIN) -#define MAYBE_Scrolling DISABLED_Scrolling -#define MAYBE_ScrollBarFitsToBottom DISABLED_ScrollBarFitsToBottom -#else -#define MAYBE_Scrolling Scrolling -#define MAYBE_ScrollBarFitsToBottom ScrollBarFitsToBottom -#endif - -TEST_F(ScrollBarViewsTest, MAYBE_Scrolling) { +TEST_F(ScrollBarViewsTest, Scrolling) { EXPECT_EQ(0, scrollbar_->GetPosition()); EXPECT_EQ(900, scrollbar_->GetMaxPosition()); EXPECT_EQ(0, scrollbar_->GetMinPosition()); @@ -135,7 +125,7 @@ TEST_F(ScrollBarViewsTest, MAYBE_Scrolling) { EXPECT_EQ(0, controller_->last_position); } -TEST_F(ScrollBarViewsTest, MAYBE_ScrollBarFitsToBottom) { +TEST_F(ScrollBarViewsTest, ScrollBarFitsToBottom) { scrollbar_->Update(100, 1999, 0); EXPECT_EQ(0, scrollbar_->GetPosition()); EXPECT_EQ(1899, scrollbar_->GetMaxPosition()); diff --git a/chromium/ui/views/controls/slider_unittest.cc b/chromium/ui/views/controls/slider_unittest.cc index 5ce176bd113..96900ebc17d 100644 --- a/chromium/ui/views/controls/slider_unittest.cc +++ b/chromium/ui/views/controls/slider_unittest.cc @@ -18,12 +18,14 @@ #include "ui/events/gesture_event_details.h" #include "ui/events/keycodes/keyboard_codes.h" #include "ui/events/test/event_generator.h" +#include "ui/views/accessibility/ax_event_manager.h" +#include "ui/views/accessibility/ax_event_observer.h" #include "ui/views/test/slider_test_api.h" -#include "ui/views/test/test_views_delegate.h" #include "ui/views/test/views_test_base.h" #include "ui/views/view.h" #include "ui/views/widget/widget.h" #include "ui/views/widget/widget_delegate.h" +#include "ui/views/widget/widget_utils.h" namespace { @@ -117,6 +119,28 @@ void TestSliderListener::SliderDragEnded(views::Slider* sender) { last_drag_ended_epoch_ = ++last_event_epoch_; } +class TestAXEventObserver : public views::AXEventObserver { + public: + TestAXEventObserver() { views::AXEventManager::Get()->AddObserver(this); } + + ~TestAXEventObserver() override { + views::AXEventManager::Get()->RemoveObserver(this); + } + + bool value_changed() const { return value_changed_; } + + // views::AXEventObserver: + void OnViewEvent(views::View* view, ax::mojom::Event event_type) override { + if (event_type == ax::mojom::Event::kValueChanged) + value_changed_ = true; + } + + private: + bool value_changed_ = false; + + DISALLOW_COPY_AND_ASSIGN(TestAXEventObserver); +}; + } // namespace namespace views { @@ -124,22 +148,8 @@ namespace views { // Base test fixture for Slider tests. class SliderTest : public views::ViewsTestBase { public: - class SliderTestViewsDelegate : public views::TestViewsDelegate { - public: - bool has_value_changed() { return has_value_changed_; } - - private: - void NotifyAccessibilityEvent(View* view, - ax::mojom::Event event_type) override { - if (event_type == ax::mojom::Event::kValueChanged) - has_value_changed_ = true; - } - - bool has_value_changed_ = false; - }; - - SliderTest(); - ~SliderTest() override; + SliderTest() = default; + ~SliderTest() override = default; protected: Slider* slider() { @@ -168,43 +178,26 @@ class SliderTest : public views::ViewsTestBase { return event_generator_.get(); } - SliderTestViewsDelegate* delegate() { return delegate_; } - private: // The Slider to be tested. - Slider* slider_; + Slider* slider_ = nullptr; // A simple SliderListener test double. TestSliderListener slider_listener_; // Stores the default locale at test setup so it can be restored // during test teardown. std::string default_locale_; // The maximum x value within the bounds of the slider. - int max_x_; + int max_x_ = 0; // The maximum y value within the bounds of the slider. - int max_y_; + int max_y_ = 0; // The widget container for the slider being tested. - views::Widget* widget_; + views::Widget* widget_ = nullptr; // An event generator. std::unique_ptr<ui::test::EventGenerator> event_generator_; - // A TestViewsDelegate that intercepts accessibility notifications. Weak. - SliderTestViewsDelegate* delegate_; DISALLOW_COPY_AND_ASSIGN(SliderTest); }; -SliderTest::SliderTest() - : slider_(NULL), - default_locale_(), - max_x_(0), - max_y_(0), - delegate_(new SliderTestViewsDelegate()) { - std::unique_ptr<views::TestViewsDelegate> delegate(delegate_); - set_views_delegate(std::move(delegate)); -} - -SliderTest::~SliderTest() { -} - void SliderTest::SetUp() { views::ViewsTestBase::SetUp(); @@ -225,8 +218,8 @@ void SliderTest::SetUp() { widget_->SetContentsView(slider_); widget_->Show(); - event_generator_.reset( - new ui::test::EventGenerator(widget_->GetNativeWindow())); + event_generator_ = + std::make_unique<ui::test::EventGenerator>(GetRootWindow(widget_)); } void SliderTest::TearDown() { @@ -400,7 +393,8 @@ TEST_F(SliderTest, SliderListenerEventsForMultiFingerScrollGesture) { // Verifies the correct SliderListener events are raised for an accessible // slider. TEST_F(SliderTest, SliderRaisesA11yEvents) { - EXPECT_FALSE(delegate()->has_value_changed()); + TestAXEventObserver observer; + EXPECT_FALSE(observer.value_changed()); // First, detach/reattach the slider without setting value. // Temporarily detach the slider. @@ -409,18 +403,18 @@ TEST_F(SliderTest, SliderRaisesA11yEvents) { // Re-attachment should cause nothing to get fired. root_view->AddChildView(slider()); - EXPECT_FALSE(delegate()->has_value_changed()); + EXPECT_FALSE(observer.value_changed()); // Now, set value before reattaching. root_view->RemoveChildView(slider()); // Value changes won't trigger accessibility events before re-attachment. slider()->SetValue(22); - EXPECT_FALSE(delegate()->has_value_changed()); + EXPECT_FALSE(observer.value_changed()); // Re-attachment should trigger the value change. root_view->AddChildView(slider()); - EXPECT_TRUE(delegate()->has_value_changed()); + EXPECT_TRUE(observer.value_changed()); } #endif // !defined(OS_MACOSX) || defined(USE_AURA) diff --git a/chromium/ui/views/controls/tabbed_pane/tabbed_pane.cc b/chromium/ui/views/controls/tabbed_pane/tabbed_pane.cc index 96309bc1709..881e4df535d 100644 --- a/chromium/ui/views/controls/tabbed_pane/tabbed_pane.cc +++ b/chromium/ui/views/controls/tabbed_pane/tabbed_pane.cc @@ -157,7 +157,7 @@ Tab::Tab(TabbedPane* tabbed_pane, const base::string16& title, View* contents) // Use leaf so that name is spoken by screen reader without exposing the // children. - GetViewAccessibility().OverrideIsLeaf(); + GetViewAccessibility().OverrideIsLeaf(true); } Tab::~Tab() {} diff --git a/chromium/ui/views/controls/tabbed_pane/tabbed_pane_accessibility_mac_unittest.mm b/chromium/ui/views/controls/tabbed_pane/tabbed_pane_accessibility_mac_unittest.mm index d0ae166c1cf..8f08e724af2 100644 --- a/chromium/ui/views/controls/tabbed_pane/tabbed_pane_accessibility_mac_unittest.mm +++ b/chromium/ui/views/controls/tabbed_pane/tabbed_pane_accessibility_mac_unittest.mm @@ -56,7 +56,8 @@ class TabbedPaneAccessibilityMacTest : public WidgetTest { id A11yElementAtPoint(const gfx::Point& point) { // Accessibility hit tests come in Cocoa screen coordinates. NSPoint ns_point = gfx::ScreenPointToNSPoint(point); - return [widget_->GetNativeWindow() accessibilityHitTest:ns_point]; + return [widget_->GetNativeWindow().GetNativeNSWindow() + accessibilityHitTest:ns_point]; } gfx::Point TabCenterPoint(int index) { diff --git a/chromium/ui/views/controls/table/table_view_unittest.cc b/chromium/ui/views/controls/table/table_view_unittest.cc index 88df9b24fd8..df04dc1a96e 100644 --- a/chromium/ui/views/controls/table/table_view_unittest.cc +++ b/chromium/ui/views/controls/table/table_view_unittest.cc @@ -20,6 +20,7 @@ #include "ui/views/test/views_test_base.h" #include "ui/views/widget/widget.h" #include "ui/views/widget/widget_delegate.h" +#include "ui/views/widget/widget_utils.h" // Put the tests in the views namespace to make it easier to declare them as // friend classes. @@ -271,15 +272,15 @@ class TableViewTest : public ViewsTestBase { } void ClickOnRow(int row, int flags) { - ui::test::EventGenerator generator(widget_->GetNativeWindow()); + ui::test::EventGenerator generator(GetRootWindow(widget_.get())); generator.set_assume_window_at_origin(false); generator.set_flags(flags); - generator.set_current_location(GetPointForRow(row)); + generator.set_current_screen_location(GetPointForRow(row)); generator.PressLeftButton(); } void TapOnRow(int row) { - ui::test::EventGenerator generator(widget_->GetNativeWindow()); + ui::test::EventGenerator generator(GetRootWindow(widget_.get())); generator.GestureTapAt(GetPointForRow(row)); } @@ -301,7 +302,7 @@ class TableViewTest : public ViewsTestBase { } void PressKey(ui::KeyboardCode code) { - ui::test::EventGenerator generator(widget_->GetNativeWindow()); + ui::test::EventGenerator generator(GetRootWindow(widget_.get())); generator.PressKey(code, ui::EF_NONE); } diff --git a/chromium/ui/views/controls/textfield/textfield.h b/chromium/ui/views/controls/textfield/textfield.h index 4d20d983432..f5d0914d43e 100644 --- a/chromium/ui/views/controls/textfield/textfield.h +++ b/chromium/ui/views/controls/textfield/textfield.h @@ -21,7 +21,7 @@ #include "ui/base/ime/text_input_client.h" #include "ui/base/ime/text_input_type.h" #include "ui/base/models/simple_menu_model.h" -#include "ui/base/touch/touch_editing_controller.h" +#include "ui/base/pointer/touch_editing_controller.h" #include "ui/events/keycodes/keyboard_codes.h" #include "ui/gfx/font_list.h" #include "ui/gfx/range/range.h" diff --git a/chromium/ui/views/controls/textfield/textfield_model.cc b/chromium/ui/views/controls/textfield/textfield_model.cc index 1bfea07108c..6c63283e596 100644 --- a/chromium/ui/views/controls/textfield/textfield_model.cc +++ b/chromium/ui/views/controls/textfield/textfield_model.cc @@ -270,7 +270,7 @@ gfx::Range GetFirstEmphasizedRange(const ui::CompositionText& composition) { // the default kill ring size of 1 (i.e. a single buffer) is assumed. base::string16* GetKillBuffer() { static base::NoDestructor<base::string16> kill_buffer; - DCHECK(base::MessageLoopForUI::IsCurrent()); + DCHECK(base::MessageLoopCurrentForUI::IsSet()); return kill_buffer.get(); } diff --git a/chromium/ui/views/controls/textfield/textfield_unittest.cc b/chromium/ui/views/controls/textfield/textfield_unittest.cc index be57d658a24..879ebf11a83 100644 --- a/chromium/ui/views/controls/textfield/textfield_unittest.cc +++ b/chromium/ui/views/controls/textfield/textfield_unittest.cc @@ -56,6 +56,7 @@ #include "ui/views/test/views_test_base.h" #include "ui/views/test/widget_test.h" #include "ui/views/widget/widget.h" +#include "ui/views/widget/widget_utils.h" #include "url/gurl.h" #if defined(OS_WIN) @@ -492,8 +493,8 @@ class TextfieldTest : public ViewsTestBase, public TextfieldController { widget_->Show(); textfield_->RequestFocus(); - event_generator_.reset( - new ui::test::EventGenerator(widget_->GetNativeWindow())); + event_generator_ = + std::make_unique<ui::test::EventGenerator>(GetRootWindow(widget_)); event_generator_->set_target(ui::test::EventGenerator::Target::WINDOW); } ui::MenuModel* GetContextMenuModel() { @@ -3074,7 +3075,7 @@ TEST_F(TextfieldTest, CursorBlinkRestartsOnInsertOrReplace) { TEST_F(TextfieldTest, VirtualKeyboardFocusEnsureCaretNotInRect) { InitTextfield(); - aura::Window* root_window = widget_->GetNativeView()->GetRootWindow(); + aura::Window* root_window = GetRootWindow(widget_); int keyboard_height = 200; gfx::Rect root_bounds = root_window->bounds(); gfx::Rect orig_widget_bounds = gfx::Rect(0, 300, 400, 200); diff --git a/chromium/ui/views/controls/tree/tree_view.cc b/chromium/ui/views/controls/tree/tree_view.cc index b8b07ce5377..b57f0535ac0 100644 --- a/chromium/ui/views/controls/tree/tree_view.cc +++ b/chromium/ui/views/controls/tree/tree_view.cc @@ -11,7 +11,6 @@ #include "components/vector_icons/vector_icons.h" #include "ui/accessibility/ax_node_data.h" #include "ui/base/ime/input_method.h" -#include "ui/base/material_design/material_design_controller.h" #include "ui/base/resource/resource_bundle.h" #include "ui/events/event.h" #include "ui/events/keycodes/keyboard_codes.h" diff --git a/chromium/ui/views/controls/views_text_services_context_menu_mac.mm b/chromium/ui/views/controls/views_text_services_context_menu_mac.mm index c7731fb7547..0ae218c0f28 100644 --- a/chromium/ui/views/controls/views_text_services_context_menu_mac.mm +++ b/chromium/ui/views/controls/views_text_services_context_menu_mac.mm @@ -114,7 +114,7 @@ class ViewsTextServicesContextMenuMac gfx::DecoratedText text; if (client()->GetWordLookupDataFromSelection(&text, &baseline_point)) { Widget* widget = client()->GetWidget(); - gfx::NativeView view = widget->GetNativeView(); + NSView* view = widget->GetNativeView().GetNativeNSView(); views::View::ConvertPointToTarget(client(), widget->GetRootView(), &baseline_point); diff --git a/chromium/ui/views/controls/webview/unhandled_keyboard_event_handler.cc b/chromium/ui/views/controls/webview/unhandled_keyboard_event_handler.cc index d7bd93654d1..80ce19eb64d 100644 --- a/chromium/ui/views/controls/webview/unhandled_keyboard_event_handler.cc +++ b/chromium/ui/views/controls/webview/unhandled_keyboard_event_handler.cc @@ -16,12 +16,12 @@ UnhandledKeyboardEventHandler::UnhandledKeyboardEventHandler() UnhandledKeyboardEventHandler::~UnhandledKeyboardEventHandler() { } -void UnhandledKeyboardEventHandler::HandleKeyboardEvent( +bool UnhandledKeyboardEventHandler::HandleKeyboardEvent( const content::NativeWebKeyboardEvent& event, FocusManager* focus_manager) { if (!focus_manager) { NOTREACHED(); - return; + return false; } // Previous calls to TranslateMessage can generate Char events as well as // RawKeyDown events, even if the latter triggered an accelerator. In these @@ -29,7 +29,7 @@ void UnhandledKeyboardEventHandler::HandleKeyboardEvent( if (event.GetType() == blink::WebInputEvent::kChar && ignore_next_char_event_) { ignore_next_char_event_ = false; - return; + return false; } // It's necessary to reset this flag, because a RawKeyDown event may not // always generate a Char event. @@ -45,9 +45,8 @@ void UnhandledKeyboardEventHandler::HandleKeyboardEvent( // set the flag and fix it if no event was handled. ignore_next_char_event_ = true; - if (focus_manager->ProcessAccelerator(accelerator)) { - return; - } + if (focus_manager->ProcessAccelerator(accelerator)) + return true; // ProcessAccelerator didn't handle the accelerator, so we know both // that |this| is still valid, and that we didn't want to set the flag. @@ -55,7 +54,9 @@ void UnhandledKeyboardEventHandler::HandleKeyboardEvent( } if (event.os_event && !event.skip_in_browser) - HandleNativeKeyboardEvent(event.os_event, focus_manager); + return HandleNativeKeyboardEvent(event.os_event, focus_manager); + + return false; } } // namespace views diff --git a/chromium/ui/views/controls/webview/unhandled_keyboard_event_handler.h b/chromium/ui/views/controls/webview/unhandled_keyboard_event_handler.h index 8f06105703c..67cb6563e72 100644 --- a/chromium/ui/views/controls/webview/unhandled_keyboard_event_handler.h +++ b/chromium/ui/views/controls/webview/unhandled_keyboard_event_handler.h @@ -23,12 +23,12 @@ class WEBVIEW_EXPORT UnhandledKeyboardEventHandler { UnhandledKeyboardEventHandler(); ~UnhandledKeyboardEventHandler(); - void HandleKeyboardEvent(const content::NativeWebKeyboardEvent& event, + bool HandleKeyboardEvent(const content::NativeWebKeyboardEvent& event, FocusManager* focus_manager); private: // Platform specific handling for unhandled keyboard events. - static void HandleNativeKeyboardEvent(gfx::NativeEvent event, + static bool HandleNativeKeyboardEvent(gfx::NativeEvent event, FocusManager* focus_manager); // Whether to ignore the next Char keyboard event. diff --git a/chromium/ui/views/controls/webview/unhandled_keyboard_event_handler_default.cc b/chromium/ui/views/controls/webview/unhandled_keyboard_event_handler_default.cc index 8d30e36ad8a..f6cb4a38d17 100644 --- a/chromium/ui/views/controls/webview/unhandled_keyboard_event_handler_default.cc +++ b/chromium/ui/views/controls/webview/unhandled_keyboard_event_handler_default.cc @@ -10,10 +10,10 @@ namespace views { // static -void UnhandledKeyboardEventHandler::HandleNativeKeyboardEvent( +bool UnhandledKeyboardEventHandler::HandleNativeKeyboardEvent( gfx::NativeEvent event, FocusManager* focus_manager) { - focus_manager->OnKeyEvent(*static_cast<ui::KeyEvent*>(event)); + return !focus_manager->OnKeyEvent(*(event->AsKeyEvent())); } } // namespace views diff --git a/chromium/ui/views/controls/webview/unhandled_keyboard_event_handler_mac.mm b/chromium/ui/views/controls/webview/unhandled_keyboard_event_handler_mac.mm index e87f913ff61..5e004e4190e 100644 --- a/chromium/ui/views/controls/webview/unhandled_keyboard_event_handler_mac.mm +++ b/chromium/ui/views/controls/webview/unhandled_keyboard_event_handler_mac.mm @@ -10,11 +10,12 @@ namespace views { // static -void UnhandledKeyboardEventHandler::HandleNativeKeyboardEvent( +bool UnhandledKeyboardEventHandler::HandleNativeKeyboardEvent( gfx::NativeEvent event, FocusManager* focus_manager) { [[base::mac::ObjCCastStrict<NativeWidgetMacNSWindow>([event window]) commandDispatcher] redispatchKeyEvent:event]; + return true; } } // namespace views diff --git a/chromium/ui/views/controls/webview/unhandled_keyboard_event_handler_win.cc b/chromium/ui/views/controls/webview/unhandled_keyboard_event_handler_win.cc index e31aabbf901..04f7604c970 100644 --- a/chromium/ui/views/controls/webview/unhandled_keyboard_event_handler_win.cc +++ b/chromium/ui/views/controls/webview/unhandled_keyboard_event_handler_win.cc @@ -9,13 +9,14 @@ namespace views { // static -void UnhandledKeyboardEventHandler::HandleNativeKeyboardEvent( +bool UnhandledKeyboardEventHandler::HandleNativeKeyboardEvent( gfx::NativeEvent event, FocusManager* focus_manager) { // Any unhandled keyboard/character messages should be defproced. // This allows stuff like F10, etc to work correctly. const MSG& message(event->native_event()); DefWindowProc(message.hwnd, message.message, message.wParam, message.lParam); + return true; } } // namespace views diff --git a/chromium/ui/views/controls/webview/web_dialog_view.cc b/chromium/ui/views/controls/webview/web_dialog_view.cc index 56fc98b2840..e2023e4030f 100644 --- a/chromium/ui/views/controls/webview/web_dialog_view.cc +++ b/chromium/ui/views/controls/webview/web_dialog_view.cc @@ -287,12 +287,13 @@ void WebDialogView::SetContentsBounds(WebContents* source, // A simplified version of BrowserView::HandleKeyboardEvent(). // We don't handle global keyboard shortcuts here, but that's fine since // they're all browser-specific. (This may change in the future.) -void WebDialogView::HandleKeyboardEvent(content::WebContents* source, +bool WebDialogView::HandleKeyboardEvent(content::WebContents* source, const NativeWebKeyboardEvent& event) { if (!event.os_event) - return; + return false; GetWidget()->native_widget_private()->RepostNativeEvent(event.os_event); + return true; } void WebDialogView::CloseContents(WebContents* source) { diff --git a/chromium/ui/views/controls/webview/web_dialog_view.h b/chromium/ui/views/controls/webview/web_dialog_view.h index 29d18cb2e1f..6467993163f 100644 --- a/chromium/ui/views/controls/webview/web_dialog_view.h +++ b/chromium/ui/views/controls/webview/web_dialog_view.h @@ -97,7 +97,7 @@ class WEBVIEW_EXPORT WebDialogView : public views::ClientView, // Overridden from content::WebContentsDelegate: void SetContentsBounds(content::WebContents* source, const gfx::Rect& bounds) override; - void HandleKeyboardEvent( + bool HandleKeyboardEvent( content::WebContents* source, const content::NativeWebKeyboardEvent& event) override; void CloseContents(content::WebContents* source) override; diff --git a/chromium/ui/views/controls/webview/webview.cc b/chromium/ui/views/controls/webview/webview.cc index 2a4cc7e8938..15e497fcf41 100644 --- a/chromium/ui/views/controls/webview/webview.cc +++ b/chromium/ui/views/controls/webview/webview.cc @@ -129,6 +129,10 @@ void WebView::SetCrashedOverlayView(View* crashed_overlay_view) { if (crashed_overlay_view_) { RemoveChildView(crashed_overlay_view_); + // Show the hosted web contents view iff the crashed + // overlay is NOT showing, to ensure hit testing is + // correct on Mac. See https://crbug.com/896508 + holder_->SetVisible(true); if (!crashed_overlay_view_->owned_by_client()) delete crashed_overlay_view_; } @@ -136,6 +140,7 @@ void WebView::SetCrashedOverlayView(View* crashed_overlay_view) { crashed_overlay_view_ = crashed_overlay_view; if (crashed_overlay_view_) { AddChildView(crashed_overlay_view_); + holder_->SetVisible(false); crashed_overlay_view_->SetBoundsRect(gfx::Rect(size())); } diff --git a/chromium/ui/views/controls/webview/webview_unittest.cc b/chromium/ui/views/controls/webview/webview_unittest.cc index f1d0ce7758d..27be01b37a9 100644 --- a/chromium/ui/views/controls/webview/webview_unittest.cc +++ b/chromium/ui/views/controls/webview/webview_unittest.cc @@ -17,6 +17,7 @@ #include "content/public/test/mock_render_process_host.h" #include "content/public/test/test_browser_context.h" #include "content/public/test/test_browser_thread_bundle.h" +#include "content/public/test/test_renderer_host.h" #include "content/public/test/web_contents_tester.h" #include "content/test/test_content_browser_client.h" #include "ui/events/event.h" @@ -187,6 +188,7 @@ class WebViewUnitTest : public views::test::WidgetTest { private: content::TestBrowserThreadBundle test_browser_thread_bundle_; + content::RenderViewHostTestEnabler rvh_enabler_; std::unique_ptr<content::TestBrowserContext> browser_context_; content::TestContentBrowserClient test_browser_client_; std::unique_ptr<views::WebView::ScopedWebContentsCreatorForTesting> |