A lice-sized code review (suzhe localrev 1613)

[jame...@gmail.com]

Hello phnixwxz,

I'd like you to do a code review. Please review the following patch:

----------------------------------------------------------------------
r1613: suzhe | 2010-01-08 15:18:12 +0800

Fire direct mouse up/out events event the element is disabled, so that the element can update its internal state correctly.
----------------------------------------------------------------------

=== ggadget/basic_element.cc
==================================================================
--- ggadget/basic_element.cc (revision 1612)
+++ ggadget/basic_element.cc (revision 1613)
@@ -866,7 +866,10 @@
*hittest = this_hittest;
}

- if (!enabled_) {
+ // Direct EVENT_MOUSE_UP and EVENT_MOUSE_OUT must be processed even if the
+ // element is disabled.
+ if (!enabled_ && !(direct && (type == Event::EVENT_MOUSE_UP ||
+ type == Event::EVENT_MOUSE_OUT))) {
return EVENT_RESULT_UNHANDLED;
}

=== ggadget/gtk/view_widget_binder.cc
==================================================================
--- ggadget/gtk/view_widget_binder.cc (revision 1612)
+++ ggadget/gtk/view_widget_binder.cc (revision 1613)
@@ -328,9 +328,11 @@

self_draw_ = true;
GdkRegion *region = GetInvalidateRegion();
- gdk_window_invalidate_region(widget_->window, region, TRUE);
+ if (!gdk_region_empty(region)) {
+ gdk_window_invalidate_region(widget_->window, region, TRUE);
+ gdk_window_process_updates(widget_->window, TRUE);
+ }
gdk_region_destroy(region);
- gdk_window_process_updates(widget_->window, TRUE);
last_self_draw_time_ = GetCurrentTime();
self_draw_ = false;
}
=== ggadget/view.cc
==================================================================
--- ggadget/view.cc (revision 1612)
+++ ggadget/view.cc (revision 1613)
@@ -404,7 +404,10 @@
// We used to check IsReallyEnabled() here, which seems too strict
// because the grabmouse_element_ may move out of the visible area, but
// it should still grab the mouse.
- if (grabmouse_element_.Get()->IsEnabled() &&
+ // EVENT_MOUSE_UP should always be fired no matter if the element is
+ // enabled or not.
+ if ((grabmouse_element_.Get()->IsEnabled() ||
+ type == Event::EVENT_MOUSE_UP) &&
(event.GetButton() & MouseEvent::BUTTON_LEFT) &&
(type == Event::EVENT_MOUSE_MOVE || type == Event::EVENT_MOUSE_UP ||
type == Event::EVENT_MOUSE_CLICK)) {

This is a semiautomated message from "svkmail". Complaints or suggestions?
Mail edy...@gmail.com.

[Xianzhu Wang]

LGTM

2010/1/8 <jame...@gmail.com>: