Check every SVG in gitlab-svgs for a viewBox

What does this MR do?

The svgs:build-and-check job runs packages/gitlab-svgs/build_scripts/svg-lint.sh, which fails the pipeline for an SVG without a viewBox by running grep over **/*.svg. Bash does not enable globstar unless a script asks for it, so **/*.svg behaves like */*.svg and reaches one directory level only: sprite_icons/*.svg, illustrations/*.svg and the top level of dist/, file_icons/ and file_icons_unused/. The check never saw the SVGs in the illustration subdirectories or their optimized copies in dist/illustrations/, and 47 of those source files have no viewBox.

!6444 (merged) adds a step to the illustration export guide that asks for exactly this, a viewBox that matches the width and height, and tells contributors to export into illustrations/ or one of its subdirectories, which is where the check was not looking.

This MR enforces the rule the script already states, and brings the files it was missing into line with it:

  • The check now reads every SVG under packages/gitlab-svgs at any depth, leaving out node_modules, with the same grep pattern and the same messages. After a build it reads 2,501 files instead of 1,343, and every file it read before is still among them.
  • The 47 files get viewBox="0 0 W H", taken from each file's own width and height. Nothing else in them changes: each diff is that one attribute.
  • A patch changeset for @gitlab/svgs.

This is not a fix for a break someone reported. At its own size, viewBox="0 0 W H" maps the drawing one to one onto the box it already had, so nothing moves. What changes is an inline <svg> sized with CSS: without a viewBox the drawing stays at its own size from the top left corner and a smaller box crops it, and with one it scales to fit. As an <img>, Chromium, Firefox and WebKit already scaled the three files I tried without one, so <img> use looks the same.

The built package changes in exactly those 47 optimized files and in the size fields of illustrations_individual.json and logos.json. dist/illustrations.svg is byte for byte the same, because svg-sprite already gave the six sprite entries among the 47 a viewBox of their own size.

The 47 files

Under packages/gitlab-svgs/illustrations/:

  • empty-state/: empty-agent-sessions-md, empty-ai-catalog-md, empty-artifacts-md, empty-artifacts-sm
  • error/: error-404-lg, error-429-lg
  • logos/: aws, bash, clojure, composer, c_plus_plus, crystal, dart, django, docker, elixir, fastlane, flutter, grails, hexo, jekyll, julia, kubernetes, laravel, mono, netlify, node_js, npm, openshift, packer, php, python, ruby, rust, scala, terraform, typo3
  • third-party-logos/: chorus, ci_cd-template-logos/django, civo, gatsby, hotjar, integrations-logos/github, knowbe4, linux, siemens, wish
How I tested it

All in node:22.23.3-trixie, the job's NODE_IMAGE, running the job's own commands from a clean checkout: source scripts/utils.sh, cd packages/gitlab-svgs, retry yarn install --frozen-lockfile, bash ./build_scripts/build-svgs.sh, bash ./build_scripts/svg-lint.sh.

  • On main (be7677381), the current script passes. The new script fails and lists 94 files: the 47 sources and their 47 optimized copies in dist/illustrations/.
  • On this branch the new script passes, which also covers every file under dist/ and file_icons_unused/ at any depth.
  • With main's copy of error/error-404-lg.svg put back and the package rebuilt, it fails again and lists dist/illustrations/error/error-404-lg.svg and illustrations/error/error-404-lg.svg. With the fix restored it passes.
  • In throwaway directories, the new check passes with no SVG at all (xargs -r then runs no grep), fails on a file nested two levels down, names files whose names start with a dash or contain spaces, and ignores a file under node_modules.
  • shellcheck reports nothing on the new script. On main it reports SC2035 twice, for the two **/*.svg globs.
  • Every changed SVG holds exactly one viewBox, equal to 0 0 and its own width and height, and removing that attribute gives back main's file byte for byte.
  • In headless Chrome 154, each file as an <img> at its own size, before and after: 44 of the 47 render pixel for pixel the same. The other three have fractional sizes (kubernetes 722.846 by 701.966, civo 711.364 by 238.193, linux 289.333 by 341.333) and differ in edge anti-aliasing on 310, 378 and 66 pixels, at most 0.22% of the image.
  • In Playwright 1.63.0's Chromium, Firefox and WebKit, empty-artifacts-md, error-404-lg and logos/ruby as <img> elements render pixel for pixel the same before and after: at their own size, at 72 by 72, and at half their height with no width, which is how GlEmptyState sizes an svg-path (its svgHeight defaults to 144, and error-404-lg is 288 high). Inlined and sized to 72 by 72 with CSS, they differ on 772 to 3,331 of the 5,184 pixels: before this change each shows only the top left corner of its drawing at full size, and after it each renders exactly like the 72 by 72 <img>.

Screenshots or screen recordings

None. As an <img>, each file I tried looks the same before and after in Chromium, Firefox and WebKit, at its own size and scaled down, and every reference to these 47 files I found in gitlab-org/gitlab loads them as an image (image_tag, image_path or a webpack URL import). The difference shows only when one of them is inlined and sized with CSS, as described above.

Integrations

No file is added, renamed or removed, so no project needs a migration.

Does this MR meet the acceptance criteria?

This checklist encourages the authors, reviewers, and maintainers of merge requests (MRs) to confirm changes were analyzed for conformity with the project's guidelines, security and accessibility.

Toggle the acceptance checklist

Conformity

  • The “What does this MR do?” section in the MR description is filled out, explaining the reasons for and scope of the proposed changes, per “Say why not just what”.
    • For example, if the MR is focused on usage guidelines, addressing accessibility challenges could be added in a separate MR.
  • Relevant label(s) are applied to the MR.
  • The MR is added to a milestone.
  • Added the ~"component:*" label(s) if applicable.
  • A changeset is added when this MR will do a patch, minor, or major update. More information can be found in doc/changesets.md.

Components

Documentation

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • Security reports checked/validated by a reviewer from the AppSec team

Accessibility

If this MR adds or modifies a component, take a few moments to review the following:

  • All actions and functionality can be done with a keyboard.
  • Links, buttons, and controls have a visible focus state.
  • All content is presented in text or with a text equivalent. For example, alt text for SVG, or aria-label for icons that have meaning or perform actions.
  • Changes in a component’s state are announced by a screen reader. For example, changing aria-expanded="false" to aria-expanded="true" when an accordion is expanded.
  • Color combinations have sufficient contrast.

Merge request reports

Loading
Loading