Skip to content

Commit

Permalink
Enabling the default button behavior on Linux toolkit_views.
Browse files Browse the repository at this point in the history
I had to make the KeyEvent constructor include the event flags.
Also cleaned-up some unit-tests

BUG=None
TEST=Run the unit-tests.

Review URL: http://codereview.chromium.org/266012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@28397 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
jcampan@chromium.org committed Oct 8, 2009
1 parent b57c599 commit 0e0e6d7
Show file tree
Hide file tree
Showing 12 changed files with 105 additions and 161 deletions.
6 changes: 5 additions & 1 deletion views/accelerator.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class Accelerator {
modifiers_ = accelerator.modifiers_;
}

~Accelerator() { };
~Accelerator() { }

Accelerator& operator=(const Accelerator& accelerator) {
if (this != &accelerator) {
Expand Down Expand Up @@ -79,6 +79,10 @@ class Accelerator {
return key_code_;
}

int modifiers() const {
return modifiers_;
}

// Returns a string with the localized shortcut if any.
std::wstring GetShortcutText() const;

Expand Down
14 changes: 2 additions & 12 deletions views/controls/button/custom_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,18 +94,8 @@ bool CustomButton::IsTriggerableEvent(const MouseEvent& e) {
bool CustomButton::AcceleratorPressed(const Accelerator& accelerator) {
if (enabled_) {
SetState(BS_NORMAL);
#if defined(OS_WIN)
KeyEvent key_event(Event::ET_KEY_RELEASED, accelerator.GetKeyCode(), 0, 0);
#elif defined(OS_LINUX)
GdkEventKey gdk_key;
memset(&gdk_key, 0, sizeof(GdkEventKey));
gdk_key.type = GDK_KEY_RELEASE;
gdk_key.keyval = accelerator.GetKeyCode();
gdk_key.state = (accelerator.IsAltDown() << 3) +
(accelerator.IsCtrlDown() << 2) +
accelerator.IsShiftDown();
KeyEvent key_event(&gdk_key);
#endif
KeyEvent key_event(Event::ET_KEY_RELEASED, accelerator.GetKeyCode(),
accelerator.modifiers(), 0, 0);
NotifyClick(key_event);
return true;
}
Expand Down
7 changes: 6 additions & 1 deletion views/controls/button/native_button_gtk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ void NativeButtonGtk::UpdateEnabled() {
void NativeButtonGtk::UpdateDefault() {
if (!native_view())
return;
NOTIMPLEMENTED();
if (native_button_->is_default())
gtk_widget_grab_default(native_view());
}

View* NativeButtonGtk::GetView() {
Expand Down Expand Up @@ -96,6 +97,10 @@ void NativeButtonGtk::CreateNativeControl() {
GtkWidget* widget = gtk_button_new();
g_signal_connect(G_OBJECT(widget), "clicked",
G_CALLBACK(CallClicked), this);

// Any push button can become the default button.
GTK_WIDGET_SET_FLAGS(widget, GTK_CAN_DEFAULT);

NativeControlCreated(widget);
}

Expand Down
8 changes: 8 additions & 0 deletions views/event.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ LocatedEvent::LocatedEvent(const LocatedEvent& model, View* from, View* to)
View::ConvertPointToView(from, to, &location_);
}

KeyEvent::KeyEvent(EventType type, base::KeyboardCode key_code,
int event_flags, int repeat_count, int message_flags)
: Event(type, event_flags),
key_code_(key_code),
repeat_count_(repeat_count),
message_flags_(message_flags) {
}

MouseEvent::MouseEvent(EventType type,
View* from,
View* to,
Expand Down
10 changes: 6 additions & 4 deletions views/event.h
Original file line number Diff line number Diff line change
Expand Up @@ -241,13 +241,13 @@ class MouseEvent : public LocatedEvent {
////////////////////////////////////////////////////////////////////////////////
class KeyEvent : public Event {
public:
#if defined(OS_WIN)
// Create a new key event
KeyEvent(EventType type,
base::KeyboardCode key_code,
int event_flags,
int repeat_count,
int message_flags);
#elif defined(OS_LINUX)
#if defined(OS_LINUX)
explicit KeyEvent(GdkEventKey* event);
#endif

Expand All @@ -267,11 +267,13 @@ class KeyEvent : public Event {
return repeat_count_;
}

private:
#if defined(OS_WIN)
int GetKeyStateFlags() const;
// Returns the current state of the KeyState.
static int GetKeyStateFlags();
#endif

private:

base::KeyboardCode key_code_;
int repeat_count_;
int message_flags_;
Expand Down
4 changes: 1 addition & 3 deletions views/event_gtk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@

namespace views {

// TODO(jcampan): the same physical key can send different keyvals (ex: a or A).
// In order for accelerators to work, we need to normalize that. The right
// solution should probably to get the key-code out of the keystate.
KeyEvent::KeyEvent(GdkEventKey* event)
: Event(event->type == GDK_KEY_PRESS ?
Event::ET_KEY_PRESSED : Event::ET_KEY_RELEASED,
Expand All @@ -23,6 +20,7 @@ KeyEvent::KeyEvent(GdkEventKey* event)
message_flags_(0) {
}

// static
int Event::GetFlagsFromGdkState(int state) {
int flags = 0;
if (state & GDK_CONTROL_MASK)
Expand Down
11 changes: 2 additions & 9 deletions views/event_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,8 @@ int Event::ConvertWindowsFlags(UINT win_flags) {
return r;
}

KeyEvent::KeyEvent(EventType type, base::KeyboardCode key_code,
int repeat_count, int message_flags)
: Event(type, GetKeyStateFlags()),
key_code_(key_code),
repeat_count_(repeat_count),
message_flags_(message_flags) {
}

int KeyEvent::GetKeyStateFlags() const {
// static
int KeyEvent::GetKeyStateFlags() {
// Windows Keyboard messages don't come with control key state as parameters
// like mouse messages do, so we need to explicitly probe for these key
// states.
Expand Down
1 change: 1 addition & 0 deletions views/focus/accelerator_handler_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ bool AcceleratorHandler::Dispatch(const MSG& msg) {
case WM_SYSKEYDOWN: {
KeyEvent event(Event::ET_KEY_PRESSED,
win_util::WinToKeyboardCode(msg.wParam),
KeyEvent::GetKeyStateFlags(),
msg.lParam & 0xFFFF,
(msg.lParam & 0xFFFF0000) >> 16);
process_message = focus_manager->OnKeyEvent(event);
Expand Down
27 changes: 14 additions & 13 deletions views/focus/focus_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -414,19 +414,19 @@ void FocusTraversalTest::InitContentView() {
y += label_height + gap_between_labels;

NativeButton* button = new NativeButton(NULL, L"Click me");
button->SetBounds(label_x, y + 10, 50, 20);
button->SetBounds(label_x, y + 10, 80, 30);
button->SetID(kFruitButtonID);
left_container->AddChildView(button);
y += 40;

cb = new Checkbox(L"This is another check box");
cb->SetBounds(label_x + label_width + 5, y, 150, 20);
cb->SetBounds(label_x + label_width + 5, y, 180, 20);
cb->SetID(kFruitCheckBoxID);
left_container->AddChildView(cb);
y += 20;

Combobox* combobox = new Combobox(&combobox_model_);
combobox->SetBounds(label_x + label_width + 5, y, 150, 20);
combobox->SetBounds(label_x + label_width + 5, y, 150, 30);
combobox->SetID(kComboboxID);
left_container->AddChildView(combobox);

Expand All @@ -439,7 +439,7 @@ void FocusTraversalTest::InitContentView() {
right_container->SetBounds(270, 35, 300, 200);

y = 10;
int radio_button_height = 15;
int radio_button_height = 18;
int gap_between_radio_buttons = 10;
RadioButton* radio_button = new RadioButton(L"Asparagus", 1);
radio_button->SetID(kAsparagusButtonID);
Expand Down Expand Up @@ -507,22 +507,23 @@ void FocusTraversalTest::InitContentView() {
}

y = 250;
int width = 50;
int width = 60;
button = new NativeButton(NULL, L"OK");
button->SetID(kOKButtonID);
button->SetIsDefault(true);

content_view_->AddChildView(button);
button->SetBounds(150, y, width, 20);
button->SetBounds(150, y, width, 30);

button = new NativeButton(NULL, L"Cancel");
button->SetID(kCancelButtonID);
content_view_->AddChildView(button);
button->SetBounds(250, y, width, 20);
button->SetBounds(220, y, width, 30);

button = new NativeButton(NULL, L"Help");
button->SetID(kHelpButtonID);
content_view_->AddChildView(button);
button->SetBounds(350, y, width, 20);
button->SetBounds(290, y, width, 30);

y += 40;

Expand Down Expand Up @@ -571,20 +572,20 @@ void FocusTraversalTest::InitContentView() {

button = new NativeButton(NULL, L"Search");
contents->AddChildView(button);
button->SetBounds(115, 10, 50, 20);
button->SetBounds(112, 5, 60, 30);
button->SetID(kSearchButtonID);

link = new Link(L"Help");
link->SetHorizontalAlignment(Label::ALIGN_LEFT);
link->SetID(kHelpLinkID);
contents->AddChildView(link);
link->SetBounds(170, 20, 30, 15);
link->SetBounds(175, 10, 30, 20);

search_border_view_ = new BorderView(contents);
search_border_view_->SetID(kSearchContainerID);

content_view_->AddChildView(search_border_view_);
search_border_view_->SetBounds(300, y, 200, 50);
search_border_view_->SetBounds(300, y, 240, 50);

y += 60;

Expand All @@ -594,11 +595,11 @@ void FocusTraversalTest::InitContentView() {
contents->SetID(kThumbnailContainerID);
button = new NativeButton(NULL, L"Star");
contents->AddChildView(button);
button->SetBounds(5, 5, 50, 20);
button->SetBounds(5, 5, 50, 30);
button->SetID(kThumbnailStarID);
button = new NativeButton(NULL, L"SuperStar");
contents->AddChildView(button);
button->SetBounds(60, 5, 100, 20);
button->SetBounds(60, 5, 100, 30);
button->SetID(kThumbnailSuperStarID);

content_view_->AddChildView(contents);
Expand Down
6 changes: 0 additions & 6 deletions views/view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -988,16 +988,13 @@ void View::RemoveAccelerator(const Accelerator& accelerator) {
return;
}

// TODO(port): Fix this once we have a FocusManger for Linux.
#if defined(OS_WIN)
FocusManager* focus_manager = GetFocusManager();
if (focus_manager) {
// We may not have a FocusManager if the window containing us is being
// closed, in which case the FocusManager is being deleted so there is
// nothing to unregister.
focus_manager->UnregisterAccelerator(accelerator, this);
}
#endif
}

void View::ResetAccelerators() {
Expand Down Expand Up @@ -1047,8 +1044,6 @@ void View::UnregisterAccelerators() {

RootView* root_view = GetRootView();
if (root_view) {
// TODO(port): Fix this once we have a FocusManger for Linux.
#if defined(OS_WIN)
FocusManager* focus_manager = GetFocusManager();
if (focus_manager) {
// We may not have a FocusManager if the window containing us is being
Expand All @@ -1059,7 +1054,6 @@ void View::UnregisterAccelerators() {
accelerators_->clear();
accelerators_.reset();
registered_accelerator_count_ = 0;
#endif
}
}

Expand Down
Loading

0 comments on commit 0e0e6d7

Please sign in to comment.