2021-09-20 09:53 BST

View Issue Details Jump to Notes ]
IDProjectCategoryView StatusLast Update
0002818LibCSSLibCSSpublic2021-05-10 11:40
ReporterRalf Junker 
Assigned ToMichael Drake 
PrioritynormalSeveritymajorReproducibilityalways
StatusfeedbackResolutionreopened 
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.
Fixed in CI build #
Reported in CI build #
Attached Files

-Relationships
+Relationships

-Notes
Michael Drake

~0002334

Michael Drake (administrator)

Thanks for reporting! I'll look into it this weekend!
Michael Drake

~0002335

Michael Drake (administrator)

Should be fixed now on master! Thanks for the report!
Ralf Junker

~0002336

Ralf Junker (reporter)

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);
+Notes

-Issue History
Date Modified Username Field Change
2021-05-07 07:32 Ralf Junker New Issue
2021-05-07 07:32 Ralf Junker File Added: select.c.modified.zip
2021-05-07 10:39 Michael Drake Note Added: 0002334
2021-05-07 10:39 Michael Drake Status new => acknowledged
2021-05-08 18:05 Michael Drake Assigned To => Michael Drake
2021-05-08 18:05 Michael Drake Status acknowledged => resolved
2021-05-08 18:05 Michael Drake Resolution open => fixed
2021-05-08 18:05 Michael Drake Note Added: 0002335
2021-05-10 11:40 Ralf Junker Status resolved => feedback
2021-05-10 11:40 Ralf Junker Resolution fixed => reopened
2021-05-10 11:40 Ralf Junker Note Added: 0002336
+Issue History