Commit 52b60b4e authored by Christos Froussios's avatar Christos Froussios Committed by Commit Bot

Revert "Make dialogs not draggable (Widgets with MODAL_TYPE_NONE remain"

This reverts commit 0a25dcde.

Reason for revert: Suspected of making DialogTest.InitialFocusWithDeactivatedWidget very flaky on Mac

Bug:949911

Original change's description:
> Make dialogs not draggable (Widgets with MODAL_TYPE_NONE remain
> draggable).
> 
> This change makes the title area of a dialog act as the client. Also
> removed code so system menu does not appear on dialogs. This makes more
> sense since the system dialog has properties such as move (which is now
> illegal, also it doesn't work), and resize (illegal), and close with
> hints of using Alt+F4 (Alt+F4 does not close dialogs).
> 
> Bug: 930782
> Change-Id: I0956b85df2c3059af7b38b63ccf8774ccd150158
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1536198
> Commit-Queue: Charlene Yan <cyan@chromium.org>
> Reviewed-by: Trent Apted <tapted@chromium.org>
> Reviewed-by: Bret Sepulveda <bsep@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#647985}

TBR=tapted@chromium.org,bsep@chromium.org,cyan@chromium.org

Change-Id: I800192f4a32822b68b9027f0b7b1ebcab9e22684
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 930782
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1553544Reviewed-by: default avatarChristos Froussios <cfroussios@chromium.org>
Commit-Queue: Christos Froussios <cfroussios@chromium.org>
Cr-Commit-Position: refs/heads/master@{#648116}
parent c4f9ca95
...@@ -286,7 +286,7 @@ TEST_F(BubbleDialogDelegateViewTest, NonClientHitTest) { ...@@ -286,7 +286,7 @@ TEST_F(BubbleDialogDelegateViewTest, NonClientHitTest) {
const int hit; const int hit;
} kTestCases[] = { } kTestCases[] = {
#if defined(OS_WIN) #if defined(OS_WIN)
{0, is_aero_glass_enabled ? HTTRANSPARENT : HTCAPTION}, {0, is_aero_glass_enabled ? HTTRANSPARENT : HTNOWHERE},
#else #else
{0, HTTRANSPARENT}, {0, HTTRANSPARENT},
#endif #endif
......
...@@ -181,6 +181,19 @@ int BubbleFrameView::NonClientHitTest(const gfx::Point& point) { ...@@ -181,6 +181,19 @@ int BubbleFrameView::NonClientHitTest(const gfx::Point& point) {
if (close_->visible() && close_->GetMirroredBounds().Contains(point)) if (close_->visible() && close_->GetMirroredBounds().Contains(point))
return HTCLOSE; return HTCLOSE;
// Allow dialogs to show the system menu and be dragged.
if (GetWidget()->widget_delegate()->AsDialogDelegate() &&
!GetWidget()->widget_delegate()->AsBubbleDialogDelegate()) {
gfx::Rect bounds(GetContentsBounds());
bounds.Inset(title_margins_);
gfx::Rect sys_rect(0, 0, bounds.x(), bounds.y());
sys_rect.set_origin(gfx::Point(GetMirroredXForRect(sys_rect), 0));
if (sys_rect.Contains(point))
return HTSYSMENU;
if (point.y() < title()->bounds().bottom())
return HTCAPTION;
}
// Convert to RRectF to accurately represent the rounded corners of the // Convert to RRectF to accurately represent the rounded corners of the
// dialog and allow events to pass through the shadows. // dialog and allow events to pass through the shadows.
gfx::RRectF round_contents_bounds(gfx::RectF(GetContentsBounds()), gfx::RRectF round_contents_bounds(gfx::RectF(GetContentsBounds()),
...@@ -191,16 +204,6 @@ int BubbleFrameView::NonClientHitTest(const gfx::Point& point) { ...@@ -191,16 +204,6 @@ int BubbleFrameView::NonClientHitTest(const gfx::Point& point) {
if (!round_contents_bounds.Contains(rectf_point)) if (!round_contents_bounds.Contains(rectf_point))
return HTTRANSPARENT; return HTTRANSPARENT;
if (HasTitle() && point.y() < title()->bounds().bottom()) {
auto* dialog_delegate = GetWidget()->widget_delegate()->AsDialogDelegate();
// Allow the dialog to be dragged if it is not modal. This can happen if the
// dialog has no parent browser window.
if (dialog_delegate &&
dialog_delegate->GetModalType() == ui::MODAL_TYPE_NONE) {
return HTCAPTION;
}
}
return GetWidget()->client_view()->NonClientHitTest(point); return GetWidget()->client_view()->NonClientHitTest(point);
} }
......
...@@ -98,13 +98,6 @@ class TestDialog : public DialogDelegateView { ...@@ -98,13 +98,6 @@ class TestDialog : public DialogDelegateView {
views::Textfield* input() { return input_; } views::Textfield* input() { return input_; }
ui::ModalType GetModalType() const override {
return modal_type_none_ ? ui::MODAL_TYPE_NONE : ui::MODAL_TYPE_WINDOW;
}
void set_modal_type_none(bool modal_type_none) {
modal_type_none_ = modal_type_none;
}
private: private:
views::Textfield* input_; views::Textfield* input_;
bool canceled_ = false; bool canceled_ = false;
...@@ -116,7 +109,6 @@ class TestDialog : public DialogDelegateView { ...@@ -116,7 +109,6 @@ class TestDialog : public DialogDelegateView {
bool show_close_button_ = true; bool show_close_button_ = true;
bool should_handle_escape_ = false; bool should_handle_escape_ = false;
int dialog_buttons_ = ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL; int dialog_buttons_ = ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL;
bool modal_type_none_ = false;
DISALLOW_COPY_AND_ASSIGN(TestDialog); DISALLOW_COPY_AND_ASSIGN(TestDialog);
}; };
...@@ -224,22 +216,22 @@ TEST_F(DialogTest, HitTest_HiddenTitle) { ...@@ -224,22 +216,22 @@ TEST_F(DialogTest, HitTest_HiddenTitle) {
const NonClientView* view = dialog()->GetWidget()->non_client_view(); const NonClientView* view = dialog()->GetWidget()->non_client_view();
BubbleFrameView* frame = static_cast<BubbleFrameView*>(view->frame_view()); BubbleFrameView* frame = static_cast<BubbleFrameView*>(view->frame_view());
constexpr struct { struct {
const int point; const int point;
const int hit; const int hit;
} kCases[] = { } cases[] = {
{0, HTTRANSPARENT}, {0, HTSYSMENU},
{10, HTNOWHERE}, {10, HTSYSMENU},
{20, HTNOWHERE}, {20, HTNOWHERE},
{50, HTCLIENT /* Space is reserved for the close button. */}, {50, HTCLIENT /* Space is reserved for the close button. */},
{60, HTCLIENT}, {60, HTCLIENT},
{1000, HTNOWHERE}, {1000, HTNOWHERE},
}; };
for (const auto test_case : kCases) { for (size_t i = 0; i < base::size(cases); ++i) {
gfx::Point point(test_case.point, test_case.point); gfx::Point point(cases[i].point, cases[i].point);
EXPECT_EQ(test_case.hit, frame->NonClientHitTest(point)) EXPECT_EQ(cases[i].hit, frame->NonClientHitTest(point))
<< " at point " << test_case.point; << " case " << i << " at point " << cases[i].point;
} }
} }
...@@ -251,18 +243,18 @@ TEST_F(DialogTest, HitTest_HiddenTitleNoCloseButton) { ...@@ -251,18 +243,18 @@ TEST_F(DialogTest, HitTest_HiddenTitleNoCloseButton) {
const NonClientView* view = dialog()->GetWidget()->non_client_view(); const NonClientView* view = dialog()->GetWidget()->non_client_view();
BubbleFrameView* frame = static_cast<BubbleFrameView*>(view->frame_view()); BubbleFrameView* frame = static_cast<BubbleFrameView*>(view->frame_view());
constexpr struct { struct {
const int point; const int point;
const int hit; const int hit;
} kCases[] = { } cases[] = {
{0, HTTRANSPARENT}, {10, HTCLIENT}, {20, HTCLIENT}, {0, HTSYSMENU}, {10, HTSYSMENU}, {20, HTCLIENT},
{50, HTCLIENT}, {60, HTCLIENT}, {1000, HTNOWHERE}, {50, HTCLIENT}, {60, HTCLIENT}, {1000, HTNOWHERE},
}; };
for (const auto test_case : kCases) { for (size_t i = 0; i < base::size(cases); ++i) {
gfx::Point point(test_case.point, test_case.point); gfx::Point point(cases[i].point, cases[i].point);
EXPECT_EQ(test_case.hit, frame->NonClientHitTest(point)) EXPECT_EQ(cases[i].hit, frame->NonClientHitTest(point))
<< " at point " << test_case.point; << " case " << i << " at point " << cases[i].point;
} }
} }
...@@ -274,43 +266,18 @@ TEST_F(DialogTest, HitTest_WithTitle) { ...@@ -274,43 +266,18 @@ TEST_F(DialogTest, HitTest_WithTitle) {
dialog()->GetWidget()->LayoutRootViewIfNecessary(); dialog()->GetWidget()->LayoutRootViewIfNecessary();
BubbleFrameView* frame = static_cast<BubbleFrameView*>(view->frame_view()); BubbleFrameView* frame = static_cast<BubbleFrameView*>(view->frame_view());
constexpr struct { struct {
const int point;
const int hit;
} kCases[] = {
{0, HTTRANSPARENT}, {10, HTNOWHERE}, {20, HTNOWHERE},
{50, HTCLIENT}, {60, HTCLIENT}, {1000, HTNOWHERE},
};
for (const auto test_case : kCases) {
gfx::Point point(test_case.point, test_case.point);
EXPECT_EQ(test_case.hit, frame->NonClientHitTest(point))
<< " at point " << test_case.point;
}
}
TEST_F(DialogTest, HitTest_ModalTypeNone_WithTitle) {
// Ensure that BubbleFrameView hit-tests as expected when the title is shown
// and the modal type is none.
const NonClientView* view = dialog()->GetWidget()->non_client_view();
dialog()->set_title(base::ASCIIToUTF16("Title"));
dialog()->GetWidget()->UpdateWindowTitle();
dialog()->set_modal_type_none(true);
dialog()->GetWidget()->LayoutRootViewIfNecessary();
BubbleFrameView* frame = static_cast<BubbleFrameView*>(view->frame_view());
constexpr struct {
const int point; const int point;
const int hit; const int hit;
} kCases[] = { } cases[] = {
{0, HTTRANSPARENT}, {10, HTCAPTION}, {20, HTCAPTION}, {0, HTSYSMENU}, {10, HTSYSMENU}, {20, HTCAPTION},
{50, HTCLIENT}, {60, HTCLIENT}, {1000, HTNOWHERE}, {50, HTCLIENT}, {60, HTCLIENT}, {1000, HTNOWHERE},
}; };
for (const auto test_case : kCases) { for (size_t i = 0; i < base::size(cases); ++i) {
gfx::Point point(test_case.point, test_case.point); gfx::Point point(cases[i].point, cases[i].point);
EXPECT_EQ(test_case.hit, frame->NonClientHitTest(point)) EXPECT_EQ(cases[i].hit, frame->NonClientHitTest(point))
<< " at point " << test_case.point; << " at point " << cases[i].point;
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment