[webkit-reviews] review denied: [Bug 45274] Breaking Float: floated block level element following inline element in floated container breaks to next line : [Attachment 151457] Patch

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied ChangSeok Oh <kevin.cs.oh at gmail.com>'s
request for review:
Bug 45274: Breaking Float: floated block level element following inline element
in floated container breaks to next line
https://bugs.webkit.org/show_bug.cgi?id=45274

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

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


r-

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:81
>      bool fitsOnLine(float extra) const { return currentWidth() + extra <=
m_availableWidth; }
> +    bool floatFitsOnLine(float extra) const { return currentWidth() -
whitespaceWidth() + extra <= m_availableWidth; }

I think I would prefer this:

bool fitsOnLine(float extra, LineFittingCheck includeWhitespace =
IncludeTrailingWhitespace) const { return currentWidth() - (includeWhitespace ?
trailingWhitespaceWidth() : 0) + extra <= m_availableWidth; }

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:88
> +    float whitespaceWidth() const { return m_whitespaceWidth; }

float trailingWhitespaceWidth() const { return m_trailingWhitespaceWidth; }

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:100
> +    void setWhitespaceWidth(float width) { m_whitespaceWidth = width; }

void setTrailingWhitespaceWidth(float width) { m_trailingWhitespaceWidth =
width; }

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:113
> +    float m_whitespaceWidth;

float m_trailingWhitespaceWidth;

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:2248
> +	       if (floatsFitOnLine &&
width.floatFitsOnLine(m_block->logicalWidthForFloat(f))) {

if (floatsFitOnLine && width.floatFitsOnLine(m_block->logicalWidthForFloat(f),
ExcludeTrailingWhitespace)) {

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:2423
> +			   width.setWhitespaceWidth(additionalTempWidth);

width.setTrailingWhitespaceWidth(additionalTempWidth);