Page MenuHomePhabricator

"Collapse" link on add/edit reviewers screen is showing weird icons
Closed, ResolvedPublicBUG REPORT

Assigned To
Authored By
Novem_Linguae
Jun 11 2024, 12:12 AM
Referenced Files
F55227881: image.png
Jun 11 2024, 10:28 AM
F55227603: material_icons.png
Jun 11 2024, 10:22 AM
F55201274: image.png
Jun 11 2024, 12:12 AM
F55201260: image.png
Jun 11 2024, 12:12 AM
F55201247: image.png
Jun 11 2024, 12:12 AM

Description

Steps to replicate the issue (include links if applicable):

  • Find one of your own open patches
  • Click the pencil icon by "Reviewers"
  • Add a reviewer

What happens?:

  • Collapse button has extra icons to the left of it
  • These icons are stretched

image.png (132×492 px, 5 KB)

image.png (1×1 px, 115 KB)

What should have happened instead?:

  • No extra icons

image.png (62×196 px, 2 KB)

Software version (on Special:Version page; skip for WMF-hosted wikis like Wikipedia):

Other information (browser name/version, screenshots, etc.):

Event Timeline

This seems to come from a <gr-icon icon="expand_circle_up">, which renders the icon attribute (CSS has content: attr(icon); on the ::before) using a special icon font – and the font renders it as “expand, underscore, circle, underscore, U, P”. Perhaps we have an outdated version of the icon font for some reason? (Some other icons also seem to be written as underscores, e.g. the “attention” icon for a reviewer has icon="label_important" and renders as a single icon – for fun, see what happens if you change the underscore to a hyphen in dev tools).

Yeah, this doesn’t sound great (2022):

/**
 * This file has been produced by downloading this file on Sep 6, 2022:
 * https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:opsz,wght,FILL,GRAD@24,400,0..1,0
 * The corresponding ttf file was downloaded on Sep 6, 2022 from:
 * https://fonts.gstatic.com/s/materialsymbolsoutlined/v51/kJF4BvYX7BgnkSrUwT8OhrdQw4oELdPIeeII9v6oDMzBwG-RpA6RzaxHMPdY40KH8nGzv3fzfVJU22ZZLsYEpzC_1ver5Y0J1Llf.woff2
 */
@font-face {
  font-family: 'Material Symbols Outlined';
  font-style: normal;
  font-weight: 100 700;
  src: url(../fonts/material-icons.woff2) format('woff2');
}

If I visit the given source now, I get a different woff2 link instead which is 62% larger (357444 instead of 220068 bytes).

(Note: I have no idea where that CSS snippet lives in Gerrit’s sources, I just found it in the dev tools. So I don’t know if this is an issue in our installation or in upstream Gerrit.)

The element is a <gr-icon> which has for style:

:host {
  font-family: var(--icon-font-family, 'Material Symbols Outlined');
}
:host::before {
  content: attr(icon);
}

That attr(icon) comes from the parent element which has icon=expand_circle_up , If I change the value to expand_circle_down I get a down arrow!

A reproduction using stable-3.9:

<html>
  <head>
      <style>
@font-face {
  font-family: 'Material Symbols Outlined';
  font-style: normal;
  font-weight: 100 700;
  src: url(lib/fonts/material-icons.woff2) format('woff2');
}
dd {
  font-family: 'Material Symbols Outlined';
}
  </style>
</head>
<body>
<dl>
  <dt>Expand circle up:</dt>
  <dd>expand_circle_up</dd>
  <dt>Expand circle down:</dt>
  <dd>expand_circle_down</dd>
</dl>

Which gives:

material_icons.png (113×233 px, 8 KB)

But the latest font file seems to have the “up” version too:

(same HTML only with the src URL changed)
<html>
  <head>
      <style>
@font-face {
  font-family: 'Material Symbols Outlined';
  font-style: normal;
  font-weight: 100 700;
  src: url(https://fonts.gstatic.com/s/materialsymbolsoutlined/v192/kJF4BvYX7BgnkSrUwT8OhrdQw4oELdPIeeII9v6oDMzBwG-RpA6RzaxHMPdY40KH8nGzv3fzfVJU22ZZLsYEpzC_1ver5Y0.woff2) format('woff2');
}
dd {
  font-family: 'Material Symbols Outlined';
}
  </style>
</head>
<body>
<dl>
  <dt>Expand circle up:</dt>
  <dd>expand_circle_up</dd>
  <dt>Expand circle down:</dt>
  <dd>expand_circle_down</dd>
</dl>

image.png (108×174 px, 4 KB)

The lib/fonts/material-icons.woff2 has NOT changed with the upgrade and is still the same in master.

Looking at upstream Gerrit instance, the font is not embedded in Gerrit but shipped from https://fonts.gstatic.com/s/materialsymbolsoutlined/v192/kJF4BvYX7BgnkSrUwT8OhrdQw4oELdPIeeII9v6oDMzBwG-RpA6RzaxHMPdY40KH8nGzv3fzfVJU22ZZLsYEpzC_1ver5Y0.woff2 and that one works.

The font we have, and the one in the git source code, is https://fonts.gstatic.com/s/materialsymbolsoutlined/v47/kJF4BvYX7BgnkSrUwT8OhrdQw4oELdPIeeII9v6oDMzBwG-RpA6RzaxHMPdY40KH8nGzv3fzHVJU22ZZLsYEpzC_1ver5Y0J1Llf.woff2 which is missing icons.

The reason is Gerrit generated index html has:

resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
{if $useGoogleFonts}
  <link rel="preload" as="style" href="https://fonts.googleapis.com/css?family=Roboto+Mono:400,500,700|Roboto:400,500,700|Open+Sans:400,600,700&display=swap">
  <link rel="preload" as="style" href="https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:opsz,wght,FILL,GRAD@24,400,0..1,0" />{\n}
{else}
  // $useGoogleFonts only exists so that hosts can opt-out of loading fonts from fonts.googleapis.com.
  // fonts.css and the woff2 files in the fonts/ directory are only relevant, if $useGoogleFonts is false.

And that style URL gives me a CSS which has the same font URL has the one from upstream. But to avoid relying on a third party, we disable useGoogleFonts and thus end up with the stall font released in the war.

So, IIUC, upstream should update the font file so it works on installs that don’t useGoogleFonts? (Whereas Google’s own install uses Google fonts and therefore they didn’t notice that the bundled font is outdated.)

So, IIUC, upstream should update the font file so it works on installs that don’t useGoogleFonts? (Whereas Google’s own install uses Google fonts and therefore they didn’t notice that the bundled font is outdated.)

Yes that is correct! :)

It took me a while to test locally with Gerrit 3.8 / 3.9 and my test case above + figuring out how the Google font API works, but eventually I have sent a patch Upstream https://gerrit-review.googlesource.com/c/gerrit/+/429477 Update Material Icons.

It took me a while to test locally with Gerrit 3.8 / 3.9 and my test case above + figuring out how the Google font API works, but eventually I have sent a patch Upstream https://gerrit-review.googlesource.com/c/gerrit/+/429477 Update Material Icons.

Amazing, thanks <3

Change #1042976 had a related patch set uploaded (by Hashar; author: Hashar):

[operations/software/gerrit@deploy/wmf/stable-3.9] Update to a snapshot of Gerrit 3.9.6

https://gerrit.wikimedia.org/r/1042976

Change #1042976 merged by jenkins-bot:

[operations/software/gerrit@deploy/wmf/stable-3.9] Update to a snapshot of Gerrit 3.9.6

https://gerrit.wikimedia.org/r/1042976

Mentioned in SAL (#wikimedia-operations) [2024-06-13T10:08:03Z] <hashar@deploy1002> Started deploy [gerrit/gerrit@ee8252a]: Gerrit to snapshot version 3.9.5-21-g553ea468a1 on gerrit1003 # T367029 T367135

Mentioned in SAL (#wikimedia-operations) [2024-06-13T10:08:09Z] <hashar@deploy1002> Finished deploy [gerrit/gerrit@ee8252a]: Gerrit to snapshot version 3.9.5-21-g553ea468a1 on gerrit1003 # T367029 T367135 (duration: 00m 06s)

Done by updating the Material Icons font that is embedded in Gerrit. I have confirmed the fix worked on our instance. Thank you @Novem_Linguae to have reported it!