[webkit-reviews] review denied: [Bug 102784] Percentage width replaced element incorrectly rendered when intrinsic size changed : [Attachment 176190] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Nov 27 11:35:46 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 176190: Patch
https://bugs.webkit.org/attachment.cgi?id=176190&action=review
------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=176190&action=review
We don't want to throw away this optimization.
Instead of always forcing a layout, can't we check to see if there's a percent
logical width, min-width, or max-width and only layout when needed? Also, this
seems like it's only a problem in shrink-to-fit objects
(sizesLogicalWidthToFitContent returns true). We could maybe detect that as
well.
> LayoutTests/ChangeLog:11
> + * fast/css/percent-width-img-src-change-expected.html: Added.
> + * fast/css/percent-width-img-src-change.html: Added.
I would make this a dumpAsText/js-test-pre.js test that checks the size of the
image and makes sure it is 64. The color is confusing since red boxes normally
mean fail.
> LayoutTests/fast/css/percent-width-img-src-change.html:1
> +<html>
Missing <!DOCTYPE html>
> LayoutTests/fast/css/percent-width-img-src-change.html:6
> + htmlImageElements = document.getElementsByClassName("imgForChange");
> + for (i = 0; i < htmlImageElements.length; i++)
Nit: 'var' for local variables, ++i instead of i++.
More information about the webkit-reviews
mailing list