[webkit-reviews] review denied: [Bug 34544] [Chromium] RenderTheme does not draw focus rings on SL for checkboxes, radio buttons : [Attachment 48248] Update style, comment
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Feb 9 07:48:58 PST 2010
Timothy Hatcher <timothy at hatcher.name> has denied Avi Drissman
<avi at drissman.com>'s request for review:
Bug 34544: [Chromium] RenderTheme does not draw focus rings on SL for
checkboxes, radio buttons
https://bugs.webkit.org/show_bug.cgi?id=34544
Attachment 48248: Update style, comment
https://bugs.webkit.org/attachment.cgi?id=48248&action=review
------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>
> ++ (NSView*)TCMInterposing_focusView;
Should have a space between the type and the star for ObjC types.
> +BOOL CurrentOSHasSetFocusRingStyleInBitmapBug()
You should use bool. Should start with a lowercase "c".
> + UInt32 *pixelPlane = &pixel;
> + UInt32 **pixelPlanes = &pixelPlane;
Put the stars next to the type here.
> + NSBitmapImageRep *bitmap = [[NSBitmapImageRep alloc]
initWithBitmapDataPlanes:(unsigned char **)pixelPlanes
Use UInt8 to match the UInt32s used earlier. No space should be between the
type and stars.
> + NSRectFill(NSMakeRect(0,0,1,1));
Spaces needed after the commas.
> +bool swizzle()
Could use a more descriptive name since the swizzles specific methods.
> +class ScopedFixer
> +{
Brace should go on the previous line.
> ++ (NSView*)TCMInterposing_focusView
Space between the star and ObjC type.
> + NSView* view = [self TCMInterposing_focusView];
Srar should be next to the variable.
More information about the webkit-reviews
mailing list