From f8e6759dde05fb998f5625d05dc81567ede0929a Mon Sep 17 00:00:00 2001 From: Shark Date: Wed, 18 Sep 2024 12:58:35 +0200 Subject: [PATCH] fix declarations using only the first value --- .../gosub_css3/src/convert/ast_converter.rs | 19 +++++++--- crates/gosub_css3/src/stylesheet.rs | 4 +-- crates/gosub_html5/Cargo.toml | 2 +- .../gosub_styling/src/property_definitions.rs | 23 ++++++++++-- crates/gosub_styling/src/render_tree.rs | 35 ++++++++++++------- 5 files changed, 61 insertions(+), 22 deletions(-) diff --git a/crates/gosub_css3/src/convert/ast_converter.rs b/crates/gosub_css3/src/convert/ast_converter.rs index ba05aa2ca..3b3834c96 100644 --- a/crates/gosub_css3/src/convert/ast_converter.rs +++ b/crates/gosub_css3/src/convert/ast_converter.rs @@ -1,11 +1,13 @@ +use anyhow::anyhow; +use log::warn; + +use gosub_shared::types::Result; + use crate::node::{Node as CssNode, NodeType}; use crate::stylesheet::{ AttributeSelector, Combinator, CssDeclaration, CssOrigin, CssRule, CssSelector, CssSelectorPart, CssStylesheet, CssValue, MatcherType, }; -use anyhow::anyhow; -use gosub_shared::types::Result; -use log::warn; /* @@ -208,9 +210,15 @@ pub fn convert_ast_to_stylesheet( 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, }); } @@ -223,10 +231,11 @@ pub fn convert_ast_to_stylesheet( #[cfg(test)] mod tests { - use super::*; use crate::parser_config::ParserConfig; use crate::Css3; + use super::*; + #[test] fn convert_font_family() { let ast = Css3::parse( diff --git a/crates/gosub_css3/src/stylesheet.rs b/crates/gosub_css3/src/stylesheet.rs index 1a4a40a8b..7dfa5d494 100644 --- a/crates/gosub_css3/src/stylesheet.rs +++ b/crates/gosub_css3/src/stylesheet.rs @@ -56,7 +56,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, + pub value: CssValue, // ie: !important pub important: bool, } @@ -472,7 +472,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, }], }; diff --git a/crates/gosub_html5/Cargo.toml b/crates/gosub_html5/Cargo.toml index d398a05ce..731f86819 100644 --- a/crates/gosub_html5/Cargo.toml +++ b/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.61" diff --git a/crates/gosub_styling/src/property_definitions.rs b/crates/gosub_styling/src/property_definitions.rs index 2ed361366..7a8f59d50 100644 --- a/crates/gosub_styling/src/property_definitions.rs +++ b/crates/gosub_styling/src/property_definitions.rs @@ -1,12 +1,11 @@ use std::collections::HashMap; +use std::sync::LazyLock; use log::warn; use gosub_css3::stylesheet::CssValue; use crate::shorthands::{FixList, Shorthands}; -use std::sync::LazyLock; - use crate::syntax::GroupCombinators::Juxtaposition; use crate::syntax::{CssSyntax, SyntaxComponent}; use crate::syntax_matcher::CssSyntaxTree; @@ -892,4 +891,24 @@ mod tests { 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) + ])); + } } diff --git a/crates/gosub_styling/src/render_tree.rs b/crates/gosub_styling/src/render_tree.rs index 4529e3f31..910ef0938 100644 --- a/crates/gosub_styling/src/render_tree.rs +++ b/crates/gosub_styling/src/render_tree.rs @@ -1,5 +1,6 @@ use std::collections::HashMap; use std::fmt::{Debug, Formatter}; +use std::slice; use log::warn; @@ -314,15 +315,22 @@ impl RenderTree { let value = resolve_functions(&declaration.value, node, &doc); + let match_value = if let CssValue::List(value) = &value { + &**value + } else { + slice::from_ref(&value) + }; + + // create property for the given values + let property_name = declaration.property.clone(); + // 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; } - // create property for the given values - let property_name = declaration.property.clone(); let decl = CssDeclaration { property: property_name.to_string(), value, @@ -585,7 +593,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.clone(), important: declaration.important, location: sheet.location.clone(), @@ -964,13 +972,11 @@ pub fn node_is_undernderable(node: &gosub_html5::node::Node) -> bool { } pub fn resolve_functions( - value: &[CssValue], + value: &CssValue, node: &gosub_html5::node::Node, doc: &Document, -) -> Vec { - 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(val: &CssValue, node: &gosub_html5::node::Node, doc: &Document) -> CssValue { match val { CssValue::Function(func, values) => { let resolved = match func.as_str() { @@ -980,13 +986,18 @@ pub fn resolve_functions( _ => vec![val.clone()], }; - result.extend(resolved); + CssValue::List(resolved) } - _ => result.push(val.clone()), + _ => val.clone(), } } - result + 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) + } } // pub fn walk_render_tree(tree: &RenderTree, visitor: &mut Box>) {