[webkit-reviews] review denied: [Bug 102784] Percentage width replaced element incorrectly rendered when intrinsic size changed : [Attachment 178457] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied KyungTae Kim <ktf.kim at samsung.com>'s
request for review:
Bug 102784: Percentage width replaced element incorrectly rendered when
intrinsic size changed
https://bugs.webkit.org/show_bug.cgi?id=102784

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

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=178457&action=review


(In reply to comment #7)
> (In reply to comment #6)
> > Upon further discussion and inspection, I think the bug is in
RenderBox::computeReplacedLogicalWidthUsing.  Specifically, the if (cw > 0 ||)
check seems fishy.  I'm not entirely sure what the check is trying to do, but
it seems like an accident that the first time cw is 0 (because we haven't done
a layout yet) and the second time we use the old cw value.  I think we want the
code to fall through to intrinsicLogicalWidth() on relayout.
> 
> If make it to fall through to intrinsicLogicalWidth(), the percentage width
calculation could be wrong in other cases.

Can you describe the other cases where the percentage width calculations would
be wrong?

> LayoutTests/fast/css/percent-width-img-src-change-expected.txt:3
> +layer at (0,0) size 800x600
> +  RenderView at (0,0) size 800x600
> +layer at (0,0) size 800x234

This should not be a pixel test.  It should be a dumpAsText test.