[webkit-reviews] review denied: [Bug 10794] [CSS1] Background-attachment: scroll should scroll within the containing block : [Attachment 20542] patch

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied Anatoli Papirovski
<apapirovski at mac.com>'s request for review:
Bug 10794: [CSS1] Background-attachment: scroll should scroll within the
containing block
http://bugs.webkit.org/show_bug.cgi?id=10794

Attachment 20542: patch
http://bugs.webkit.org/attachment.cgi?id=20542&action=edit

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
This is on the right track.  Thanks for contributing.  Comments below:

background-size and background-position are both supposed to be computed using
the box specified by background-origin.  This is CSS3 stuff so that may not
really be obvious yet, but that's how it is supposed to work.

Your check for background-position is correct.	However background-size is now
taking rather funny values, since it could end up being the viewport's
width/height - margins.  background-size should be using the same width and
height you're passing to background-position.

The destination rect for fixed backgrounds is also being potentially messed up
by your modifications to make left/right non-zero.  For example:

cw = pw + left + right;

Now that you put margins into left/right, the destination rect for fixed
backgrounds will be too small.

Style nitpick: Put spaces between arithmetic expressions, e.g.,

height() + marginTop() + marginBottom()

and not

height()+marginTop()+marginBottom().