[webkit-reviews] review granted: [Bug 27272] [Leopard] Fixed position elements flash when CSS transforms are applied on page > 4096px tall : [Attachment 34349] Patch

[bugzilla-daemon at webkit.org]

mitz at webkit.org has granted Simon Fraser (smfr) <simon.fraser at apple.com>'s
request for review:
Bug 27272: [Leopard] Fixed position elements flash when CSS transforms are
applied on page > 4096px tall
https://bugs.webkit.org/show_bug.cgi?id=27272

Attachment 34349: Patch
https://bugs.webkit.org/attachment.cgi?id=34349&action=review

------- Additional Comments from mitz at webkit.org
> +	   WebHTMLView* htmlView = (WebHTMLView*)documentView;

With Objective-C objects, the star should go next to the variable name. There
should also be a space before the star in the cast.

> +#if defined(BUILDING_ON_LEOPARD)
> +    // Turn off default animations.
> +    NSNull* nullValue = [NSNull null];

The star should be next to nullValue.

> +    NSDictionary* actions = [NSDictionary dictionaryWithObjectsAndKeys:
> +				nullValue, @"anchorPoint",
> +				nullValue, @"bounds",
> +				nullValue, @"contents",
> +				nullValue, @"contentsRect",
> +				nullValue, @"opacity",
> +				nullValue, @"position",
> +				nullValue, @"shadowColor",
> +				nullValue, @"sublayerTransform",
> +				nullValue, @"sublayers",
> +				nullValue, @"transform",
> +				nullValue, @"zPosition",
> +				nil];
> +    [viewLayer setStyle:[NSDictionary dictionaryWithObject:actions
forKey:@"actions"]];

We usually avoid autoreleased objects like the above NSNull and two
NSDictionaries. It doesn’t really matter that they are autoreleased in this
case.

> +    const float kMaxHeight = 4096;

Please use CGFloat. I don’t think the k prefix is necessary.

> +	   float documentHeight = layerViewFrame.size.height;

Please use CGFloat.

> +	   float topOffset = NSMinY(visibleRect);

Ditto.

> +	   float bottomOffset = documentHeight - layerViewFrame.size.height -
topOffset;

…

r=me