[webkit-reviews] review granted: [Bug 88705] Text with text-overflow:ellipsis and text-align:right is left aligned : [Attachment 147922] Patch

[bugzilla-daemon at webkit.org]

mitz at webkit.org has granted Benjamin Poulain <benjamin at webkit.org>'s request
for review:
Bug 88705: Text with text-overflow:ellipsis and text-align:right is left
aligned
https://bugs.webkit.org/show_bug.cgi?id=88705

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

------- Additional Comments from mitz at webkit.org
View in context: https://bugs.webkit.org/attachment.cgi?id=147922&action=review


> Source/WebCore/ChangeLog:9
> +	   an ellipsis and where it should be. Because of that, text is layed
out as if it does

Typo: “layed” instead of “laid”.

> Source/WebCore/rendering/InlineBox.h:75
> +    void adjustLogicalPosition(float dx, float dy)

We should reserve “x” and “y” for the physical axes.

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:2694
> +	       float logicalLeft =
pixelSnappedLogicalLeftOffsetForLine(curr->logicalTop(), firstLine);
> +	       float availableLogicalWidth =
logicalRightOffsetForLine(curr->logicalTop(), false) - logicalLeft;

You should use curr->lineTop() here, not logicalTop().

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:2726
> +	   LayoutUnit blockRightEdge =
logicalRightOffsetForLine(curr->logicalTop(), firstLine);
> +	   LayoutUnit blockLeftEdge =
logicalLeftOffsetForLine(curr->logicalTop(), firstLine);

Same here.

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:2740
> +		   float truncatedWidth =
pixelSnappedLogicalRightOffsetForLine(curr->logicalTop(), firstLine);

Same here.