Fix Firefox underline link rendering (#2297)

core/tests/test_v3_underline_css.py

@@ -0,0 +1,135 @@
+"""Guards the cross-browser underline invariant from #2297.
+
+Firefox derives underline thickness and position from the font's own metrics
+whenever those properties are left to the browser. For Mona Sans that seats the
+line inside the descenders, and `text-decoration-skip-ink` then carves a gap
+around every g/p/y, so the underline renders as disconnected fragments and
+heavier than in Chromium.
+
+`static/css/v3/foundations.css` pins the three properties once for every v3
+anchor. These tests fail if a component rule takes that decision back, which is
+the only way the bug can return. They read the stylesheets as text rather than
+rendering anything, so they cost nothing and run in CI with everything else.
+"""
+
+import re
+from pathlib import Path
+
+import pytest
+
+V3_CSS = Path(__file__).resolve().parents[2] / "static" / "css" / "v3"
+
+# Vendored from the boostlook repo and updated wholesale, so it is not ours to
+# hold to this rule.
+EXCLUDED = {"boostlook-v3.css"}
+
+# The declarations that hand the decision back to the browser. A percentage is
+# included because Figma exports these as percentages of the font size, which
+# land on fractional pixels (7.5% of 14px = 1.05px) and round per engine.
+HANDS_BACK_CONTROL = re.compile(
+    r"""
+    (?:text-decoration-thickness|text-underline-offset)\s*:\s*
+        (?:auto|[\d.]+%)
+    | text-underline-position\s*:\s*from-font
+    | text-decoration-skip-ink\s*:\s*auto
+    """,
+    re.VERBOSE,
+)
+
+# `text-decoration` is a shorthand and resets text-decoration-thickness to
+# `auto`. In a :hover or :focus rule it outranks the shared rule, so the bug
+# comes back on hover only, which is the hardest variant to notice. The -line
+# longhand carries the same intent without the reset.
+UNDERLINE_SHORTHAND = re.compile(r"(?<!-)\btext-decoration\s*:\s*[^;]*underline")
+STATE_SELECTOR = re.compile(r":(?:hover|focus|focus-visible|active|visited)")
+
+
+def _without_comments(text):
+    """Blank out /* ... */ comments, preserving newlines so line numbers hold.
+
+    Needed because foundations.css's own warning comment names the very values
+    these tests ban, and would otherwise match.
+    """
+    return re.sub(
+        r"/\*.*?\*/",
+        lambda m: re.sub(r"[^\n]", " ", m.group(0)),
+        text,
+        flags=re.DOTALL,
+    )
+
+
+def _stylesheets():
+    return sorted(p for p in V3_CSS.glob("*.css") if p.name not in EXCLUDED)
+
+
+def _rules(path):
+    """Yield (selector, body) for each rule in a stylesheet.
+
+    Deliberately naive: a regex, not a parser. It is enough for flat component
+    stylesheets and keeps the test dependency-free.
+    """
+    source = _without_comments(path.read_text())
+    for match in re.finditer(r"([^{}]+)\{([^{}]*)\}", source):
+        selector = match.group(1).strip().split("\n")[-1].strip()
+        yield selector, match.group(2)
+
+
+def test_stylesheets_are_discoverable():
+    """A silent glob miss would make every test below pass for the wrong reason."""
+    sheets = _stylesheets()
+    assert len(sheets) > 20, f"only found {len(sheets)} v3 stylesheets in {V3_CSS}"
+    assert (V3_CSS / "foundations.css") in sheets
+
+
+def test_shared_underline_rule_is_present():
+    """The other tests only make sense if the rule they defer to still exists."""
+    foundations = (V3_CSS / "foundations.css").read_text()
+    for prop, value in (
+        ("text-decoration-skip-ink", "none"),
+        ("text-decoration-thickness", "1px"),
+        ("text-underline-offset", "2px"),
+    ):
+        assert re.search(rf"{prop}\s*:\s*{value}", foundations), (
+            f"foundations.css no longer pins {prop}: {value}. If the shared rule "
+            f"moved, point these tests at its new home rather than deleting them."
+        )
+
+
+@pytest.mark.parametrize("path", _stylesheets(), ids=lambda p: p.name)
+def test_no_rule_hands_underline_geometry_back_to_the_browser(path):
+    """No v3 rule may set these to `auto`, a percentage, or `from-font`.
+
+    Any of them reopens #2297 for that component alone, which is why this is
+    worth a test rather than a comment.
+    """
+    offenders = [
+        (i, line.strip())
+        for i, line in enumerate(_without_comments(path.read_text()).splitlines(), start=1)
+        if HANDS_BACK_CONTROL.search(line)
+    ]
+    assert not offenders, (
+        f"{path.name} hands underline geometry back to the browser:\n"
+        + "\n".join(f"  line {i}: {text}" for i, text in offenders)
+        + "\n\nLeave these three properties to the shared rule in foundations.css."
+    )
+
+
+@pytest.mark.parametrize("path", _stylesheets(), ids=lambda p: p.name)
+def test_state_rules_do_not_reset_thickness_via_the_shorthand(path):
+    """A :hover rule using the `text-decoration` shorthand resets thickness.
+
+    It outranks the shared rule, so the underline goes heavy on hover only. Use
+    `text-decoration-line` instead, or pin the thickness in the same rule.
+    """
+    offenders = [
+        selector
+        for selector, body in _rules(path)
+        if STATE_SELECTOR.search(selector)
+        and UNDERLINE_SHORTHAND.search(body)
+        and "text-decoration-thickness" not in body
+    ]
+    assert not offenders, (
+        f"{path.name} resets text-decoration-thickness on a state rule:\n"
+        + "\n".join(f"  {s}" for s in offenders)
+        + "\n\nUse `text-decoration-line: underline`, which does not reset it."
+    )

static/css/v3/auth-page.css

@@ -143,8 +143,6 @@
   letter-spacing: -0.12px;
   color: var(--color-text-primary);
   text-decoration: underline;
-  text-decoration-skip-ink: auto;
-  text-underline-offset: auto;
   width: fit-content;
 }
 

static/css/v3/banner.css

@@ -29,9 +29,6 @@
 .banner__message a {
     text-decoration-line: underline;
     text-decoration-style: solid;
-    text-decoration-skip-ink: auto;
-    text-decoration-thickness: auto;
-    text-underline-offset: auto;
 }
 
 .banner__icon, .banner__close-icon {

static/css/v3/buttons.css

@@ -189,7 +189,7 @@
 }
 
 .btn-card-link:hover {
-  text-decoration: underline;
+  text-decoration-line: underline;
 }
 
 .btn-card-link .btn-icon {

static/css/v3/content.css

@@ -98,12 +98,15 @@ a:hover .content-detail-icon:not(.content-detail-icon--contained) {
 }
 
 .content-detail-icon__cta:hover {
-  text-decoration: underline;
+  text-decoration-line: underline;
+  /* Match .learn-card__link:hover. Without this the hover was inert, since the
+     link is already underlined at rest. */
+  color: var(--color-text-link-accent);
 }
 .content-detail-icon__cta,
 .content-detail-icon__cta:focus,
 .content-detail-icon__cta:focus-visible {
-  text-decoration: underline;
+  text-decoration-line: underline;
   text-decoration-skip-ink: none;
 }
 

static/css/v3/foundations.css

@@ -147,6 +147,30 @@ html.v3 code {
 .border-error { border-color: var(--color-stroke-error); }
 .border-link-accent { border-color: var(--color-stroke-link-accent); }
 
+/* ==========================================================================
+   Underline links (#2297)
+   Left at `auto`, Firefox derives underline thickness and position from the
+   font's own metrics. For Mona Sans that seats the line inside the descenders,
+   and `skip-ink` then carves a gap around every g/p/y, so the underline renders
+   as disconnected fragments and heavier than in Chromium.
+
+   These three properties are inert unless a decoration is actually drawn, so
+   setting them on every v3 anchor is safe and covers links added later. Keep the
+   specificity low so a component can still override deliberately, and do not
+   reintroduce `auto`, a percentage, or `text-underline-position: from-font` in a
+   component rule: any of those puts the bug back for that component only.
+   ========================================================================== */
+body.v3 a,
+/* Non-anchor elements that carry their own underline. .learn-card__link is a
+   span inside the anchor, so the selector above does not reach it. */
+body.v3 .learn-card__link,
+body.v3 .btn-underline,
+body.v3 .dropdown__item--selected {
+  text-decoration-skip-ink: none;
+  text-decoration-thickness: 1px;
+  text-underline-offset: 2px;
+}
+
 /* Container for v3 content */
 .v3-container {
   width: 100%;

static/css/v3/join-card.css

@@ -27,9 +27,12 @@
   text-decoration-line: underline;
   text-decoration-style: solid;
   text-decoration-skip-ink: none;
-  text-decoration-thickness: auto;
-  text-underline-offset: auto;
-  text-underline-position: from-font;
+}
+
+/* Matches .learn-card__link:hover and .content-detail-icon__cta:hover. The link
+   is underlined at rest, so colour is the only affordance left to change. */
+.join-card__start-here:hover {
+  color: var(--color-text-link-accent);
 }
 
 .join-card__title-block {

static/css/v3/learn-cards.css

@@ -87,10 +87,6 @@
     letter-spacing: -0.14px;
     text-decoration-line: underline;
     text-decoration-style: solid;
-    text-decoration-skip-ink: auto;
-    text-decoration-thickness: 7.5%;
-    text-underline-offset: 15.2%;
-    text-underline-position: auto;
     color: var(--color-text-primary);
 }
 

static/css/v3/library-item.css

@@ -47,7 +47,7 @@
 }
 
 a.library-item__name:hover {
-  text-decoration: underline;
+  text-decoration-line: underline;
 }
 
 .library-item p {

static/css/v3/mailing-list-card.css

@@ -94,7 +94,7 @@
 }
 
 .mailing-list-modal__select-all:hover {
-  text-decoration: underline;
+  text-decoration-line: underline;
 }
 
 .mailing-list-modal__list-card {

static/css/v3/markdown-card.css

@@ -33,7 +33,6 @@
     color: var(--color-text-link-accent);
     text-decoration: underline;
     text-decoration-skip-ink: none;
-    text-underline-position: from-font;
 }
 
 .markdown-content {

static/css/v3/post-detail.css

@@ -101,7 +101,6 @@
 .post-detail__external-url a {
   color: var(--color-text-secondary);
   text-decoration: underline;
-  text-underline-position: from-font;
 }
 
 .post-detail__body img {

static/css/v3/v3-examples-section.css

@@ -283,7 +283,7 @@ html.dark .v3-examples-section__toc {
 }
 
 .v3-examples-section__toc-link:hover {
-  text-decoration: underline;
+  text-decoration-line: underline;
 }
 
 .v3-examples-section-horizontal-container {