Bug 112227

Summary: REGRESSION (r133351, sub-pixel layout): Right-to-left block with text-overflow: ellipsis truncates prematurely (breaks facebook.com Hebrew UI)
Product: WebKit Reporter: mitz
Component: Layout and RenderingAssignee: zalan <zalan>
Status: RESOLVED FIXED    
Severity: Normal CC: allan.jensen, commit-queue, eae, esprehn+autocc, glenn, kondapallykalyan, leviw, robert, simon.fraser, webkit-bug-importer, WebkitBugTracker, zalan
Priority: P1 Keywords: InRadar, Regression
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: data:text/html,%3Cspan%20style=%22font-family:%20lucida%20grande;%20position:%20absolute;%20outline:%20thin%20dashed%20lightblue;%20overflow:%20hidden;%20text-overflow:%20ellipsis;%20direction:%20rtl;%22%3Esample%3C/span%3E
Attachments:
Description Flags
Patch none

Description mitz 2013-03-12 23:01:31 PDT 
To reproduce, navigate to the URL. The text truncated with ellipsis, even though it could fit untruncated (or the box could be made wider to let it fit). The same can be seen in many UI elements at facebook.com when the UI language is set to Hebrew on the website.

This was caused by <http://trac.webkit.org/r133351>.
Comment 1 Radar WebKit Bug Importer 2013-03-12 23:02:09 PDT 
<rdar://problem/13408101>
Comment 2 Levi Weintraub 2013-03-12 23:27:53 PDT 
Thanks for the great reduction!
Comment 3 Simon Fraser (smfr) 2013-03-26 14:02:58 PDT 
Levi: any update?
Comment 4 Robert Hogan 2013-03-30 02:11:08 PDT 
I can't reproduce with r144213.
Comment 5 Allan Sandfeld Jensen 2013-04-26 09:30:29 PDT 
I can't reproduce the issue either, using r149181 and Qt with subpixel-layout.
Comment 6 zalan 2013-12-09 13:33:12 PST 
I can repro it on Mac with subpixel on. (r160307)
Comment 7 zalan 2014-04-22 12:23:31 PDT 
Truncation happens when right/left offsets are integral snapped (so available width is int snapped too) while the computed text length is not. When we try to fit the text, we start truncating because available width < text length. The fix is either snap both the available width and the text length or neither. Our approach is to not to snap during layout time, however removing just these 2 snappings introduce regression. I am going to address it when inlines are properly un-snapped. 

diff --git a/Source/WebCore/rendering/RenderBlockLineLayout.cpp b/Source/WebCore/rendering/RenderBlockLineLayout.cpp
index 112d972..56685d8 100644
--- a/Source/WebCore/rendering/RenderBlockLineLayout.cpp
+++ b/Source/WebCore/rendering/RenderBlockLineLayout.cpp
@@ -614,8 +614,8 @@ void RenderBlockFlow::updateLogicalWidthForAlignment(const ETextAlign& textAlign
 static void updateLogicalInlinePositions(RenderBlockFlow& block, float& lineLogicalLeft, float& lineLogicalRight, float& availableLogicalWidth, bool firstLine, IndentTextOrNot shouldIndentText, Layou
 {
     LayoutUnit lineLogicalHeight = block.minLineHeightForReplacedRenderer(firstLine, boxLogicalHeight);
-    lineLogicalLeft = block.pixelSnappedLogicalLeftOffsetForLine(block.logicalHeight(), shouldIndentText == IndentText, lineLogicalHeight);
-    lineLogicalRight = block.pixelSnappedLogicalRightOffsetForLine(block.logicalHeight(), shouldIndentText == IndentText, lineLogicalHeight);
+    lineLogicalLeft = block.logicalLeftOffsetForLine(block.logicalHeight(), shouldIndentText == IndentText, lineLogicalHeight);
+    lineLogicalRight = block.logicalRightOffsetForLine(block.logicalHeight(), shouldIndentText == IndentText, lineLogicalHeight);
     availableLogicalWidth = lineLogicalRight - lineLogicalLeft;
 }
Comment 8 zalan 2014-05-16 10:45:32 PDT 
Created attachment 231580 [details]
Patch
Comment 9 Maciej Stachowiak 2014-05-18 17:21:59 PDT 
Comment on attachment 231580 [details]
Patch

r=me
Comment 10 WebKit Commit Bot 2014-05-19 10:37:57 PDT 
Comment on attachment 231580 [details]
Patch

Clearing flags on attachment: 231580

Committed r169048: <http://trac.webkit.org/changeset/169048>
Comment 11 WebKit Commit Bot 2014-05-19 10:38:09 PDT 
All reviewed patches have been landed.  Closing bug.