[webkit-reviews] review granted: [Bug 127021] Repeating background images should continue into margin tiles : [Attachment 221215] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Beth Dakin
<bdakin at apple.com>'s request for review:
Bug 127021: Repeating background images should continue into margin tiles
https://bugs.webkit.org/show_bug.cgi?id=127021

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

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=221215&action=review


> Source/WebCore/page/FrameView.h:197
> +    bool hasExtendedBackground();
> +    IntRect extendedBackgroundRect();

Both should be const. I would like to see extendedBackgroundRect() have a
comment saying what coordinate system the rect is in.

> Source/WebCore/rendering/RenderBox.cpp:1603
> +		   if (rectToRepaint.width() == rendererRect.width()) {
> +		       rectToRepaint.move(extendedBackgroundRect.x(), 0);
> +		       rectToRepaint.setWidth(extendedBackgroundRect.width());
> +		   }
> +		   if (rectToRepaint.height() == rendererRect.height()) {
> +		       rectToRepaint.move(0, extendedBackgroundRect.y());
> +		      
rectToRepaint.setHeight(extendedBackgroundRect.height());
> +		   }

It seems a bit arbitrary to say "if we're invalidating the width, just
invalidate the extended width". What about a non-repeating animating background
image that is projecting into the left margin but not the right?

I think the background geometry code needs to know about extended rects.