New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 12 users
Status: Fixed
Owner:
Closed: Jul 7
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Feature



Sign in to add a comment
Chrome should use font metrics to calculate underline thickness
Reported by h.jo...@samsung.com, Jan 26 2014 Back to list
UserAgent: Mozilla/5.0 (iPhone; CPU iPhone OS 7_0_4 like Mac OS X) AppleWebKit/537.51.1 (KHTML, like Gecko) CriOS/31.0.1650.18 Mobile/11B554a Safari/8536.25 (5616E139-056E-4A75-83F0-9352805BD2B0)

Example URL:

Steps to reproduce the problem:
1. Open any site with underline features
2. Observed chrome apply font size based underline thickness
3. 

What is the expected behavior?
Chrome should use font data to calculate correct underline thickness in case of faulty font metrics it should fallback on font size dependent calculation.

What went wrong?
To draw underline currently Chrome is depending on Font Size taking 10% 
Font Size as underline thickness which is not correct.

Does it occur on multiple sites: Yes

Is it a problem with a plugin? N/A 

Did this work before? No 

Does this work in other browsers? N/A 

Chrome version: <Copy from: 'about:version'>  Channel: n/a
OS Version: 
Flash Version:
 
Comment 1 by h.jo...@samsung.com, Jan 27 2014
I am working on this bug and code for review is submitted
https://codereview.chromium.org/147703002/

 Patches include change in Blink and Skia
Comment 2 by h.jo...@samsung.com, Jan 28 2014
Blink and Skia git repo are different so added two more patches related to Skia changes only.
https://codereview.chromium.org/148453005/
https://codereview.chromium.org/137543008/
Project Member Comment 3 by bugdroid1@chromium.org, Feb 11 2014
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=166890

------------------------------------------------------------------------
r166890 | h.joshi@samsung.com | 2014-02-11T03:54:48.681999Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/css/line-thickness-underline-strikethrough-overline-expected.png?r1=166890&r2=166889&pathrev=166890
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/FontMetrics.h?r1=166890&r2=166889&pathrev=166890
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/mac/SimpleFontDataMac.mm?r1=166890&r2=166889&pathrev=166890
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/InlineTextBox.cpp?r1=166890&r2=166889&pathrev=166890
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/TestExpectations?r1=166890&r2=166889&pathrev=166890
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/skia/SimpleFontDataSkia.cpp?r1=166890&r2=166889&pathrev=166890

To draw underline currently Chrome is depending on Font Size taking 10%
Font Size as underline thickness which is not correct. Font properties
should be taken into account to calculate underline thickness. Following change does the same, some Skia files are also updated for this change.

Blink and Skia git repo are different so added two more patches related to Skia changes only.
https://codereview.chromium.org/148453005/
https://codereview.chromium.org/137543008/
 and made this patch Skia independent.

BUG= 338148 

Review URL: https://codereview.chromium.org/147703002
------------------------------------------------------------------------
Project Member Comment 4 by bugdroid1@chromium.org, Feb 12 2014
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=167037

------------------------------------------------------------------------
r167037 | ojan@chromium.org | 2014-02-12T20:35:58.382539Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-lion/fast/repaint/shadow-multiple-vertical-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/inserting/return-key-with-selection-001-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/repaint/shadow-multiple-vertical-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/tables/mozilla/bugs/bug128229-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/deleting/delete-4083333-fix-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-mountainlion/fast/dom/clone-node-dynamic-style-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-lion/fast/block/positioning/auto/vertical-lr/007-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/block/positioning/auto/vertical-rl/007-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/pasteboard/paste-line-endings-001-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/text/international/rtl-white-space-pre-wrap-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/css/clip-zooming-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/http/tests/misc/acid2-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/pasteboard/paste-line-endings-005-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/text/trailing-white-space-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-lion/fast/repaint/shadow-multiple-horizontal-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/pasteboard/paste-line-endings-009-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/block/positioning/auto/007-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-lion/fast/css/line-thickness-underline-strikethrough-overline-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/css/first-line-text-decoration-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/repaint/shadow-multiple-strict-horizontal-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/inserting/insert-div-022-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/inserting/insert-div-026-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-mountainlion/fast/block/float/centered-float-avoidance-complexity-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/inserting/return-key-with-selection-002-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-mountainlion/editing/inserting/insert-div-023-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-snowleopard/fast/block/positioning/auto/vertical-lr/007-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/svg/custom/svg-fonts-without-missing-glyph-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-lion/fast/text/stroking-decorations-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/pasteboard/paste-line-endings-010-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/pasteboard/paste-line-endings-002-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/pasteboard/paste-line-endings-006-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/text/decorations-with-text-combine-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/pasteboard/paste-4038267-fix-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-snowleopard/fast/css/line-thickness-underline-strikethrough-overline-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-mountainlion/fast/block/positioning/auto/vertical-rl/007-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/css/acid2-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/block/float/centered-float-avoidance-complexity-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/inserting/insert-div-023-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/block/float/float-in-float-painting-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-mountainlion/fast/block/positioning/auto/007-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/style/style-3998892-fix-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/inserting/return-key-with-selection-003-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/text/stroking-decorations-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/repaint/shadow-multiple-strict-vertical-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-lion/fast/block/positioning/auto/vertical-rl/007-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/block/positioning/auto/vertical-lr/007-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/pasteboard/paste-line-endings-003-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/pasteboard/paste-line-endings-007-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/style/block-styles-007-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/repaint/shadow-multiple-horizontal-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/block/float/float-in-float-hit-testing-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-lion/fast/block/positioning/auto/007-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-lion/fast/block/float/centered-float-avoidance-complexity-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-lion/editing/inserting/insert-div-023-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/inserting/insert-div-024-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/text/line-breaks-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-lion/fast/dom/clone-node-dynamic-style-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/dom/clone-node-dynamic-style-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/css3-text/css3-text-decoration/text-decoration-style-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-mountainlion/fast/css/line-thickness-underline-strikethrough-overline-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/css3-text/css3-text-decoration/repaint/repaint-text-decoration-style-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-lion/ietestcenter/css3/text/textshadow-002-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-snowleopard/fast/block/positioning/auto/vertical-rl/007-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/ietestcenter/css3/text/textshadow-002-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-lion/fast/repaint/shadow-multiple-strict-vertical-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/pasteboard/paste-line-endings-004-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-lion/fast/text/decorations-with-text-combine-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/TestExpectations?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/text/trailing-white-space-2-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-snowleopard/fast/block/positioning/auto/007-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/pasteboard/paste-line-endings-008-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-snowleopard/fast/block/float/centered-float-avoidance-complexity-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-mountainlion/fast/block/positioning/auto/vertical-lr/007-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-snowleopard/editing/inserting/insert-div-023-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-lion/fast/repaint/shadow-multiple-strict-horizontal-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/fast/css/first-line-text-decoration-inherited-from-parent-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac-snowleopard/fast/dom/clone-node-dynamic-style-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/pasteboard/paste-match-style-002-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/style/smoosh-styles-003-expected.png?r1=167037&r2=167036&pathrev=167037
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/editing/pasteboard/paste-text-019-expected.png?r1=167037&r2=167036&pathrev=167037

Auto-rebaseline for r166890

http://src.chromium.org/viewvc/blink?view=revision&revision=166890

BUG= 338148 
TBR=h.joshi@samsung.com

Review URL: https://codereview.chromium.org/161163002
------------------------------------------------------------------------
Comment 5 by js...@chromium.org, Feb 27 2014
Does this issue also deal with using the underline position specified in a font?  CFF/PS fonts have the underline position as well as the underline thickness specified in post and CFF table. 
Comment 6 by h.jo...@samsung.com, Feb 27 2014
In this patch only Underline Thickness is added, need to work on Underline position. Changes for Underline Thickness and Position are added to Skia "https://codereview.chromium.org/152073003/". 
 
Will be making patch for other platforms and submit for review. 
Comment 7 by jia...@opera.com, Apr 21 2014
h.joshi, was this bug fixed already? Should the status be Unconfirmed?
Comment 8 by h.jo...@samsung.com, Apr 22 2014
Issue is fixed and code is committed. Let me check how to change bug status, I have no idea on how to make change to bug status.
Comment 9 by rsesek@chromium.org, Apr 28 2014
 Issue 313206  has been merged into this issue.
Comment 10 by kea...@keavon.com, Oct 27 2014
This still appears to be an issue with the current release of Chrome. What's the status?
Comment 11 by drott@chromium.org, Jun 25 2015
Cc: e...@chromium.org drott@chromium.org
Labels: Cr-Blink-Fonts
Owner: h.jo...@samsung.com
Status: Assigned
Summary: Chrome should use font metrics to calculate underline thickness (was: Chrome using font metrics to calculate underline thickness)
This issue is not fixed and as far as I can tell your patches h.joshi@ are somewhat in limbo - see eae's revert of your underline thickness changes in https://codereview.chromium.org/555883002. This means, currently setUnderlineThickness of FontMetrics.h is dead code at the moment, and we always fall back to the heuristic of calculating a thickness based on the font size. I would strongly suggest to find a sound and complete solution for this issue first, before looking at issue 386703. You brought this change in based on what you found in FF - which sounded promising to improve Blink's rendering.  Perhaps you can get a solution for the non-desirable cases/fonts as well, which were the reason eae@ reverted the change. Thanks.

Comment 12 by tkent@chromium.org, Jul 10 2015
Labels: -Cr-Content Cr-Blink
Labels: Hotlist-Recharge
This issue likely requires triage.  The current issue owner maybe inactive (i.e. hasn't fixed an issue in the last 30 days).  Thanks for helping out!

-Anthony
Labels: -Cr-Blink
Removing Cr-Blink from issues that already have Cr-Blink sub-label set.
Project Member Comment 15 by sheriffbot@chromium.org, Jul 6 2016
Labels: Hotlist-OpenBugWithCL
A change has landed for this issue, but it's been open for over 6 months. Please review and close it if applicable. If this issue should remain open, remove the "Hotlist-OpenBugWithCL" label. If no action is taken, it will be archived in 30 days.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-OpenBugWithCL
Owner: ----
Status: Available
h.joshi@samsung.com does not seem to work on the project any longer.
Project Member Comment 17 by sheriffbot@chromium.org, Jul 6
Labels: Hotlist-Recharge-Cold
Status: Untriaged
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: kojii@chromium.org
Labels: -Type-Bug Type-Feature
Status: Available
Owner: kojii@chromium.org
Status: Fixed
I believe we already do this.

ComputeDecorationThickness
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/paint/TextPainterBase.cpp?type=cs&q=ComputeDecorationThickness&l=230

If anyone sees this not working properly yet, please leave comments here.
Sign in to add a comment