New issue
Advanced search Search tips

Issue 879361 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug



Sign in to add a comment

SVG's getSubStringLength is not returning the correct length for emojis with ligatures.

Project Member Reported by cterefinko@google.com, Aug 30

Issue description

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

Steps to reproduce the problem:
Demo: https://jsfiddle.net/Lcahb301/27/

1. Try to measure an emoji that has a ligature in an svg text element. (such as πŸ‘¨β€πŸ‘©β€πŸ‘§β€πŸ‘¦) with getSubStringLength.
2. Observe value returned.
3. 

What is the expected behavior?
Value returned from getSubStringLength is correct.

What went wrong?
Value returned from getSubStringLength is incorrect.

Did this work before? N/A 

Does this work in other browsers? Yes

Chrome version: 68.0.3440.106  Channel: stable
OS Version: OS X 10.13.6
Flash Version:
 
Labels: Needs-Triage-M68
Labels: OS-Linux
Status: Available (was: Unconfirmed)
I'd suspect this is a dupe of issue 622336.
Cc: susan.boorgula@chromium.org
Labels: Triaged-ET Needs-Feedback
cterefinko@ Thanks for the issue.

Tested this issue on Mac OS 10.13.3 on the reported version 68.0.3440.106  and the latest Canary 70.0.3538.0 by following the below steps.

1. Launched chrome and navigated to the above given JSfiddle.
2. Could observe different behaviors in Chrome and Firefox.
Attached are the screen shots for reference.

Request you to check and confirm if the value returned in Firefox is expected behavior, which will help in further triaging of the issue.

Thanks..
879361-Firefox.png
306 KB View Download
879361-Chrome.png
228 KB View Download
I've confirmed that the underlying issue is the same as issue 622336 (metrics being smear across the code points in a typographic unit.) I think we should be able to do slightly better in this case though with a slight tweak (i.e in this case we lose parts because we don't consider the possibility of multiple surrogate pairs within a typographical unit.)
Should have given the values I was seeing. I also came across getComputedTextLength, so I added that to https://jsfiddle.net/Lcahb301/45/

Results:
Chrome
textContent|subStringLength|bboxWidth|computedTextLength 
πŸ‘¦|50|50|50
πŸ‘¨β€πŸ‘©β€πŸ‘§β€πŸ‘¦|35|50|50
πŸ‘¨β€|50|50|50
πŸ‘©πŸ½β€πŸŒΎ|33|50|50
πŸ‘¨β€πŸ‘¦|38|50|50
πŸ‘±πŸ½β€β™‚οΈ|42|50|50
πŸŽ…πŸ½|33|50|50

Firefox
textContent|subStringLength|bboxWidth|computedTextLength
πŸ‘¦|50|51|50
πŸ‘¨β€πŸ‘©β€πŸ‘§β€πŸ‘¦|50|51|50
πŸ‘¨β€|50|51|50
πŸ‘©πŸ½β€πŸŒΎ|50|51|50
πŸ‘¨β€πŸ‘¦|50|51|50
πŸ‘±πŸ½β€β™‚οΈ|50|51|50
πŸŽ…πŸ½|50|51|50 

Safari
textContent|subStringLength|bboxWidth|computedTextLength 
πŸ‘¦|50|50|50
πŸ‘¨β€πŸ‘©β€πŸ‘§β€πŸ‘¦|50|50|50
πŸ‘¨β€|50|50|50
πŸ‘©πŸ½β€πŸŒΎ|50|50|50
πŸ‘¨β€πŸ‘¦|50|50|50
πŸ‘±πŸ½β€β™‚οΈ|50|50|50
πŸŽ…πŸ½|50|50|50

But it seems like you already know the underlying issue.
Does this have to be restrict-view-google? Fs@opera.com is working on this but cannot see the bug anymore. It's not clear to me how fs@opera could ever have seen this bug, but I'll ask the monorail team about that separately.

cterefinko@google.com, is there anything that can't be public here? If not, can you remove the "restrict-view-google" label and add the "allpublic" label? That will make the bug public.
There is nothing requiring this to be restrict-view-google. Feel free to open this up, I don't think I have access to do that.
Labels: allpublic
Owner: f...@opera.com
Status: Started (was: Available)
Thanks! I filed https://bugs.chromium.org/p/monorail/issues/detail?id=4212 for the monorail issue.

Fredrik has a fix for this in review now.
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 31

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

commit 45ed266358b38ba6cd8b4f9b234f49fe064d521b
Author: Fredrik SΓΆderquist <fs@opera.com>
Date: Fri Aug 31 21:54:40 2018

Distribute widths evenly among code points in SynthesizeGraphemeWidths

Previously we did not consider the possibility of a typographic unit
containing multiple surrogate pairs, and could as an effect "lose" part
of the width.

Distribute the width evenly among code points instead to at least get
some sort of consistency.

Bug: 879361
Change-Id: I51a4e6c574926215a087b0bc824ac90d580ff169
Reviewed-on: https://chromium-review.googlesource.com/1199423
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Fredrik SΓΆderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#588183}
[add] https://crrev.com/45ed266358b38ba6cd8b4f9b234f49fe064d521b/third_party/WebKit/LayoutTests/svg/text/getsubstringlength-emoji-ligatures.html
[modify] https://crrev.com/45ed266358b38ba6cd8b4f9b234f49fe064d521b/third_party/blink/renderer/core/layout/svg/layout_svg_inline_text.cc

Status: Fixed (was: Started)
ZWJ and ZWNJ not being zero width (and similar issues) is left for issue 622336 to address.
Project Member

Comment 12 by bugdroid1@chromium.org, Sep 3

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

commit 9ab9087fea0756f52dd28f56e9d530d7f7420dd8
Author: calamity <calamity@chromium.org>
Date: Mon Sep 03 08:56:13 2018

Revert "Distribute widths evenly among code points in SynthesizeGraphemeWidths"

This reverts commit 45ed266358b38ba6cd8b4f9b234f49fe064d521b.

Reason for revert: getsubstringlength-emoji-ligatures.html is failing on Webkit Win10 starting at
https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Win10/40081


Original change's description:
> Distribute widths evenly among code points in SynthesizeGraphemeWidths
> 
> Previously we did not consider the possibility of a typographic unit
> containing multiple surrogate pairs, and could as an effect "lose" part
> of the width.
> 
> Distribute the width evenly among code points instead to at least get
> some sort of consistency.
> 
> Bug: 879361
> Change-Id: I51a4e6c574926215a087b0bc824ac90d580ff169
> Reviewed-on: https://chromium-review.googlesource.com/1199423
> Reviewed-by: Philip Rogers <pdr@chromium.org>
> Commit-Queue: Fredrik SΓΆderquist <fs@opera.com>
> Cr-Commit-Position: refs/heads/master@{#588183}

TBR=pdr@chromium.org,fs@opera.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 879361
Change-Id: I2c73e397bcfb9d458e4bf29f022cc95ac53ed5c0
Reviewed-on: https://chromium-review.googlesource.com/1201628
Reviewed-by: calamity <calamity@chromium.org>
Commit-Queue: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588354}
[delete] https://crrev.com/ff5b5b8d6a34f4dd063a6b9263228fbd5fc28a13/third_party/WebKit/LayoutTests/svg/text/getsubstringlength-emoji-ligatures.html
[modify] https://crrev.com/9ab9087fea0756f52dd28f56e9d530d7f7420dd8/third_party/blink/renderer/core/layout/svg/layout_svg_inline_text.cc

Labels: OS-Windows
Owner: ----
Status: Available (was: Fixed)

Sign in to add a comment