Bug 180377 – REGRESSION (r211531): Text flow changes and overlaps other text after double-click selecting paragraph

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[alan]

rdar://problem/34337967

[alan]

Created attachment 328387 [details]
Patch

[Antti Koivisto]

Comment on attachment 328387 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=328387&action=review

> Source/WebCore/rendering/SimpleLineLayout.cpp:605
> +    line.setLogicalLeftOffset(flow.logicalLeftOffsetForLine(height, DoNotIndentText, logicalHeight) + (shouldApplyTextIndent && isFirstLine ? flow.textIndentOffset() : LayoutUnit(0)));

I wonder if the default argument for the height in flow.logicalLeft/RightOffsetForLine could be removed? It feels wrong to call it without knowing where the line is.

[alan]

(In reply to Antti Koivisto from comment #2)
> Comment on attachment 328387 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=328387&action=review
> 
> > Source/WebCore/rendering/SimpleLineLayout.cpp:605
> > +    line.setLogicalLeftOffset(flow.logicalLeftOffsetForLine(height, DoNotIndentText, logicalHeight) + (shouldApplyTextIndent && isFirstLine ? flow.textIndentOffset() : LayoutUnit(0)));
> 
> I wonder if the default argument for the height in
> flow.logicalLeft/RightOffsetForLine could be removed? It feels wrong to call
> it without knowing where the line is.
Yeah, good point. Will address it in a separate patch.

[WebKit Commit Bot]

Comment on attachment 328387 [details]
Patch

Clearing flags on attachment: 328387

Committed r225497: <https://trac.webkit.org/changeset/225497>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.