[webkit-reviews] review denied: [Bug 101803] REGRESSION(r121789): Text not wrapping in presence of floating objects : [Attachment 175597] 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 101803: REGRESSION(r121789): Text not wrapping in presence of floating
objects
https://bugs.webkit.org/show_bug.cgi?id=101803

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

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


r-

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:140
> +    if (block->document()->inQuirksMode() && replacedHeight)
> +	   return replacedHeight;

I think this replaced stuff needs work. I'd prefer we just get the simplest fix
in first. For now, let's just change this to:

if (!block->document()->inNoQuirksMode())
    return 0;

Note that the descent clipping for replaced elements happens even in
LimitedQuirksMode, so you need to specifically target NoQuirksMode only.

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:950
> +	   if (!previousObject && r->m_object->isReplaced()) {
> +	       RenderBox* renderBox = toRenderBox(r->m_object);
> +	       LayoutUnit lineLogicalHeight = logicalHeightForLine(this,
firstLine, renderBox->logicalHeight());
> +	       logicalLeft =
pixelSnappedLogicalLeftOffsetForLine(logicalHeight(), firstLine,
lineLogicalHeight);
> +	       logicalRight =
pixelSnappedLogicalRightOffsetForLine(logicalHeight(), firstLine,
lineLogicalHeight);
> +#if ENABLE(CSS_EXCLUSIONS)
> +	       if (exclusionShapeInsideInfo &&
exclusionShapeInsideInfo->hasSegments()) {
> +		   logicalLeft =
max<float>(roundToInt(exclusionShapeInsideInfo->segments()[0].logicalLeft),
logicalLeft);
> +		   logicalRight =
min<float>(floorToInt(exclusionShapeInsideInfo->segments()[0].logicalRight),
logicalRight);
> +	       }
> +#endif
> +	       availableLogicalWidth = logicalRight - logicalLeft;
> +	   }

Just cut this for now.

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:2500
> +	       if (atStart)
> +		   width.updateAvailableWidth(replacedBox->logicalHeight());

Just drop this for now. Let's figure out the replaced element handling in a
separate patch.