MantisBT - LibCSS
View Issue Details
0002818LibCSSLibCSSpublic2021-05-07 07:322026-10-05 23:17
ReporterRalf Junker 
Assigned ToMichael Drake 
PrioritynormalSeveritymajorReproducibilityalways
StatusresolvedResolutionfixed 
PlatformOSOS Version
Fixed in CI build #
Reported in CI build #
Summary0002818: Regresssion in css_font_resolution_fn: Unit values no longer passed to computed values
DescriptionUnit values specified in a css_font_resolution_fn() callback are no longer passed on to the computed style result values.
Steps To ReproduceExample css_font_resolution_fn() implementation:

css_error css_font_resolution_func(void *pw, lwc_string *name, css_system_font *system_font) {
  system_font->style = CSS_FONT_STYLE_NORMAL;
  system_font->variant = CSS_FONT_VARIANT_NORMAL;
  system_font->weight = CSS_FONT_WEIGHT_NORMAL;
  system_font->size.size = INTTOFIX(22);
  system_font->size.unit = CSS_UNIT_PT; // + (1u << 8);
  system_font->line_height.size = INTTOFIX(33);
  system_font->line_height.unit = CSS_UNIT_EM; // + (1u << 8);
  lwc_intern_string("abcde", 5, &system_font->family);
  return CSS_OK;
}

Attached is the complete source of the modified select.c test, with expected result data.

The pt and em units do not show up in the result. The result unit is always px for all unit types set in the callback.
Additional InformationThe bug dates back to GIT
0b84fa9cc67593667002d7e7953d90400a66ac09, 2020-11-15 which changes the values of the UNIT_XX values so that CSS_UNIT_PT no longer matches UNIT_PT. Function css__to_css_unit() in helpers.h relied on this in the past. Now it no longer matches and always returns 0, which equals CSS_UNIT_PX.

Units are recognized if the new length bit is set. Remove comments from the example above to see the effects.

Suggestion: Assert that only valid values are passed to css__to_css_unit(), or return an error instead of 0 == CSS_UNIT_PX.
TagsNo tags attached.
Attached Fileszip select.c.modified.zip (7,972) 2021-05-07 07:32
https://bugs.netsurf-browser.org/mantis/file_download.php?file_id=703&type=bug

Notes
(0002334)
Michael Drake   
2021-05-07 10:39   
Thanks for reporting! I'll look into it this weekend!
(0002335)
Michael Drake   
2021-05-08 18:05   
Should be fixed now on master! Thanks for the report!
(0002336)
Ralf Junker   
2021-05-10 11:40   
Thanks for the fix. One suggestion, if I may:

Instead of GIT be9acc6c80dfc41974425824b77585e45e4498e9, it is more sufficient not to increase the reference of system_font->family in parse_system_font().

libcss\src\parse\properties\font.c, line 197:

error = css__stylesheet_string_add(c->sheet, /* Ralf Junker removed: lwc_string_ref( */ system_font->family /* ) */, &snumber);
(0002514)
John-Mark Bell   
2026-10-05 23:16   
No, that is not safe. css__parse_font will release the reference on the client-provided string (noting that it has to do this to conform to that interface and also has no way of knowing if parse_system_font() has actually got as far as trying to insert the font family into the stylesheet's string table). So, the reason that parse_system_font() takes an additional reference before inserting the string into the table is because that additional reference is then owned by the stylesheet (even if the insertion fails). If it did not do this, then the string would potentially be destroyed when css__parse_font subsequently unrefs it -- this would then leave a dangling pointer in the stylesheet's string table.

Issue History
2021-05-07 07:32Ralf JunkerNew Issue
2021-05-07 07:32Ralf JunkerFile Added: select.c.modified.zip
2021-05-07 10:39Michael DrakeNote Added: 0002334
2021-05-07 10:39Michael DrakeStatusnew => acknowledged
2021-05-08 18:05Michael DrakeAssigned To => Michael Drake
2021-05-08 18:05Michael DrakeStatusacknowledged => resolved
2021-05-08 18:05Michael DrakeResolutionopen => fixed
2021-05-08 18:05Michael DrakeNote Added: 0002335
2021-05-10 11:40Ralf JunkerStatusresolved => feedback
2021-05-10 11:40Ralf JunkerResolutionfixed => reopened
2021-05-10 11:40Ralf JunkerNote Added: 0002336
2026-10-05 23:16John-Mark BellNote Added: 0002514
2026-10-05 23:17John-Mark BellStatusfeedback => resolved
2026-10-05 23:17John-Mark BellResolutionreopened => fixed