[webkit-reviews] review denied: [Bug 7427] move event classes to platform directory and other changes : [Attachment 6673] patch, including change log

[bugzilla-request-daemon at opendarwin.org]

John Sullivan <sullivan at apple.com> has denied John Sullivan
<sullivan at apple.com>'s request for review:
Bug 7427: move event classes to platform directory and other changes
http://bugzilla.opendarwin.org/show_bug.cgi?id=7427

Attachment 6673: patch, including change log
http://bugzilla.opendarwin.org/attachment.cgi?id=6673&action=edit

------- Additional Comments from John Sullivan <sullivan at apple.com>
I see you changed the values representing mouse buttons from (left = 1, right =
2, mid = 4) to (left = 0, right = 2, mid = 1), and eliminated the symbolic
constants for these values. These values seem to get propagated by
MouseEvent::button(). Did you check that all existing callers were
appropriately updated? It seems a little confusing to me to use these integers
rather than symbolic constants for the different values. I can easily imagine
some future coder thinking that a value of 1 for button() would mean the left
button. Please explain/justify.


+    if (resign && _box && _box->eventFilterObject()) {
+	 _box->eventFilterObject()->eventFilterFocusOut();
+	 if (_box)
	     [MacFrame::bridgeForWidget(_box)
formControlIsResigningFirstResponder:self];
-	 }

The if (_box) check is unnecessary.

+	 if (widget && widget->eventFilterObject()) {
+	     widget->eventFilterObject()->eventFilterFocusOut();
+	     if (widget)
		 [MacFrame::bridgeForWidget(widget)
formControlIsResigningFirstResponder:self];
-	     }

The if (widget) check is unnecessary.

That's all I noticed. I'll give this an r- for now until you explain the button
number issue.