[webkit-reviews] review denied: [Bug 108134] min/max-content as min/max-width is not set properly : [Attachment 185109] Patch

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has denied KyungTae Kim <ktf.kim at samsung.com>'s
request for review:
Bug 108134: min/max-content as min/max-width is not set properly
https://bugs.webkit.org/show_bug.cgi?id=108134

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

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=185109&action=review


> Source/WebCore/rendering/RenderBlock.cpp:5671
> +	   && !styleToUse->logicalMinWidth().isMinContent() &&
!styleToUse->logicalMaxWidth().isMinContent()
> +	   && !styleToUse->logicalMinWidth().isMaxContent() &&
!styleToUse->logicalMaxWidth().isMaxContent())

This isn't quite right. See
http://dev.w3.org/csswg/css3-sizing/#block-intrinsic. Specifically, see the
difference between the min/max-measure vs. the min/max-measure *contribution*.
Translating those terms into WebKit terms, the preferred width is the min/max
measure contribution. The newly added concept of intrinsic with is the min/max
measure. This is a bit confusing, because there's lots of old code that treats
intrinsic widths and preferred widths as the same.

In this patch, you're changed both, but you only want to be changing the
min/max measure.

I've been working on this in pieces. Look at my recent patches adding
computeIntrinsicLogicalWidths. We basically need to make changes like that for
all the computePreferredLogicalWidths overrides and then in the end, we change
min-content/max-content to use the intrinsic widths and not the preferred
widths.