| View Issue Details [ Jump to Notes ] | [ Issue History ] [ Print ] | ||||||||
| ID | Project | Category | View Status | Date Submitted | Last Update | ||||
|---|---|---|---|---|---|---|---|---|---|
| 0002818 | LibCSS | LibCSS | public | 2021-05-07 07:32 | 2026-10-05 23:17 | ||||
| Reporter | Ralf Junker | ||||||||
| Assigned To | Michael Drake | ||||||||
| Priority | normal | Severity | major | Reproducibility | always | ||||
| Status | resolved | Resolution | fixed | ||||||
| Summary | 0002818: Regresssion in css_font_resolution_fn: Unit values no longer passed to computed values | ||||||||
| Description | Unit values specified in a css_font_resolution_fn() callback are no longer passed on to the computed style result values. | ||||||||
| Steps To Reproduce | Example 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 Information | The 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. | ||||||||
| Tags | No tags attached. | ||||||||
| Fixed in CI build # | |||||||||
| Reported in CI build # | |||||||||
| Attached Files |
| ||||||||
 Relationships |
|
 Relationships |
| Notes |
|
Michael Drake (administrator) 2021-05-07 10:39 |
Thanks for reporting! I'll look into it this weekend! |
|
Michael Drake (administrator) 2021-05-08 18:05 |
Should be fixed now on master! Thanks for the report! |
Ralf Junker (reporter) 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); |
John-Mark Bell (administrator) 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. |
| 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 | |
| 2026-10-05 23:16 | John-Mark Bell | Note Added: 0002514 | |
| 2026-10-05 23:17 | John-Mark Bell | Status | feedback => resolved |
| 2026-10-05 23:17 | John-Mark Bell | Resolution | reopened => fixed |
| Issue History |