New issue
Advanced search Search tips

Issue 793762 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: 2017-12-17
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 2
Type: Bug



Sign in to add a comment

For Hangul, Underlines drawn with 'text-decoration: underline solid' are broken at some font sizes or under some characters

Project Member Reported by js...@chromium.org, Dec 11 2017

Issue description

Chrome Version: 64.0.3278.0 (Official Build) dev (64-bit)
OS: macOS 10.13

What steps will reproduce the problem?
(1) Try the following data url:

data:text/html;charset=utf-8,<span style="font-family: Apple SD Gothic Neo; text-decoration: underline solid red;">%ED%95%9C%EA%B8%80 %EA%B0%84%EB%8B%A4%EC%9D%91%EB%8C%80%ED%95%9C%EB%AF%BC%EA%B5%AD</span>

(Other fonts - "Nanum Gothic", 'Noto Sans CJK'  have the same issue). 

(2) Zoom in multiple steps
(3)

What is the expected result?

Underlines are continous under all characters at all sizes. 

What happens instead?

Underlines are broken and completely disappear under some characters at certain font sizes.  See the screenshot below. 


Additional information: 

This began to happen a few weeks ago with dev builds at  http://www.khan.co.kr . When the mouse is hovering over the headline (in the 2nd screenshot below, it starts with '바른정당 서울 지지율,'),  the underline is shown but it's not continuous but broken at multiple places.  Where it is broken are NOT glyph/character boundaries.

 
Screen Shot 2017-12-11 at 2.18.25 AM.png
17.6 KB View Download
Screen Shot 2017-12-11 at 2.20.00 AM.png
169 KB View Download

Comment 1 by js...@chromium.org, Dec 11 2017

Interestingly, with Noto Sans CJK KR (that covers all the characters in the test string (위키피디어 한국어 维基百科 ),  only Korean Hangul part (위키피디어 한국어) has the problem. 

data:text/html;charset=utf-8,<span style="font-family: Noto Sans CJK KR,sans-serif; text-decoration: underline solid red;">%EC%9C%84%ED%82%A4%EB%B0%B1%EA%B3%BC %ED%95%9C%EA%B5%AD%EC%96%B4 %E7%BB%B4%E5%9F%BA%E7%99%BE%E7%A7%91</span>

(Noto Sans CJK has to be installed)

Attached is a screenshot with even longer Chinese Hanzi strings. Only Korean Hangul (위키피디어 한국어) has this issue. Chinese Hanzi (CJK Ideograph) is just fine. 




Screen Shot 2017-12-11 at 2.32.48 AM.png
46.7 KB View Download

Comment 2 by js...@chromium.org, Dec 11 2017

The Korean Hangul part in the screenshot above is '위키백과 한국어' instead of '위키피디어 한국어'. 

Comment 3 by js...@chromium.org, Dec 11 2017

Labels: Needs-Bisect

Comment 4 by drott@chromium.org, Dec 11 2017

> Attached is a screenshot with even longer Chinese Hanzi strings. Only Korean Hangul (위키피디어 한국어) has this issue. Chinese Hanzi (CJK Ideograph) is just fine. 

Koji, does this mean we should add more characters/character classes to the CJK ink-skipping exemption list?

Jungshik, the issue / CL and Intent to Ship are here:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/47BHtmz0jVY
https://bugs.chromium.org/p/chromium/issues/detail?id=777428


Comment 5 by kojii@chromium.org, Dec 11 2017

Yeah, confirmed WebKit disables skipping for Hangul too. Remember they disables by Unicode blocks but we preferred to use IsCJKIdeographOrSymbol()? Hangul is CJK but isn't ideograph, so we probably missed it.

Comment 6 by kojii@chromium.org, Dec 11 2017

Components: -Blink>Layout
Labels: -Needs-Bisect
Owner: kojii@chromium.org
Status: Assigned (was: Untriaged)

Comment 7 by kojii@chromium.org, Dec 11 2017

Labels: -Type-Bug-Regression Type-Bug
Found the CL that disabled for ideographic characters:
https://codereview.chromium.org/2598393002

Comment 8 by kojii@chromium.org, Dec 11 2017

Labels: OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Windows

Comment 9 by js...@chromium.org, Dec 11 2017

Ok. That explains why Hangul has this issue while CJK ideograph does not. 

How about other scripts whose glyphs typically go below the bottom of EM box? A lot of complex scripts do that.  

data:text/html;charset=utf-8,<span style="text-decoration: underline solid red;">%E1%9E%9F%E1%9E%BC%E1%9E%98%E1%9E%9F%E1%9F%92%E1%9E%9C%E1%9E%B6%E1%9E%82%E1%9E%98%E1%9E%93%E1%9F%8D</span>

data:text/html;charset=utf-8,<span style="text-decoration: underline solid red;">%E0%B4%AA%E0%B5%86%E0%B4%B0%E0%B5%81%E0%B4%AE%E0%B4%BE%E0%B4%B1%E0%B5%8D%E0%B4%B1%E0%B4%A4%E0%B5%8D%E0%B4%A4%E0%B4%BF%E0%B4%A8%E0%B5%8D%E0%B4%B1%E0%B5%87%E0%B4%AF%E0%B5%81%E0%B4%82</span>

For them (Khmer and Malayalam in the above examples), skipping ink looks ok/good. 


Comment 10 by js...@chromium.org, Dec 11 2017

> For them (Khmer and Malayalam in the above examples), skipping ink looks ok/good. 

I was testing with Chrome 62.x (which does not seem to turn on skip-ink....). With canary build, I got the attached screenshot  I'm not sure whether it's good or bad. (regardless, I guess Blink  need a better way to determine the underline position.)  
Screen Shot 2017-12-11 at 1.05.41 PM.png
80.6 KB View Download

Comment 11 by js...@chromium.org, Dec 11 2017

Labels: M-64 ReleaseBlock-Stable
> IsCJKIdeographOrSymbol()? Hangul is CJK but isn't ideograph, so we probably missed it.

Where is the set defined? Anyway, Hangul ( [:sc=Hangul:] ) has to be added to the set. 

Comment 12 by js...@chromium.org, Dec 11 2017

hird_party/WebKit/Source/platform/text/CharacterPropertyDataGenerator.h  : 

All the Hangul blocks (syllable blocks, 3 conjoining jamo blocks, 2 compat. jamo blocks, enclosed/paren forms with Hangul) have to be added. 

Comment 13 by js...@chromium.org, Dec 11 2017

Well, adding Hangul to the set and renaming it to IsCJKOrSymbol wouldn't work because of the following (a bit suspicious) usage. So, a separate property for Hangul has to be defined, I'm afraid. 

  // Chinese and Japanese lack word boundary marks, and there is no clear
  // agreement on what constitutes a word, so treat the position before any CJK
  // character as a word start.
  if (Character::IsCJKIdeographOrSymbol(first_character))
    return true;
Project Member

Comment 14 by bugdroid1@chromium.org, Dec 11 2017

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

commit f551cd311c417887b71d5f0a6285e1918f04c350
Author: Koji Ishii <kojii@chromium.org>
Date: Mon Dec 11 22:53:03 2017

Fix more CJK characters not to skip ink for text decorations

The CL[1] disabled skipping inks for characters of
IsCJKIdeographOrSymbol(). This does not cover all characters in CJK.

This patch adds other CJK characters from Unicode blocks, in the
same way as WebKit does.

[1] https://codereview.chromium.org/2598393002

Bug:  793762 
Change-Id: I20e10588adc988399c900b336d0c22ad984bb011
Reviewed-on: https://chromium-review.googlesource.com/820210
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: Dominik Röttsches <drott@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523241}
[modify] https://crrev.com/f551cd311c417887b71d5f0a6285e1918f04c350/third_party/WebKit/LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-skip-ink-ideograph-expected.html
[modify] https://crrev.com/f551cd311c417887b71d5f0a6285e1918f04c350/third_party/WebKit/LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-skip-ink-ideograph.html
[modify] https://crrev.com/f551cd311c417887b71d5f0a6285e1918f04c350/third_party/WebKit/Source/platform/fonts/shaping/ShapeResultBloberizer.cpp
[modify] https://crrev.com/f551cd311c417887b71d5f0a6285e1918f04c350/third_party/WebKit/Source/platform/text/Character.cpp
[modify] https://crrev.com/f551cd311c417887b71d5f0a6285e1918f04c350/third_party/WebKit/Source/platform/text/Character.h

Comment 15 by kojii@chromium.org, Dec 12 2017

NextAction: 2017-12-17

Comment 16 by js...@chromium.org, Dec 12 2017

Summary: For Hangul, Underlines drawn with 'text-decoration: underline solid' are broken at some font sizes or under some characters (was: Underlines drawn with 'text-decoration: underline solid' are broken at some font sizes or under some characters)
Breaks in underline are showing in English too, Chrome 64.0.3282.24 (Official Build) beta (64-bit). See screenshot.

Slashes have an underline-break below them too in the inspector console. I thought this was purposeful styling in the new version??


Screenshot from 2017-12-15 09-03-05.png
20.8 KB View Download

Comment 18 by kojii@chromium.org, Dec 16 2017

#17: this is a feature to skip underlines where it crosses descenders. Ideographs characters conceptually doesn't have descenders (use different baseline at the bottom) but fonts give descenders so that they align nicely with other scripts. Also the user feedback to not to do it was strong and consistent.

We discussed about other scripts with W3C i18n WG, but other scripts either didn't want to disable, wasn't strong nor consistent, or we didn't receive feedback.

Also given WebKit is doing this for ~4 years, we currently disable only for specific ideographs scripts.

Comment 19 by kojii@chromium.org, Dec 16 2017

Labels: Merge-Request-64
The fix is in dev/Canary for a few days, requesting merge.
Project Member

Comment 20 by sheriffbot@chromium.org, Dec 16 2017

Labels: -Merge-Request-64 Hotlist-Merge-Review Merge-Review-64
This bug requires manual review: M64 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
The NextAction date has arrived: 2017-12-17
Did we confirm if this is a M64 regression or a feature to improve underlying in certain edge cases?

Comment 23 by kojii@chromium.org, Dec 18 2017

The ink-skipping is a new feature we turned on in M64, but the feature missed proper handling of Hangul (Korean) that all Korean users might be in trouble from M64 without this fix.
Labels: -Merge-Review-64 Merge-Approved-64
This has been well tested in canary and dev. Approving merge to M64. Branch:3282
Project Member

Comment 25 by bugdroid1@chromium.org, Dec 19 2017

Labels: -merge-approved-64 merge-merged-3282
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9f802345bd20185c79cbec975e0d19a4bf813411

commit 9f802345bd20185c79cbec975e0d19a4bf813411
Author: Koji Ishii <kojii@chromium.org>
Date: Tue Dec 19 04:03:55 2017

Merge 3282: Fix more CJK characters not to skip ink for text decorations

The CL[1] disabled skipping inks for characters of
IsCJKIdeographOrSymbol(). This does not cover all characters in CJK.

This patch adds other CJK characters from Unicode blocks, in the
same way as WebKit does.

[1] https://codereview.chromium.org/2598393002

Bug:  793762 
Change-Id: I20e10588adc988399c900b336d0c22ad984bb011
Reviewed-on: https://chromium-review.googlesource.com/820210
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: Dominik Röttsches <drott@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#523241}(cherry picked from commit f551cd311c417887b71d5f0a6285e1918f04c350)
Reviewed-on: https://chromium-review.googlesource.com/832627
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#292}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/9f802345bd20185c79cbec975e0d19a4bf813411/third_party/WebKit/LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-skip-ink-ideograph-expected.html
[modify] https://crrev.com/9f802345bd20185c79cbec975e0d19a4bf813411/third_party/WebKit/LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-skip-ink-ideograph.html
[modify] https://crrev.com/9f802345bd20185c79cbec975e0d19a4bf813411/third_party/WebKit/Source/platform/fonts/shaping/ShapeResultBloberizer.cpp
[modify] https://crrev.com/9f802345bd20185c79cbec975e0d19a4bf813411/third_party/WebKit/Source/platform/text/Character.cpp
[modify] https://crrev.com/9f802345bd20185c79cbec975e0d19a4bf813411/third_party/WebKit/Source/platform/text/Character.h

Comment 26 by kojii@chromium.org, Dec 19 2017

Status: Fixed (was: Assigned)

Sign in to add a comment