Merge pull request #607 from Sharktheone/css/delcarations-first-value

fix declarations using only the first value

crates/gosub_css3/src/ast.rs

@@ -1,11 +1,12 @@
+use log::warn;
+
 use crate::node::{Node as CssNode, NodeType};
 use crate::stylesheet::{
     AttributeSelector, Combinator, CssDeclaration, CssRule, CssSelector, CssSelectorPart, CssStylesheet, CssValue,
     MatcherType,
 };
 use gosub_shared::errors::{CssError, CssResult};
 use gosub_shared::traits::css3::CssOrigin;
-use log::warn;
 
 /*
 
@@ -199,9 +200,15 @@ pub fn convert_ast_to_stylesheet(css_ast: &CssNode, origin: CssOrigin, url: &str
                     continue;
                 }
 
+                let value = if css_values.len() == 1 {
+                    css_values.pop().expect("unreachable")
+                } else {
+                    CssValue::List(css_values)
+                };
+
                 rule.declarations.push(CssDeclaration {
                     property: property.clone(),
-                    value: css_values.to_vec(),
+                    value,
                     important: *important,
                 });
             }
@@ -259,7 +266,7 @@ mod tests {
         );
         assert_eq!(
             stylesheet.rules.first().unwrap().declarations.first().unwrap().value,
-            vec![CssValue::String("red".into())]
+            CssValue::String("red".into())
         );
 
         assert_eq!(
@@ -268,11 +275,11 @@ mod tests {
         );
         assert_eq!(
             stylesheet.rules.get(1).unwrap().declarations.first().unwrap().value,
-            vec![
+            CssValue::List(vec![
                 CssValue::Unit(1.0, "px".into()),
                 CssValue::String("solid".into()),
                 CssValue::String("black".into())
-            ]
+            ])
         );
     }
 }

crates/gosub_css3/src/matcher/property_definitions.rs

@@ -858,4 +858,22 @@ mod tests {
             .clone()
             .matches(&[unit!(1.0, "px"), unit!(2.0, "px"), unit!(3.0, "px"), unit!(4.0, "px"),]));
     }
+
+    #[test]
+    fn test_font_var() {
+        let definitions = get_css_definitions();
+        let def = definitions.find_property("font-variation-settings").unwrap();
+
+        assert_true!(def.matches(&[str!("normal")]));
+
+        assert_true!(def.matches(&[str!("wgth"), CssValue::Number(100.0)]));
+
+        assert_true!(def.matches(&[
+            str!("wgth"),
+            CssValue::Number(100.0),
+            CssValue::Comma,
+            str!("ital"),
+            CssValue::Number(100.0)
+        ]));
+    }
 }

crates/gosub_css3/src/stylesheet.rs

@@ -143,7 +143,7 @@ pub struct CssDeclaration {
     pub property: String,
     // Raw values of the declaration. It is not calculated or converted in any way (ie: "red", "50px" etc.)
     // There can be multiple values  (ie:   "1px solid black" are split into 3 values)
-    pub value: Vec<CssValue>,
+    pub value: CssValue,
     // ie: !important
     pub important: bool,
 }
@@ -613,7 +613,7 @@ mod test {
             }],
             declarations: vec![CssDeclaration {
                 property: "color".to_string(),
-                value: vec![CssValue::String("red".to_string())],
+                value: CssValue::String("red".to_string()),
                 important: false,
             }],
         };

crates/gosub_css3/src/system.rs

@@ -15,6 +15,7 @@ use gosub_shared::traits::node::{ElementDataType, Node, TextDataType};
 use gosub_shared::traits::render_tree::{RenderTree, RenderTreeNode};
 use gosub_shared::traits::ParserConfig;
 use log::warn;
+use std::slice;
 
 #[derive(Debug, Clone)]
 pub struct Css3System;
@@ -66,8 +67,14 @@ impl CssSystem for Css3System {
 
                         let value = resolve_functions(&declaration.value, node, handle.clone());
 
+                        let match_value = if let CssValue::List(value) = &value {
+                            &**value
+                        } else {
+                            slice::from_ref(&value)
+                        };
+
                         // Check if the declaration matches the definition and return the "expanded" order
-                        let res = definition.matches_and_shorthands(&value, &mut fix_list);
+                        let res = definition.matches_and_shorthands(match_value, &mut fix_list);
                         if !res {
                             warn!("Declaration does not match definition: {:?}", declaration);
                             continue;
@@ -182,7 +189,7 @@ pub fn add_property_to_map(
     //
     let declaration = DeclarationProperty {
         // @todo: this seems wrong. We only get the first values from the declared values
-        value: declaration.value.first().unwrap().clone(),
+        value: declaration.value.clone(),
         origin: sheet.origin,
         important: declaration.important,
         location: sheet.url.clone(),
@@ -222,27 +229,32 @@ pub fn node_is_unrenderable<D: Document<C>, C: CssSystem>(node: &D::Node) -> boo
 }
 
 pub fn resolve_functions<D: Document<C>, C: CssSystem>(
-    value: &[CssValue],
+    value: &CssValue,
     node: &D::Node,
     handle: DocumentHandle<D, C>,
-) -> Vec<CssValue> {
-    let mut result = Vec::with_capacity(value.len()); //TODO: we could give it a &mut Vec and reuse the allocation
-
-    for val in value {
+) -> CssValue {
+    fn resolve<D: Document<C>, C: CssSystem>(val: &CssValue, node: &D::Node, doc: &D) -> CssValue {
         match val {
             CssValue::Function(func, values) => {
                 let resolved = match func.as_str() {
                     "calc" => resolve_calc(values),
                     "attr" => resolve_attr(values, node),
-                    "var" => resolve_var(values, &*handle.get(), node),
+                    "var" => resolve_var(values, doc, node),
                     _ => vec![val.clone()],
                 };
 
-                result.extend(resolved);
+                CssValue::List(resolved)
             }
-            _ => result.push(val.clone()),
+            _ => val.clone(),
         }
     }
 
-    result
+    let doc = handle.get();
+
+    if let CssValue::List(list) = value {
+        let resolved = list.iter().map(|val| resolve(val, node, &*doc)).collect();
+        CssValue::List(resolved)
+    } else {
+        resolve(value, node, &*doc)
+    }
 }

crates/gosub_html5/Cargo.toml

@@ -8,7 +8,7 @@ license = "MIT"
 [dependencies]
 gosub_shared = { path = "../gosub_shared", features = [] }
 gosub_css3 = { path = "../gosub_css3", features = [] }
-derive_more = { version = "1", features = ["from"] }
+derive_more = { version = "1", features = ["from", "display"] }
 phf = { version = "0.11.2", features = ["macros"] }
 lazy_static = "1.5"
 thiserror = "1.0.64"

crates/gosub_shared/Cargo.toml

@@ -15,7 +15,7 @@ uuid = { version = "1.10.0", features = ["v4"] }
 rand = "0.9.0-alpha.1"
 chardetng = "0.1.17"
 encoding_rs = "0.8.34"
-derive_more = "1.0"
+derive_more = {version = "1.0.0", features = ["display"]}
 
 
 [target.'cfg(target_arch = "wasm32")'.dependencies]