[webkit-reviews] review denied: [Bug 102784] Percentage width replaced element incorrectly rendered when intrinsic size changed : [Attachment 176390] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Nov 28 10:03:19 PST 2012
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 176390: Patch
https://bugs.webkit.org/attachment.cgi?id=176390&action=review
------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=176390&action=review
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.
> LayoutTests/fast/css/percent-width-img-src-change-expected.html:7
> +<!-- The below boxes should have intrinsic size of greenbox-100px.png
(100x100) -->
> +
This should be a dumpAsText test. You can use check-layout.js or
js-test-pre.js. The benefits are that the test output will be clearer (it will
say PASS or FAIL with a useful message) and it will run faster.
More information about the webkit-reviews
mailing list