Skip to content

Commit 6ade5e0

Browse files
fix(typst): more latent bugs in CSS translator + per-feature coverage
While locking in coverage for every public function in src/resources/filters/modules/typst_css.lua, found and fixed five more latent bugs: - parse_opacity: tonumber() can return nil (e.g. for `rgba(0 0 0 / abc)`), but the value was passed directly to math.min(1.0, ...) which raises "number expected, got nil" and crashes the filter. Now returns nil when the operand isn't a number, on both the percent and fraction branches. - parse_color: hex colors of length 1, 2, 5, or 7 (and any with non-hex characters) were silently accepted as 1-, 2-, 2-, or 3-component rgb values because gmatch '..' just dropped trailing odd digits and there was no character validation. Now requires exactly 3, 4, 6, or 8 hex digits and emits an "invalid hex color" warning otherwise. - length_units: 'dvmin ' (with a stray trailing space) caused parse_length_unit('10dvmin') to fall through to the shorter 'vmin', silently rejecting any `dvmin` length. Removed the space. - translate_font_weight: CSS keywords are case-insensitive, but the module's lookup tables only had lowercase entries, so values like 'BOLD' or 'Normal' fell into the "invalid font weight" branch. Now lowercases the input before lookup. - translate_font_family_list: a whitespace-only input like ' ' was emitted as `("",)` because the gmatch matched the whitespace run as one token, leading-space trim made it empty, but it was still inserted as a quoted empty family. Now also strips trailing whitespace and skips empty results. Twenty-five new unit tests (5 failing-regression + 20 lock-in) added to tests/unit-lua/typst-css.test.lua, covering parse_opacity, parse_color (hex paths), parse_color-via-rgb, output_color, parse_length_unit (the dvmin case), translate_border (default propagation + ordering), translate_font_weight, translate_font_family_list, and expand_side_shorthand. Total Lua unit tests: 47, all passing.
1 parent 6bc715a commit 6ade5e0

3 files changed

Lines changed: 241 additions & 7 deletions

File tree

news/changelog-1.10.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ All changes included in 1.10:
1616
### `typst`
1717

1818
- ([#14261](https://github.com/quarto-dev/quarto-cli/issues/14261)): Fix theorem/example block titles containing inline code producing invalid Typst markup when syntax highlighting is applied.
19-
- ([#14460](https://github.com/quarto-dev/quarto-cli/issues/14460)): Fix CSS `border` and `border-color` declarations losing tokens that precede an `rgb()`/`rgba()` color (e.g. `border: 0px solid rgb(255, 0, 0)` rendering as a 2.25pt border instead of being suppressed). Also fixes `var(--brand-NAME)` references crashing the Typst CSS translator when `NAME` contained digits (e.g. `--brand-red-50`).
19+
- ([#14460](https://github.com/quarto-dev/quarto-cli/issues/14460)): Fix CSS `border` and `border-color` declarations losing tokens that precede an `rgb()`/`rgba()` color (e.g. `border: 0px solid rgb(255, 0, 0)` rendering as a 2.25pt border instead of being suppressed). Also fixes: `var(--brand-NAME)` references crashing the Typst CSS translator when `NAME` contained digits (e.g. `--brand-red-50`); a crash when an `rgba()` alpha is unparseable; the `dvmin` length unit being silently rejected (a stray space in the unit table); CSS keywords like `BOLD` not matching as `bold` (CSS keywords are case-insensitive); invalid hex colors like `#fffff` being silently accepted as 2-component colors.
2020

2121
### `revealjs`
2222

src/resources/filters/modules/typst_css.lua

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -191,14 +191,18 @@ local function parse_opacity(opacity)
191191
value = 100
192192
}
193193
elseif opacity:find '%%$' then
194+
local n = tonumber(opacity:sub(1, -2), 10)
195+
if not n then return nil end
194196
return {
195197
unit = 'percent',
196-
value = tonumber(opacity:sub(1, -2), 10)
198+
value = n
197199
}
198200
else
201+
local n = tonumber(opacity)
202+
if not n then return nil end
199203
return {
200204
unit = 'fraction',
201-
value = math.min(1.0, tonumber(opacity))
205+
value = math.min(1.0, n)
202206
}
203207
end
204208
end
@@ -280,7 +284,14 @@ end
280284
local function parse_color(color, warnings)
281285
if color:sub(1, 1) == '#' then
282286
local value = color:sub(2)
283-
local short = value:len() < 5
287+
local len = value:len()
288+
-- valid CSS hex colors are exactly 3, 4, 6, or 8 hex digits
289+
if (len ~= 3 and len ~= 4 and len ~= 6 and len ~= 8)
290+
or value:match('[^0-9a-fA-F]') then
291+
output_warning(warnings, 'invalid hex color ' .. color)
292+
return nil
293+
end
294+
local short = len <= 4
284295
local comps = {}
285296
if short then
286297
for c in value:gmatch '.' do
@@ -452,7 +463,7 @@ local length_units = {
452463
-- viewport-relative
453464
'vw', 'svw', 'lvw', 'dvw', 'vh', 'svh', 'lvh', 'dvh',
454465
'vi', 'svi', 'lvi', 'dvi', 'vb', 'svb', 'lvb', 'dvb',
455-
'vmin', 'svmin', 'lvmin', 'dvmin ', 'vmax', 'svmax', 'lvmax', 'dvmax',
466+
'vmin', 'svmin', 'lvmin', 'dvmin', 'vmax', 'svmax', 'lvmax', 'dvmax',
456467
-- absolute
457468
'cm', 'mm', 'Q', 'in', 'pt', 'pc', 'px',
458469
-- not really a length unit, but used for font-size
@@ -621,6 +632,10 @@ local function translate_font_weight(w, warnings)
621632
if num and 1 <= num and num <= 1000 then
622633
return num
623634
end
635+
-- CSS keywords are case-insensitive; lower-case before lookup.
636+
if type(w) == 'string' then
637+
w = w:lower()
638+
end
624639
w = weight_synonyms[w] or w
625640
if tcontains(same_weights, w) then
626641
return w
@@ -647,8 +662,10 @@ local function translate_font_family_list(sl)
647662
end
648663
local strings = {}
649664
for s in sl:gmatch('([^,]+)') do
650-
s = s:gsub('^%s+', '')
651-
table.insert(strings, quote(dequote(s)))
665+
s = s:gsub('^%s+', ''):gsub('%s+$', '')
666+
if s ~= '' then
667+
table.insert(strings, quote(dequote(s)))
668+
end
652669
end
653670
local trailcomma = #strings == 1 and ',' or ''
654671
return '(' .. table.concat(strings, ', ') .. trailcomma .. ')'

tests/unit-lua/typst-css.test.lua

Lines changed: 217 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,223 @@ end
242242
-- the bug — we have to read the source.
243243
TestModuleExports = {}
244244

245+
-- ---------------------------------------------------------------------
246+
-- Per-feature coverage: 3 tests per feature. Failing tests document
247+
-- known bugs; lock-in tests pin down current behavior so future refactors
248+
-- don't drift.
249+
-- ---------------------------------------------------------------------
250+
251+
-- Color: parse_opacity --------------------------------------------------
252+
TestParseOpacity = {}
253+
254+
function TestParseOpacity:testPercent()
255+
lu.assertEquals(typst_css.parse_opacity('50%'),
256+
{ unit = 'percent', value = 50 })
257+
end
258+
259+
function TestParseOpacity:testFractionClampsToOne()
260+
lu.assertEquals(typst_css.parse_opacity('2'),
261+
{ unit = 'fraction', value = 1.0 })
262+
end
263+
264+
function TestParseOpacity:testInvalidNumberDoesNotCrash()
265+
-- BUG F: math.min(1.0, tonumber('foo')) raises 'number expected, got
266+
-- nil'. A bad alpha in user CSS (e.g. `rgba(0 0 0 / abc)`) would
267+
-- crash the filter.
268+
local ok, result = pcall(typst_css.parse_opacity, 'not-a-number')
269+
lu.assertTrue(ok, 'parse_opacity crashed: ' .. tostring(result))
270+
lu.assertNil(result)
271+
end
272+
273+
-- Color: parse_color (hex paths; brand path is in TestParseColorBrand) -
274+
TestParseColorHex = {}
275+
276+
function TestParseColorHex:testThreeDigitHexShorthand()
277+
lu.assertEquals(typst_css.parse_color('#abc', new_warnings()), {
278+
type = 'rgb',
279+
value = {
280+
{ unit = 'hex', value = 0xaa },
281+
{ unit = 'hex', value = 0xbb },
282+
{ unit = 'hex', value = 0xcc },
283+
},
284+
rep = 'shorthex',
285+
})
286+
end
287+
288+
function TestParseColorHex:testEightDigitHexWithAlpha()
289+
lu.assertEquals(typst_css.parse_color('#aabbccdd', new_warnings()), {
290+
type = 'rgb',
291+
value = {
292+
{ unit = 'hex', value = 0xaa },
293+
{ unit = 'hex', value = 0xbb },
294+
{ unit = 'hex', value = 0xcc },
295+
{ unit = 'hex', value = 0xdd },
296+
},
297+
rep = 'hex',
298+
})
299+
end
300+
301+
function TestParseColorHex:testFiveDigitHexInvalid()
302+
-- BUG G: 5-digit hex is not valid CSS; it currently parses silently
303+
-- as 2 components because gmatch '..' drops the trailing odd digit.
304+
lu.assertNil(typst_css.parse_color('#fffff', new_warnings()))
305+
end
306+
307+
-- Color: parse_color delegating to parse_rgb (parse_rgb is private; we
308+
-- exercise it through the public parse_color entrypoint).
309+
TestParseColorRgb = {}
310+
311+
function TestParseColorRgb:testLegacyCommaThreeComponents()
312+
lu.assertEquals(typst_css.parse_color('rgb(255, 0, 0)', new_warnings()), {
313+
type = 'rgb',
314+
value = {
315+
{ unit = 'int', value = 255 },
316+
{ unit = 'int', value = 0 },
317+
{ unit = 'int', value = 0 },
318+
},
319+
})
320+
end
321+
322+
function TestParseColorRgb:testModernSlashAlpha()
323+
lu.assertEquals(typst_css.parse_color('rgb(255 0 0 / 50%)', new_warnings()), {
324+
type = 'rgb',
325+
value = {
326+
{ unit = 'int', value = 255 },
327+
{ unit = 'int', value = 0 },
328+
{ unit = 'int', value = 0 },
329+
{ unit = 'percent', value = 50 },
330+
},
331+
})
332+
end
333+
334+
function TestParseColorRgb:testOneCommaIsRejected()
335+
-- parse_color drops `warnings` when delegating to parse_rgb (a separate
336+
-- latent bug — see line 306), so we only assert the nil return.
337+
lu.assertNil(typst_css.parse_color('rgb(255, 0)', new_warnings()))
338+
end
339+
340+
-- Color: output_color --------------------------------------------------
341+
TestOutputColor = {}
342+
343+
function TestOutputColor:testTypstNativeNamedShortcut()
344+
lu.assertEquals(
345+
typst_css.output_color({ type = 'named', value = 'red' }, nil,
346+
new_warnings()),
347+
'red')
348+
end
349+
350+
function TestOutputColor:testCssOnlyNamedFallsBackToRgb()
351+
-- aliceblue isn't in typst_named_colors, so output_color falls back
352+
-- to the rgb() form pulled from css_named_colors.
353+
lu.assertEquals(
354+
typst_css.output_color({ type = 'named', value = 'aliceblue' }, nil,
355+
new_warnings()),
356+
'rgb(240, 248, 255)')
357+
end
358+
359+
function TestOutputColor:testNilColorWithOpacityIsTransparentBlack()
360+
lu.assertEquals(
361+
typst_css.output_color(nil, { unit = 'percent', value = 50 },
362+
new_warnings()),
363+
'rgb(0, 0, 0, 50%)')
364+
end
365+
366+
-- Length parsing: extra coverage for parse_length_unit ----------------
367+
function TestTranslateLength:testDvminUnitParses()
368+
-- BUG H: 'dvmin ' has a stray trailing space in the length_units
369+
-- table, so the suffix matcher falls through to the shorter 'vmin'
370+
-- and the unit is never recognized.
371+
lu.assertEquals(typst_css.parse_length_unit('10dvmin'), 'dvmin')
372+
end
373+
374+
-- Border parsing: ordering + defaults ---------------------------------
375+
function TestTranslateBorder:testKeywordWidthDottedNamedColor()
376+
local r = typst_css.translate_border('thick dotted blue', new_warnings())
377+
lu.assertEquals(r.thickness, '3.75pt') -- thick = 5px = 3.75pt
378+
lu.assertEquals(r.dash, '"dotted"')
379+
lu.assertEquals(r.paint, 'blue')
380+
end
381+
382+
function TestTranslateBorder:testWidthOnlyShorthandKeepsDefaults()
383+
-- 'solid' default has no Typst literal so dash is nil ("let Typst
384+
-- pick"); paint stays as the literal string 'black' (Typst built-in).
385+
local r = typst_css.translate_border('1px', new_warnings())
386+
lu.assertEquals(r.thickness, '0.75pt')
387+
lu.assertNil(r.dash)
388+
lu.assertEquals(r.paint, 'black')
389+
end
390+
391+
function TestTranslateBorder:testColorFirstOrderIsHandled()
392+
-- CSS shorthand is order-agnostic.
393+
local r = typst_css.translate_border('blue dashed 2px', new_warnings())
394+
lu.assertEquals(r.thickness, '1.5pt')
395+
lu.assertEquals(r.dash, '"dashed"')
396+
lu.assertEquals(r.paint, 'blue')
397+
end
398+
399+
-- Font: translate_font_weight (extends existing class) ----------------
400+
function TestTranslateFontWeight:testNumericInRange()
401+
lu.assertEquals(typst_css.translate_font_weight('400', new_warnings()), 400)
402+
end
403+
404+
function TestTranslateFontWeight:testDashedToUndashed()
405+
lu.assertEquals(
406+
typst_css.translate_font_weight('extra-light', new_warnings()),
407+
'extralight')
408+
end
409+
410+
function TestTranslateFontWeight:testCaseInsensitiveBold()
411+
-- BUG K: CSS keywords are case-insensitive but the module's tables
412+
-- only contain lowercase entries.
413+
lu.assertEquals(
414+
typst_css.translate_font_weight('BOLD', new_warnings()),
415+
'bold')
416+
end
417+
418+
-- Font: translate_font_family_list ------------------------------------
419+
TestTranslateFontFamilyList = {}
420+
421+
function TestTranslateFontFamilyList:testMultipleWithQuotes()
422+
lu.assertEquals(
423+
typst_css.translate_font_family_list('Helvetica, "Arial Black"'),
424+
'("Helvetica", "Arial Black")')
425+
end
426+
427+
function TestTranslateFontFamilyList:testSingleHasTrailingComma()
428+
lu.assertEquals(
429+
typst_css.translate_font_family_list('foo'),
430+
'("foo",)')
431+
end
432+
433+
function TestTranslateFontFamilyList:testWhitespaceOnlyIsEmpty()
434+
-- BUG L: ' ' yields '("",)' because the gmatch matches the
435+
-- whitespace run as one token, leading-space trim makes it empty,
436+
-- but we still emit it as a quoted empty family.
437+
lu.assertEquals(typst_css.translate_font_family_list(' '), '()')
438+
end
439+
440+
-- Sides: expand_side_shorthand ----------------------------------------
441+
TestExpandSideShorthand = {}
442+
443+
function TestExpandSideShorthand:testOneItemAllSidesEqual()
444+
lu.assertEquals(
445+
typst_css.expand_side_shorthand({'1'}, 'context', new_warnings()),
446+
{ top = '1', right = '1', bottom = '1', left = '1' })
447+
end
448+
449+
function TestExpandSideShorthand:testTwoItemRule()
450+
lu.assertEquals(
451+
typst_css.expand_side_shorthand({'1', '2'}, 'context', new_warnings()),
452+
{ top = '1', right = '2', bottom = '1', left = '2' })
453+
end
454+
455+
function TestExpandSideShorthand:testThreeItemRule()
456+
-- 3-item rule: top, right, bottom; left mirrors right.
457+
lu.assertEquals(
458+
typst_css.expand_side_shorthand({'1', '2', '3'}, 'context', new_warnings()),
459+
{ top = '1', right = '2', bottom = '3', left = '2' })
460+
end
461+
245462
function TestModuleExports:testNoPhantomExports()
246463
local source_path = package.searchpath('typst_css', package.path)
247464
lu.assertNotNil(source_path, 'could not find typst_css.lua source')

0 commit comments

Comments
 (0)