Error changing logo to SVG with prefers-color-scheme styling

[pboguslawski]

Description

When changing logo to SVG file that contains dark mode styling i.e.

<?xml version="1.0" standalone="no"?>
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20">
  <style>
    path { fill: black }
    @media (prefers-color-scheme: dark) { path { fill: white } }
  </style>
[...]

make generate-images throws error like

$ make generate-images
npm install --no-save --no-package-lock fabric@5 imagemin-zopfli@7

up to date in 44s
node build/generate-images.js 
TypeError: Cannot read properties of undefined (reading 'trim')
    at /mypath/gitea/node_modules/fabric/dist/fabric.js:5434:33
    at Array.forEach (<anonymous>)
    at Object.getCSSRules (/mypath/gitea/node_modules/fabric/dist/fabric.js:5425:15)
    at fabric.parseSVGDocument (/mypath/gitea/node_modules/fabric/dist/fabric.js:5150:38)
    at Object.loadSVGFromString (/mypath/gitea/node_modules/fabric/dist/fabric.js:5499:14)
    at file:///mypath/gitea/build/generate-images.js:14:12
    at new Promise (<anonymous>)
    at loadSvg (file:///mypath/gitea/build/generate-images.js:13:10)
    at generate (file:///mypath/gitea/build/generate-images.js:38:36)
    at main (file:///mypath/gitea/build/generate-images.js:73:5)
make: *** [Makefile:1013: generate-images] Error 1

No such problem when

@media (prefers-color-scheme: dark) { path { fill: white } }

is removed from svg file.

Gitea Version

1.21.3

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

Compiled from source.

Database

None

[pboguslawski]

Dirty workaround that works for us: use svg file version with @media... line removed for generate-images and than override public/assets/img/logo.svg and public/assets/img/favicon.svg with unmodified svg version (with @media... line) before building gitea.

[silverwind]

Sounds like a bug in fabric, could try v6 beta by changing fabric@5 to fabric@beta in the Makefile.

https://github.com/fabricjs/fabric.js?tab=readme-ov-file#migrating-to-v6

[pboguslawski]

After switching to farbic@beta:

$ make generate-images
npm install --no-save

removed 271 packages, and changed 149 packages in 4s
npm install --no-save --no-package-lock fabric@beta imagemin-zopfli@7
npm WARN deprecated uuid@3.4.0: Please upgrade  to version 7 or higher.  Older versions may use Math.random() in certain circumstances, which is known to be problematic.  See https://v8.dev/blog/math-random for details.

added 263 packages, and changed 149 packages in 3m
node build/generate-images.js 
file://mypath/gitea/build/generate-images.js:4
import {fabric} from 'fabric';
        ^^^^^^
SyntaxError: The requested module 'fabric' does not provide an export named 'fabric'
    at ModuleJob._instantiate (node:internal/modules/esm/module_job:132:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:214:5)
    at async ModuleLoader.import (node:internal/modules/esm/loader:329:24)
    at async loadESM (node:internal/process/esm_loader:28:7)
    at async handleMainPromise (node:internal/modules/run_main:113:12)

Node.js v20.11.1
make: *** [Makefile:1013: generate-images] Error 1

BTW: shouldn't gitea use npm ci when downloading dependencies instead of npm install?

[silverwind]

Sounds like there are API changes, but we can adapt. Can you share a minimal full SVG that reproduces the error?

BTW: shouldn't gitea use npm ci when downloading dependencies instead of npm install?

npm install --no-save is practically the same. I avoid npm ci because last I tried it failed in strange ways that npm install does not.

npm ci would not work in this case anyways as we don't have these two modules in package.json because of their native dependencies which bring too many issues when they are in package.json.

[pboguslawski]

Can you share a minimal full SVG that reproduces the error?

This SVG fails:

<?xml version="1.0" standalone="no"?>
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20">
  <style>
    circle { fill: black; }
    @media (prefers-color-scheme: dark) { circle { fill: white; } }
  </style>
  <circle r="10" cx="10" cy="10" />
</svg>

Then remove

    @media (prefers-color-scheme: dark) { circle { fill: white; } }

and it works fine.

we don't have these two modules in package.json

For reproducibility & security consider using only dependency versions that are stored in package-lock.json with checksum verification on installation (which npm ci quarantees if I understand correctly and npm install does not).

[silverwind]

It's a bug in fabric: fabricjs/fabric.js#9679

#29334 will upgrade us to fabric v6, which is good to do in any case.

For reproducibility & security consider using only dependency versions that are stored in package-lock.json with checksum verification on installation (which npm ci quarantees if I understand correctly and npm install does not).

Can't have unstable native dependencies like canvas in our main package-lock.json or else we risk breaking the build when those dependencies fail to compile or have no prebuilds available which is very common. The current version lock via command line is a good enough imho.

[silverwind]

Bug is confirmed and one workaround may be to remove the prefers-color-scheme media query before handing to fabric, but doing that will require a dom/xml parser (likely JSDOM's DOMParser) and a css parsers (postcss). We must of course preserve it for SVG output, but for PNG output, such a media query does not have any effect.

[silverwind]

Upstream merge was fixed, waiting for next (beta) release.