[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