diff --git a/core/tests/test_v3_underline_css.py b/core/tests/test_v3_underline_css.py new file mode 100644 index 000000000..b65524192 --- /dev/null +++ b/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"(? 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." + ) diff --git a/static/css/v3/auth-page.css b/static/css/v3/auth-page.css index d44d08c1e..3c3c3a28f 100644 --- a/static/css/v3/auth-page.css +++ b/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; } diff --git a/static/css/v3/banner.css b/static/css/v3/banner.css index ae3c8bd2b..4ef14107b 100644 --- a/static/css/v3/banner.css +++ b/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 { diff --git a/static/css/v3/buttons.css b/static/css/v3/buttons.css index d42db4444..cc8769437 100644 --- a/static/css/v3/buttons.css +++ b/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 { diff --git a/static/css/v3/content.css b/static/css/v3/content.css index 67b4a9de2..46412183b 100644 --- a/static/css/v3/content.css +++ b/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; } diff --git a/static/css/v3/foundations.css b/static/css/v3/foundations.css index a80db998f..436fdbe1b 100644 --- a/static/css/v3/foundations.css +++ b/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%; diff --git a/static/css/v3/join-card.css b/static/css/v3/join-card.css index 04238f821..d75ec2f07 100644 --- a/static/css/v3/join-card.css +++ b/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 { diff --git a/static/css/v3/learn-cards.css b/static/css/v3/learn-cards.css index 11636a9b9..4bef90743 100644 --- a/static/css/v3/learn-cards.css +++ b/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); } diff --git a/static/css/v3/library-item.css b/static/css/v3/library-item.css index d8812e6fe..467bebebe 100644 --- a/static/css/v3/library-item.css +++ b/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 { diff --git a/static/css/v3/mailing-list-card.css b/static/css/v3/mailing-list-card.css index dff230eb7..d889ee7cb 100644 --- a/static/css/v3/mailing-list-card.css +++ b/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 { diff --git a/static/css/v3/markdown-card.css b/static/css/v3/markdown-card.css index 2a192afa1..288da6e36 100644 --- a/static/css/v3/markdown-card.css +++ b/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 { diff --git a/static/css/v3/post-detail.css b/static/css/v3/post-detail.css index 291fb398b..1e379b137 100644 --- a/static/css/v3/post-detail.css +++ b/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 { diff --git a/static/css/v3/v3-examples-section.css b/static/css/v3/v3-examples-section.css index 0adabaf5e..b3d5860da 100644 --- a/static/css/v3/v3-examples-section.css +++ b/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 {