[webkit-reviews] review denied: [Bug 27968] [HTML5][Forms] <input type=number> UI : [Attachment 39538] Patch part 2: Mac implementation of outer-spin-button appearance
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Sep 23 10:34:10 PDT 2009
Eric Seidel <eric at webkit.org> has denied TAMURA, Kent <tkent at chromium.org>'s
request for review:
Bug 27968: [HTML5][Forms] <input type=number> UI
https://bugs.webkit.org/show_bug.cgi?id=27968
Attachment 39538: Patch part 2: Mac implementation of outer-spin-button
appearance
https://bugs.webkit.org/attachment.cgi?id=39538&action=review
------- Additional Comments from Eric Seidel <eric at webkit.org>
This change is mostly useless w/o pixel results. If we can't see what UI
you're adding, we can't really review it. Also your ChangeLogs are still kinda
empty. :(
Why have this big function if you're only going to return 0?
+const int* stepperMargins(NSControlSize controlSize)
+{
+ static const int margins[3][4] = {
+ {0, 0, 0, 0},
+ {0, 0, 0, 0},
+ {0, 0, 0, 0}
+ };
+ return margins[controlSize];
+}
No need:
+ static NSStepperCell* cell;
+ if (!cell) {
+ cell = [[NSStepperCell alloc] init];
+ }
Just do:
static NSStepperCell* cell = [[NSStepperCell alloc] init];
and that will only run once, the first time the function is called.
Needs { } due to the comment:
+ if (states & PressedState && states & SpinUpState)
+ // FIXME: There is no way to draw a NSSteperCell with the up button
hilighted.
+ // Disables the hilight of the down button if the up button is
pressed.
+ [cell setHighlighted:NO];
More information about the webkit-reviews
mailing list