New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 667128 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Use other robhogan account instead.
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Leading spaces in RTL flow text are not collapsed away when constructing a text run to calculate their width

Reported by drew.a.g...@gmail.com, Nov 20 2016

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.98 Safari/537.36

Steps to reproduce the problem:
1.  Open attached file
2. Shrink browser window until text can't all fit
3. Look at the ellipsis on the left. It has 0-3 dots depending on the width of the window.

What is the expected behavior?
There are always 3 dots.

What went wrong?
There were not always 3 dots.

Did this work before? No 

Does this work in other browsers? Yes

Chrome version: 54.0.2840.98  Channel: stable
OS Version: OS X 10.12.1
Flash Version: Shockwave Flash 23.0 r0

Works properly in Firebox. Exhibits a different but also incorrect behaviour in Safari.
 
untitled.html
750 bytes View Download

Comment 1 by samli@chromium.org, Nov 21 2016

Labels: Needs-Bisect
Labels: Needs-Triage-M54
Cc: sureshkumari@chromium.org
Labels: -Type-Bug -Pri-2 -Needs-Bisect -Needs-Triage-M54 hasbisect-per-revision M-57 OS-Linux OS-Windows Pri-1 Type-Bug-Regression
Owner: robhogan@chromium.org
Status: Assigned (was: Unconfirmed)
Able to reproduce the issue on windows-7, Mac-10.11.6 and Linux Ubuntu-14.04 using chrome stable version 54.0.2840.99 and canary 57.0.2926.0

This is regression issue broken in M52.Please find the bisect information as below

Narrow Bisect::
===============
Good :52.0.2722.0 --   (build revision 390853)
Bad:: 52.0.2723.0 --   (build revision 391139)

ChangeLog: 
================
https://chromium.googlesource.com/chromium/src/+log/2aaedda2430ecad19a8ac4427c145d1b093f3df9..e168de8aa7dd678277ca1e8df6cc8fe6b418acdf

Possible suspect
==================
e168de8aa7dd678277ca1e8df6cc8fe6b418acdf	

Review URL: https://codereview.chromium.org/1916943002 

robhogan@ could you please look into this issue if it is related to your change,else please help us in finding the appropriate owner for this issue.

Thanks.
Cc: ligim...@chromium.org
Labels: -M-57 M-56
Please target the fix during M56 time frame.
Labels: Needs-Triage-M54
Cc: kojii@chromium.org e...@chromium.org
Summary: Leading spaces in RTL flow text are not collapsed away when constructing a text run to calculate their width (was: Poor interaction of direction: rtl, text-align: left, and text-overflow: ellipsis)
kojii@ - Question for you. How do I get this 

 LayoutUnit widthOfVisibleText(getLineLayoutItem().width(
        ltr == flowIsLTR ? m_start : offset,
        ltr == flowIsLTR ? offset : m_len - offset, textPos(),
        flowIsLTR ? LTR : RTL, isFirstLineStyle()));

to disregard the 'leading' spaces on a construct like:

<!DOCTYPE html>
<style>
  .button-container {
    width: 100px;
  }
  .button-container > div {
    border: 1px solid gray;
    overflow: hidden;
  }
  .left-button {
    text-overflow: ellipsis;
    direction: rtl;
  }
</style>
<div class="button-container">
  <div class="left-button">
    ButtonWithAReallyLongNameButtonWithAReallyLongName <!-- want to disregard the leading spaces here. LayoutText::width() is including them when it offsets into the text. -->
  </div>
</div>
                                                         



Specifically it's collapsing the leading spaces to a single space, rather than collapsing them away completely. I believe this is because it's treating them as trailing spaces (which they are in, an RTL flow).

Don't spend too long on this, I am just thinking out loud and think I can get to the bottom of it. Any wisdom you have that shortens the process would be welcome!
OK I think I've found what I need to do! 

Comment 9 by kojii@chromium.org, Nov 29 2016

Sorry for a late reply, I'm in GMT+9.

LineLayoutItem.width() does not know about white space collapsing, so you'll need to do it before calling width. I'm not sure if we had a helper function to do that.

Wondering if we have better way to go without worrying about white space collapsing, but have you figured out a good way to go already?

Comment 10 by kojii@chromium.org, Nov 29 2016

I wasn't aware until now that LayouText::width() isn't a good interface; it returns logicalWidth if start==0 and len==textLength(), so white space collapsing is applied, but otherwise it goes to measure the text, so callers need to take care of white space collapsing...if I read the code correctly?
Project Member

Comment 11 by bugdroid1@chromium.org, Dec 5 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/77dd2d44c88d51cbda3d2fcf35c7153b16f652e4

commit 77dd2d44c88d51cbda3d2fcf35c7153b16f652e4
Author: robhogan <robhogan@gmail.com>
Date: Mon Dec 05 01:47:35 2016

Skip leading space in ltr text in rtl flow when placing ellipsis

Account for leading space when selecting our offset for the visible text.

BUG= 667128 

Review-Url: https://codereview.chromium.org/2529243002
Cr-Commit-Position: refs/heads/master@{#436216}

[modify] https://crrev.com/77dd2d44c88d51cbda3d2fcf35c7153b16f652e4/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/77dd2d44c88d51cbda3d2fcf35c7153b16f652e4/third_party/WebKit/LayoutTests/fast/text/ellipsis-ltr-text-in-rtl-flow-leading-space.html
[modify] https://crrev.com/77dd2d44c88d51cbda3d2fcf35c7153b16f652e4/third_party/WebKit/LayoutTests/platform/linux/editing/selection/select-text-overflow-ellipsis-mixed-in-ltr-expected.png
[modify] https://crrev.com/77dd2d44c88d51cbda3d2fcf35c7153b16f652e4/third_party/WebKit/LayoutTests/platform/linux/fast/css/text-overflow-ellipsis-expected.png
[modify] https://crrev.com/77dd2d44c88d51cbda3d2fcf35c7153b16f652e4/third_party/WebKit/LayoutTests/platform/linux/fast/css/text-overflow-ellipsis-strict-expected.png
[add] https://crrev.com/77dd2d44c88d51cbda3d2fcf35c7153b16f652e4/third_party/WebKit/LayoutTests/platform/linux/fast/text/ellipsis-ltr-text-in-rtl-flow-leading-space-expected.png
[add] https://crrev.com/77dd2d44c88d51cbda3d2fcf35c7153b16f652e4/third_party/WebKit/LayoutTests/platform/linux/fast/text/ellipsis-ltr-text-in-rtl-flow-leading-space-expected.txt
[modify] https://crrev.com/77dd2d44c88d51cbda3d2fcf35c7153b16f652e4/third_party/WebKit/LayoutTests/platform/linux/fast/text/ellipsis-mixed-text-in-ltr-flow-underline-expected.png
[modify] https://crrev.com/77dd2d44c88d51cbda3d2fcf35c7153b16f652e4/third_party/WebKit/Source/core/layout/line/InlineTextBox.cpp

Project Member

Comment 12 by bugdroid1@chromium.org, Dec 5 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1e9cfa7373fcef61674adb3bde4327510730e477

commit 1e9cfa7373fcef61674adb3bde4327510730e477
Author: Rebaseline Bot <blink-rebaseline-bot@chromium.org>
Date: Mon Dec 05 03:17:03 2016

Auto-rebaseline for r436216

https://chromium.googlesource.com/chromium/src/+/77dd2d44c88d5

BUG= 667128 
TBR=robhogan@gmail.com

Review URL: https://codereview.chromium.org/2551753003 .

Cr-Commit-Position: refs/heads/master@{#436225}

[modify] https://crrev.com/1e9cfa7373fcef61674adb3bde4327510730e477/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/1e9cfa7373fcef61674adb3bde4327510730e477/third_party/WebKit/LayoutTests/platform/mac-mac10.10/fast/text/ellipsis-ltr-text-in-rtl-flow-leading-space-expected.png
[modify] https://crrev.com/1e9cfa7373fcef61674adb3bde4327510730e477/third_party/WebKit/LayoutTests/platform/mac-mac10.10/fast/text/ellipsis-mixed-text-in-ltr-flow-underline-expected.png
[add] https://crrev.com/1e9cfa7373fcef61674adb3bde4327510730e477/third_party/WebKit/LayoutTests/platform/mac-mac10.9/fast/text/ellipsis-ltr-text-in-rtl-flow-leading-space-expected.png
[modify] https://crrev.com/1e9cfa7373fcef61674adb3bde4327510730e477/third_party/WebKit/LayoutTests/platform/mac-mac10.9/fast/text/ellipsis-mixed-text-in-ltr-flow-underline-expected.png
[modify] https://crrev.com/1e9cfa7373fcef61674adb3bde4327510730e477/third_party/WebKit/LayoutTests/platform/mac/editing/selection/select-text-overflow-ellipsis-mixed-in-ltr-expected.png
[modify] https://crrev.com/1e9cfa7373fcef61674adb3bde4327510730e477/third_party/WebKit/LayoutTests/platform/mac/fast/css/text-overflow-ellipsis-expected.png
[modify] https://crrev.com/1e9cfa7373fcef61674adb3bde4327510730e477/third_party/WebKit/LayoutTests/platform/mac/fast/css/text-overflow-ellipsis-strict-expected.png
[add] https://crrev.com/1e9cfa7373fcef61674adb3bde4327510730e477/third_party/WebKit/LayoutTests/platform/mac/fast/text/ellipsis-ltr-text-in-rtl-flow-leading-space-expected.png
[add] https://crrev.com/1e9cfa7373fcef61674adb3bde4327510730e477/third_party/WebKit/LayoutTests/platform/mac/fast/text/ellipsis-ltr-text-in-rtl-flow-leading-space-expected.txt
[modify] https://crrev.com/1e9cfa7373fcef61674adb3bde4327510730e477/third_party/WebKit/LayoutTests/platform/mac/fast/text/ellipsis-mixed-text-in-ltr-flow-underline-expected.png
[modify] https://crrev.com/1e9cfa7373fcef61674adb3bde4327510730e477/third_party/WebKit/LayoutTests/platform/win/fast/css/text-overflow-ellipsis-expected.png
[modify] https://crrev.com/1e9cfa7373fcef61674adb3bde4327510730e477/third_party/WebKit/LayoutTests/platform/win/fast/css/text-overflow-ellipsis-strict-expected.png
[add] https://crrev.com/1e9cfa7373fcef61674adb3bde4327510730e477/third_party/WebKit/LayoutTests/platform/win/fast/text/ellipsis-ltr-text-in-rtl-flow-leading-space-expected.png
[add] https://crrev.com/1e9cfa7373fcef61674adb3bde4327510730e477/third_party/WebKit/LayoutTests/platform/win/fast/text/ellipsis-ltr-text-in-rtl-flow-leading-space-expected.txt
[modify] https://crrev.com/1e9cfa7373fcef61674adb3bde4327510730e477/third_party/WebKit/LayoutTests/platform/win/fast/text/ellipsis-mixed-text-in-ltr-flow-underline-expected.png

Status: Fixed (was: Assigned)

Sign in to add a comment