[webkit-reviews] review denied: [Bug 37245] When deciding whether to line-break inside shrink-to-fit containers do not include trailing space when determining width of content : [Attachment 199111] Patch

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied Robert Hogan <robert at webkit.org>'s
request for review:
Bug 37245: When deciding whether to line-break inside shrink-to-fit containers
do not include trailing space when determining width of content
https://bugs.webkit.org/show_bug.cgi?id=37245

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

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=199111&action=review


> Source/WebCore/rendering/RenderBlockLineLayout.cpp:2742
> +static bool shrinksToFit(RenderObject* renderer)
> +{
> +    if (renderer->isFloatingOrOutOfFlowPositioned())
> +	   return true;
> +
> +    EDisplay display = renderer->style()->display();
> +    return display == INLINE_BLOCK || display == INLINE_TABLE || display ==
TABLE_CELL;
> +}

I am not understanding this patch at all. I don't get why shrink-to-fit should
result in special behavior here. shrink-to-fit can be caused by any number of
things, from a specified intrinsic width in CSS to default behavior for certain
objects. That width can further be influenced by max/min-width to no longer be
intrinsic, or maybe the container has a specified fixed width, etc. I need to
understand more about what is going on here in order to understand why a
behavioral change is needed only under certain circumstances. I'm unconvinced
that shrink-to-fit is the right thing to be testing here.