Do not apply text-underline-offset to overline decoration

Take into account swapping of underline and overline in vertical, when
using text-underline-position "right" and "left".

Add 3 WPT tests to ensure text-overline-offset is not applied to
overline and that the underline on the correct side is offset in
vertical.

Bug: 1143600
Change-Id: Idd52a5d2d953fc486050ef20a1960ab51145521c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2510829Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Auto-Submit: Dominik Röttsches <drott@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823706}

third_party/blink/renderer/core/layout/ng/ng_ink_overflow.cc

@@ -386,8 +386,11 @@ LayoutRect NGInkOverflow::ComputeTextDecorationOverflow(
     const NGTextFragmentPaintInfo& text_info,
     const ComputedStyle& style,
     const LayoutRect& ink_overflow) {
-  // Use a zero offset because all offsets are applied to the ink overflow
-  // after it has been computed.
+  // TODO(https://crbug.com/1145160): Reduce code duplication between here and
+  // TextPainterBase::PaintDecorations*.
+
+  // Use a zero offset because all offsets
+  // are applied to the ink overflow after it has been computed.
   PhysicalOffset offset;
   TextDecorationInfo decoration_info(offset, offset, ink_overflow.Width(),
                                      style.GetFontBaseline(), style,
@@ -423,11 +426,15 @@ LayoutRect NGInkOverflow::ComputeTextDecorationOverflow(
     float resolved_thickness = decoration_info.ResolvedThickness();
 
     if (has_underline) {
+      // Don't apply text-underline-offset to overline.
+      Length line_offset =
+          flip_underline_and_overline ? Length() : decoration.UnderlineOffset();
+
       const int paint_underline_offset =
           decoration_offset.ComputeUnderlineOffset(
               underline_position, decoration_info.Style().ComputedFontSize(),
-              decoration_info.FontData()->GetFontMetrics(),
-              decoration.UnderlineOffset(), resolved_thickness);
+              decoration_info.FontData()->GetFontMetrics(), line_offset,
+              resolved_thickness);
       decoration_info.SetPerLineData(
           TextDecoration::kUnderline, paint_underline_offset,
           TextDecorationInfo::DoubleOffsetFromThickness(resolved_thickness), 1);
@@ -435,14 +442,17 @@ LayoutRect NGInkOverflow::ComputeTextDecorationOverflow(
           decoration_info.BoundsForLine(TextDecoration::kUnderline));
     }
     if (has_overline) {
+      // Don't apply text-underline-offset to overline.
+      Length line_offset =
+          flip_underline_and_overline ? decoration.UnderlineOffset() : Length();
+
       FontVerticalPositionType position =
           flip_underline_and_overline ? FontVerticalPositionType::TopOfEmHeight
                                       : FontVerticalPositionType::TextTop;
       const int paint_overline_offset =
           decoration_offset.ComputeUnderlineOffsetForUnder(
-              decoration_info.Style().TextUnderlineOffset(),
-              decoration_info.Style().ComputedFontSize(), resolved_thickness,
-              position);
+              line_offset, decoration_info.Style().ComputedFontSize(),
+              resolved_thickness, position);
       decoration_info.SetPerLineData(
           TextDecoration::kOverline, paint_overline_offset,
           -TextDecorationInfo::DoubleOffsetFromThickness(resolved_thickness),

third_party/blink/renderer/core/paint/text_painter_base.cc

@@ -235,11 +235,15 @@ void TextPainterBase::PaintDecorationsExceptLineThrough(
     context.SetStrokeThickness(resolved_thickness);
 
     if (has_underline && decoration_info.FontData()) {
+      // Don't apply text-underline-offset to overline.
+      Length line_offset =
+          flip_underline_and_overline ? Length() : decoration.UnderlineOffset();
+
       const int paint_underline_offset =
           decoration_offset.ComputeUnderlineOffset(
               underline_position, decoration_info.Style().ComputedFontSize(),
-              decoration_info.FontData()->GetFontMetrics(),
-              decoration.UnderlineOffset(), resolved_thickness);
+              decoration_info.FontData()->GetFontMetrics(), line_offset,
+              resolved_thickness);
       decoration_info.SetPerLineData(
           TextDecoration::kUnderline, paint_underline_offset,
           TextDecorationInfo::DoubleOffsetFromThickness(resolved_thickness), 1);
@@ -248,14 +252,17 @@ void TextPainterBase::PaintDecorationsExceptLineThrough(
     }
 
     if (has_overline && decoration_info.FontData()) {
+      // Don't apply text-underline-offset to overline.
+      Length line_offset =
+          flip_underline_and_overline ? decoration.UnderlineOffset() : Length();
+
       FontVerticalPositionType position =
           flip_underline_and_overline ? FontVerticalPositionType::TopOfEmHeight
                                       : FontVerticalPositionType::TextTop;
       const int paint_overline_offset =
           decoration_offset.ComputeUnderlineOffsetForUnder(
-              decoration_info.Style().TextUnderlineOffset(),
-              decoration_info.Style().ComputedFontSize(), resolved_thickness,
-              position);
+              line_offset, decoration_info.Style().ComputedFontSize(),
+              resolved_thickness, position);
       decoration_info.SetPerLineData(
           TextDecoration::kOverline, paint_overline_offset,
           -TextDecorationInfo::DoubleOffsetFromThickness(resolved_thickness),

third_party/blink/web_tests/external/wpt/css/css-text-decor/reference/text-underline-offset-overline-ref.html

+<!DOCTYPE html>
+<html>
+<head>
+    <meta charset="utf-8">
+    <title>Reference result for text-underline-offset test</title>
+    <link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
+    <style>
+        #main {
+            margin: 30px;
+        }
+        #main div {
+            text-decoration: green overline;
+            text-decoration-thickness: 5px;
+            text-decoration-skip-ink: none;
+            font: 20px/1 Ahem;
+        }
+    </style>
+</head>
+<body>
+    <p class="instructions">Test passes if the green overline is not shifted away from black box.</p>
+    <div id="main">
+        <div>XXXX</div>
+    </div>
+</body>
+</html>

third_party/blink/web_tests/external/wpt/css/css-text-decor/reference/text-underline-offset-overline-vertical-ref.html

+<!DOCTYPE html>
+<html>
+<head>
+    <meta charset="utf-8">
+    <title>Reference case for overline placement in vertical</title>
+    <link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
+    <style>
+        #main {
+            margin: 30px;
+        }
+        #main div {
+            writing-mode: vertical-lr;
+            text-decoration: green overline;
+            text-decoration-thickness: 5px;
+            text-decoration-skip-ink: none;
+            font: 20px/1 Ahem;
+        }
+    </style>
+</head>
+<body>
+    <p class="instructions">Test passes if the green overline is not shifted away from black box.</p>
+    <div id="main">
+        <div>XXXX</div>
+    </div>
+</body>
+</html>

third_party/blink/web_tests/external/wpt/css/css-text-decor/reference/text-underline-offset-vertical-003-ref.html

+<!DOCTYPE html>
+<html>
+<head>
+    <meta charset="utf-8">
+    <title>Reference case for text-underline-offset</title>
+    <link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
+    <style>
+        span{
+            margin-left: 5em;
+            font: 20px/1 Ahem;
+            color: transparent;
+            writing-mode: vertical-lr;
+            text-decoration: green underline;
+            text-decoration-skip-ink: none;
+            text-underline-offset: 0;
+            text-underline-position: right;
+        }
+    </style>
+</head>
+<body>
+    <div>Test passes if the underline is vertical and at about a 5em margin from the left side.</div>
+    <div><span>XXXXX</span></div>
+</body>
+</html>

third_party/blink/web_tests/external/wpt/css/css-text-decor/text-underline-offset-overline-vertical.html

+<!DOCTYPE html>
+<html>
+<head>
+    <meta charset="utf-8">
+    <title>text-underline-offset does not affect overline placement in vertical text</title>
+    <meta name="assert" content="text-underline offset does not affect placement of an overline decoration in vertical">
+    <link rel="author" title="Dominik Röttsches" href="mailto:drott@chromium.org">
+    <link rel="help" href="https://drafts.csswg.org/css-text-decor-4/#underline-offset">
+    <link rel="match" href="reference/text-underline-offset-overline-vertical-ref.html">
+    <link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
+    <style>
+        #main {
+            margin: 30px;
+        }
+        #main div {
+            writing-mode: vertical-lr;
+            text-decoration: green overline;
+            text-decoration-thickness: 5px;
+            text-decoration-skip-ink: none;
+            font: 20px/1 Ahem;
+            text-underline-offset: 20px;
+        }
+    </style>
+</head>
+<body>
+    <p class="instructions">Test passes if the green overline is not shifted away from black box.</p>
+    <div id="main">
+        <div>XXXX</div>
+    </div>
+</body>
+</html>

third_party/blink/web_tests/external/wpt/css/css-text-decor/text-underline-offset-overline.html

+<!DOCTYPE html>
+<html>
+<head>
+    <meta charset="utf-8">
+    <title>text-underline-offset does not affect overline placement</title>
+    <meta name="assert" content="text-underline offset does not affect placement of an overline decoration">
+    <link rel="author" title="Dominik Röttsches" href="mailto:drott@chromium.org">
+    <link rel="help" href="https://drafts.csswg.org/css-text-decor-4/#underline-offset">
+    <link rel="match" href="reference/text-underline-offset-overline-ref.html">
+    <link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
+    <style>
+        #main {
+            margin: 30px;
+        }
+        #main div {
+            text-decoration: green overline;
+            text-decoration-thickness: 5px;
+            text-decoration-skip-ink: none;
+            font: 20px/1 Ahem;
+            text-underline-offset: 20px;
+        }
+    </style>
+</head>
+<body>
+    <p class="instructions">Test passes if the green overline is not shifted away from black box.</p>
+    <div id="main">
+        <div>XXXX</div>
+    </div>
+</body>
+</html>

third_party/blink/web_tests/external/wpt/css/css-text-decor/text-underline-offset-vertical-003.html

+<!DOCTYPE html>
+<html>
+<head>
+    <meta charset="utf-8">
+    <title>Test case for text-underline-offset</title>
+    <meta name="assert" content="text-underline-offset should work when the underline is on the right side">
+    <link rel="author" title="Dominik Röttsches" href="mailto:drott@chromium.org">
+    <link rel="help" href="https://drafts.csswg.org/css-text-decor-4/#underline-offset">
+    <link rel="match" href="reference/text-underline-offset-vertical-003-ref.html">
+    <link rel="stylesheet" type="text/css" href="/fonts/ahem.css" />
+    <style>
+        span{
+            font: 20px/1 Ahem;
+            color: transparent;
+            writing-mode: vertical-lr;
+            text-decoration: green underline;
+            text-decoration-skip-ink: none;
+            text-underline-offset: 5em;
+            text-underline-position: right;
+        }
+    </style>
+</head>
+<body>
+    <div>Test passes if the underline is vertical and at about a 5em margin from the left side.</div>
+    <div><span>XXXXX</span></div>
+</body>
+</html>