[webkit-reviews] review denied: [Bug 41163] Need ability to get rendered rectangle. : [Attachment 59666] propose patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has denied Damian Kaleta <dkaleta at apple.com>'s
request for review:
Bug 41163: Need ability to get rendered rectangle.
https://bugs.webkit.org/show_bug.cgi?id=41163

Attachment 59666: propose patch
https://bugs.webkit.org/attachment.cgi?id=59666&action=review

------- Additional Comments from Sam Weinig <sam at webkit.org>
>  
>  @interface DOMNode (WebDOMNodeOperationsPendingPublic)
> -- (NSString *)markupString;
> +- (CGRect)_renderRect:(bool *)isReplaced;

We should not remove markupString in this patch. If we want to do it it should
be done in a subsequent patch.

> + at implementation DOMNode (WebDOMNodeOperationsPendingPublic)
> +
> +- (CGRect)_renderRect:(bool *)isReplaced
> +{
> +    return CGRect(core(self)->renderRect(isReplaced));

This should return a NSRect instead of a CGRect since this is an Objective-C
API.

> +IntRect Node::renderRect(bool* isReplaced)
> +{
> +    IntRect rect(0, 0, 0, 0);
> +    
> +    RenderObject* hitRenderer = this->renderer();
> +    ASSERT(hitRenderer);
> +    RenderObject* renderer = hitRenderer;
> +    while (renderer && !renderer->isBody() && !renderer->isRoot()) {
> +	   if (renderer->isRenderBlock() ||
renderer->isInlineBlockOrInlineTable() || renderer->isReplaced()) {
> +	       *isReplaced = renderer->isReplaced();
> +	       return renderer->absoluteBoundingBoxRect(true);
> +	   }
> +	   renderer = renderer->parent();
> +    }
> +    return rect;

This should just be return IntRect(). You can remove the rect declaration at
the top.

r-. Lets iterate on this.