Commit ee2cebc4 authored by msw@chromium.org's avatar msw@chromium.org

Revise NativeWidgetWin focus hack; force focus on restore.

Remove NativeWidgetWin::RestoreFocusOnActivate's built-in hack.
Always use kReasonFocusRestore in FocusManager::RestoreFocusedView().
Always run FocusManager::SetFocusedViewWithReason with kReasonFocusRestore.
(this clears and resets child HWND focus, needed for the hack)

Fixes initial omnibox focus on Win with --enable-views-textfield; plus:
interactive_ui_tests.exe --gtest_filter=BrowserFocusTest.ClickingMovesFocus --enable-views-textfield
views_unittests.exe --gtest_filter=BubbleDelegateTest.InitiallyFocusedView --enable-views-textfield
(test updated to have the proper expectation, removing a non-Aura Win hack)
Doesn't regress child HWND focus like the previous attempt: http://crrev.com/186235

Remove SearchTextfieldView Textfield subclass; make RequestFocus non-virtual.
TODO(followup): Polish find-bar focus behavior as needed in followups.

BUG=125976,131660,224591,225963
TEST=View and web-contents focus restoration on minimize/restore; repro steps in bugs. Find bar works as expected (may need attention in followups). Tests pass with --enable-views-textfield.
R=ben@chromium.org

Review URL: https://chromiumcodereview.appspot.com/13227002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@192493 0039d316-1c4b-4281-b951-d872f2087c98
parent 537cfb7c
...@@ -81,7 +81,7 @@ FindBarView::FindBarView(FindBarHost* host) ...@@ -81,7 +81,7 @@ FindBarView::FindBarView(FindBarHost* host)
set_id(VIEW_ID_FIND_IN_PAGE); set_id(VIEW_ID_FIND_IN_PAGE);
ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance(); ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
find_text_ = new SearchTextfieldView(); find_text_ = new views::Textfield();
find_text_->set_id(VIEW_ID_FIND_IN_PAGE_TEXT_FIELD); find_text_->set_id(VIEW_ID_FIND_IN_PAGE_TEXT_FIELD);
find_text_->SetFont(rb.GetFont(ui::ResourceBundle::BaseFont)); find_text_->SetFont(rb.GetFont(ui::ResourceBundle::BaseFont));
find_text_->set_default_width_in_chars(kDefaultCharWidth); find_text_->set_default_width_in_chars(kDefaultCharWidth);
...@@ -498,19 +498,6 @@ bool FindBarView::FocusForwarderView::OnMousePressed( ...@@ -498,19 +498,6 @@ bool FindBarView::FocusForwarderView::OnMousePressed(
return true; return true;
} }
FindBarView::SearchTextfieldView::SearchTextfieldView() {
}
FindBarView::SearchTextfieldView::~SearchTextfieldView() {
}
void FindBarView::SearchTextfieldView::RequestFocus() {
if (HasFocus())
return;
views::View::RequestFocus();
SelectAll(true);
}
FindBarHost* FindBarView::find_bar_host() const { FindBarHost* FindBarView::find_bar_host() const {
return static_cast<FindBarHost*>(host()); return static_cast<FindBarHost*>(host());
} }
......
...@@ -110,25 +110,12 @@ class FindBarView : public DropdownBarView, ...@@ -110,25 +110,12 @@ class FindBarView : public DropdownBarView,
DISALLOW_COPY_AND_ASSIGN(FocusForwarderView); DISALLOW_COPY_AND_ASSIGN(FocusForwarderView);
}; };
// A wrapper of views::TextField that allows us to select all text when we
// get focus. Represents the text field where the user enters a search term.
class SearchTextfieldView : public views::Textfield {
public:
SearchTextfieldView();
virtual ~SearchTextfieldView();
virtual void RequestFocus() OVERRIDE;
private:
DISALLOW_COPY_AND_ASSIGN(SearchTextfieldView);
};
// Returns the OS-specific view for the find bar that acts as an intermediary // Returns the OS-specific view for the find bar that acts as an intermediary
// between us and the WebContentsView. // between us and the WebContentsView.
FindBarHost* find_bar_host() const; FindBarHost* find_bar_host() const;
// The controls in the window. // The controls in the window.
SearchTextfieldView* find_text_; views::Textfield* find_text_;
views::Label* match_count_text_; views::Label* match_count_text_;
FocusForwarderView* focus_forwarder_view_; FocusForwarderView* focus_forwarder_view_;
views::ImageButton* find_previous_button_; views::ImageButton* find_previous_button_;
......
...@@ -176,14 +176,8 @@ TEST_F(BubbleDelegateTest, InitiallyFocusedView) { ...@@ -176,14 +176,8 @@ TEST_F(BubbleDelegateTest, InitiallyFocusedView) {
Widget* bubble_widget = BubbleDelegateView::CreateBubble(bubble_delegate); Widget* bubble_widget = BubbleDelegateView::CreateBubble(bubble_delegate);
bubble_widget->Show(); bubble_widget->Show();
View* expected_view = bubble_delegate->GetInitiallyFocusedView(); EXPECT_EQ(bubble_delegate->GetInitiallyFocusedView(),
// TODO(ben|msw): The NativeWidgetWin::RestoreFocusOnActivate() workaround for bubble_widget->GetFocusManager()->GetFocusedView());
// http://crbug.com/125976 breaks this simple test by clearing proper focus.
#if defined(OS_WIN) && !defined(USE_AURA)
expected_view = NULL;
#endif
EXPECT_EQ(expected_view, bubble_widget->GetFocusManager()->GetFocusedView());
bubble_widget->CloseNow(); bubble_widget->CloseNow();
} }
......
...@@ -725,9 +725,8 @@ class VIEWS_EXPORT View : public ui::LayerDelegate, ...@@ -725,9 +725,8 @@ class VIEWS_EXPORT View : public ui::LayerDelegate,
virtual FocusManager* GetFocusManager(); virtual FocusManager* GetFocusManager();
virtual const FocusManager* GetFocusManager() const; virtual const FocusManager* GetFocusManager() const;
// Request the keyboard focus. The receiving view will become the // Request keyboard focus. The receiving view will become the focused view.
// focused view. void RequestFocus();
virtual void RequestFocus();
// Invoked when a view is about to be requested for focus due to the focus // Invoked when a view is about to be requested for focus due to the focus
// traversal. Reverse is this request was generated going backward // traversal. Reverse is this request was generated going backward
......
...@@ -502,16 +502,6 @@ void NativeWidgetWin::SaveFocusOnDeactivate() { ...@@ -502,16 +502,6 @@ void NativeWidgetWin::SaveFocusOnDeactivate() {
} }
void NativeWidgetWin::RestoreFocusOnActivate() { void NativeWidgetWin::RestoreFocusOnActivate() {
// Mysteriously, this only appears to be needed support restoration of focus
// to a child hwnd when restoring its top level window from the minimized
// state. If we don't do this, then ::SetFocus() to that child HWND returns
// ERROR_INVALID_PARAMETER, despite both HWNDs being of the same thread.
// See http://crbug.com/125976
{
// Since this is a synthetic reset, we don't need to tell anyone about it.
AutoNativeNotificationDisabler disabler;
GetWidget()->GetFocusManager()->ClearFocus();
}
RestoreFocusOnEnable(); RestoreFocusOnEnable();
} }
......
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