MantisBT - LibCSS
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0002818LibCSSLibCSSpublic2021-05-07 07:322021-05-10 11:40
ReporterRalf Junker 
Assigned ToMichael Drake 
PrioritynormalSeveritymajorReproducibilityalways
StatusfeedbackResolutionreopened 
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.
Relationships
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);

Issue History
Date ModifiedUsernameFieldChange
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