commit d0f3055eed7eaf6088de2f8314fa55ffa63ade08
parent 2fa8cf93bd86d9e32c8350030d24e18a6a505f44
Author: agoloman <agoloman@mozilla.com>
Date: Wed, 19 Nov 2025 19:30:37 +0200
Revert "Bug 1986627 - Parse attr function as a substitution function when generalized attr is preffed on. r=layout-reviewers,firefox-style-system-reviewers,emilio" for causing wr failures @variable-generated-content-dynamic-001.html.
This reverts commit f5edda047abe2757d89bb0b14651cb1a7de2c082.
Revert "Bug 1986627 - Add preference for parsing css-attr. r=layout-reviewers,emilio"
This reverts commit 3644366ec62cdcef51a632ab489531229dc8357f.
Diffstat:
3 files changed, 55 insertions(+), 92 deletions(-)
diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml
@@ -9859,16 +9859,6 @@
mirror: always
rust: true
-# Whether to enable generalized CSS Attr Support. If this pref is set to false
-# we treat attr as a regular function with support only for the `content` css
-# property. If it is set to true, attr is parsed as a subtitution function.
-# https://drafts.csswg.org/css-values-5/#attr-notation
-- name: layout.css.attr.enabled
- type: RelaxedAtomicBool
- value: false
- mirror: always
- rust: true
-
# Whether to enable CSS @custom-media
# https://drafts.csswg.org/mediaqueries-5/
# TODO(emilio): Needs more tests before enabling.
diff --git a/servo/components/style/custom_properties.rs b/servo/components/style/custom_properties.rs
@@ -458,13 +458,6 @@ pub enum DeferFontRelativeCustomPropertyResolution {
No,
}
-#[derive(Clone, Debug, MallocSizeOf, PartialEq, ToShmem, Parse)]
-enum SubstitutionFunctionKind {
- Var,
- Env,
- Attr,
-}
-
#[derive(Clone, Debug, MallocSizeOf, PartialEq, ToShmem)]
struct VariableFallback {
start: num::NonZeroUsize,
@@ -473,25 +466,24 @@ struct VariableFallback {
}
#[derive(Clone, Debug, MallocSizeOf, PartialEq, ToShmem)]
-struct SubstitutionFunctionReference {
+struct VarOrEnvReference {
name: Name,
start: usize,
end: usize,
fallback: Option<VariableFallback>,
prev_token_type: TokenSerializationType,
next_token_type: TokenSerializationType,
- substitution_kind: SubstitutionFunctionKind,
+ is_var: bool,
}
/// A struct holding information about the external references to that a custom
/// property value may have.
#[derive(Clone, Debug, Default, MallocSizeOf, PartialEq, ToShmem)]
struct References {
- refs: Vec<SubstitutionFunctionReference>,
+ refs: Vec<VarOrEnvReference>,
non_custom_references: NonCustomReferences,
any_env: bool,
any_var: bool,
- any_attr: bool,
}
impl References {
@@ -792,33 +784,32 @@ fn parse_declaration_value_block<'i, 't>(
return Err(input.new_custom_error(e));
},
Token::Function(ref name) => {
- let substitution_kind = SubstitutionFunctionKind::from_ident(name).ok();
- if let Some(substitution_kind) = substitution_kind {
+ let is_var = name.eq_ignore_ascii_case("var");
+ if is_var || name.eq_ignore_ascii_case("env") {
let our_ref_index = references.refs.len();
let fallback = input.parse_nested_block(|input| {
// TODO(emilio): For env() this should be <custom-ident> per spec, but no other browser does
// that, see https://github.com/w3c/csswg-drafts/issues/3262.
let name = input.expect_ident()?;
- let name =
- Atom::from(if substitution_kind == SubstitutionFunctionKind::Var {
- match parse_name(name.as_ref()) {
- Ok(name) => name,
- Err(()) => {
- let name = name.clone();
- return Err(input.new_custom_error(
- SelectorParseErrorKind::UnexpectedIdent(name),
- ));
- },
- }
- } else {
- name.as_ref()
- });
+ let name = Atom::from(if is_var {
+ match parse_name(name.as_ref()) {
+ Ok(name) => name,
+ Err(()) => {
+ let name = name.clone();
+ return Err(input.new_custom_error(
+ SelectorParseErrorKind::UnexpectedIdent(name),
+ ));
+ },
+ }
+ } else {
+ name.as_ref()
+ });
// We want the order of the references to match source order. So we need to reserve our slot
// now, _before_ parsing our fallback. Note that we don't care if parsing fails after all, since
// if this fails we discard the whole result anyways.
let start = token_start.byte_index() - input_start.byte_index();
- references.refs.push(SubstitutionFunctionReference {
+ references.refs.push(VarOrEnvReference {
name,
start,
// To be fixed up after parsing fallback and auto-closing via our_ref_index.
@@ -828,7 +819,7 @@ fn parse_declaration_value_block<'i, 't>(
next_token_type: TokenSerializationType::Nothing,
// To be fixed up after parsing fallback.
fallback: None,
- substitution_kind: substitution_kind.clone(),
+ is_var,
});
let mut fallback = None;
@@ -872,11 +863,11 @@ fn parse_declaration_value_block<'i, 't>(
reference.end = input.position().byte_index() - input_start.byte_index()
+ missing_closing_characters.len();
reference.fallback = fallback;
- match substitution_kind {
- SubstitutionFunctionKind::Var => references.any_var = true,
- SubstitutionFunctionKind::Env => references.any_env = true,
- SubstitutionFunctionKind::Attr => references.any_attr = true,
- };
+ if is_var {
+ references.any_var = true;
+ } else {
+ references.any_env = true;
+ }
} else {
nested!();
check_closed!(")");
@@ -1047,7 +1038,6 @@ impl<'a, 'b: 'a> CustomPropertiesBuilder<'a, 'b> {
// Non-custom dependency is really relevant for registered custom properties
// that require computed value of such dependencies.
let has_dependency = unparsed_value.references.any_var
- || unparsed_value.references.any_attr
|| find_non_custom_references(
registration,
unparsed_value,
@@ -1132,7 +1122,7 @@ impl<'a, 'b: 'a> CustomPropertiesBuilder<'a, 'b> {
_ => return,
};
- if !refs.any_var && !refs.any_attr {
+ if !refs.any_var {
return;
}
@@ -1161,7 +1151,7 @@ impl<'a, 'b: 'a> CustomPropertiesBuilder<'a, 'b> {
.refs
.iter()
.filter_map(|reference| {
- if reference.substitution_kind != SubstitutionFunctionKind::Var {
+ if !reference.is_var {
return None;
}
let registration = self
@@ -1517,9 +1507,7 @@ fn substitute_all(
/* include_unregistered = */ true,
);
context.non_custom_references |= non_custom_refs.unwrap_or_default();
- let has_dependency = value.references.any_var
- || value.references.any_attr
- || non_custom_refs.is_some();
+ let has_dependency = value.references.any_var || non_custom_refs.is_some();
// Nothing to resolve.
if !has_dependency {
debug_assert!(!value.references.any_env, "Should've been handled earlier");
@@ -1617,7 +1605,7 @@ fn substitute_all(
// Visit other custom properties...
// FIXME: Maybe avoid visiting the same var twice if not needed?
for next in &v.references.refs {
- if next.substitution_kind != SubstitutionFunctionKind::Var {
+ if !next.is_var {
continue;
}
visit_link(
@@ -1743,9 +1731,7 @@ fn substitute_all(
)
.is_some()
|| v.references.refs.iter().any(|reference| {
- (reference.substitution_kind == SubstitutionFunctionKind::Var
- && deferred.get(&reference.name).is_some())
- || reference.substitution_kind == SubstitutionFunctionKind::Attr
+ reference.is_var && deferred.get(&reference.name).is_some()
});
if defer {
@@ -1756,7 +1742,7 @@ fn substitute_all(
}
// If there are no var references we should already be computed and substituted by now.
- if !defer && (v.references.any_var || v.references.any_attr) {
+ if !defer && v.references.any_var {
substitute_references_if_needed_and_apply(
&name,
v,
@@ -2007,7 +1993,7 @@ fn do_substitute_chunk<'a>(
custom_properties: &'a ComputedCustomProperties,
stylist: &Stylist,
computed_context: &computed::Context,
- references: &mut std::iter::Peekable<std::slice::Iter<SubstitutionFunctionReference>>,
+ references: &mut std::iter::Peekable<std::slice::Iter<VarOrEnvReference>>,
) -> Result<Substitution<'a>, ()> {
if start == end {
// Empty string. Easy.
@@ -2068,37 +2054,31 @@ fn substitute_one_reference<'a>(
css: &'a str,
url_data: &UrlExtraData,
custom_properties: &'a ComputedCustomProperties,
- reference: &SubstitutionFunctionReference,
+ reference: &VarOrEnvReference,
stylist: &Stylist,
computed_context: &computed::Context,
- references: &mut std::iter::Peekable<std::slice::Iter<SubstitutionFunctionReference>>,
+ references: &mut std::iter::Peekable<std::slice::Iter<VarOrEnvReference>>,
) -> Result<Substitution<'a>, ()> {
- match reference.substitution_kind {
- SubstitutionFunctionKind::Var => {
- let registration = stylist.get_custom_property_registration(&reference.name);
- if let Some(v) = custom_properties.get(registration, &reference.name) {
- // Skip references that are inside the outer variable (in fallback for example).
- while references
- .next_if(|next_ref| next_ref.end <= reference.end)
- .is_some()
- {}
- return Ok(Substitution::from_value(v.to_variable_value()));
- }
- },
- SubstitutionFunctionKind::Env => {
- let device = stylist.device();
- if let Some(v) = device.environment().get(&reference.name, device, url_data) {
- while references
- .next_if(|next_ref| next_ref.end <= reference.end)
- .is_some()
- {}
- return Ok(Substitution::from_value(v));
- }
- },
- SubstitutionFunctionKind::Attr => {
- // TODO(descalante): implement attribute lookup.
- },
- };
+ if reference.is_var {
+ let registration = stylist.get_custom_property_registration(&reference.name);
+ if let Some(v) = custom_properties.get(registration, &reference.name) {
+ // Skip references that are inside the outer variable (in fallback for example).
+ while references
+ .next_if(|next_ref| next_ref.end <= reference.end)
+ .is_some()
+ {}
+ return Ok(Substitution::from_value(v.to_variable_value()));
+ }
+ } else {
+ let device = stylist.device();
+ if let Some(v) = device.environment().get(&reference.name, device, url_data) {
+ while references
+ .next_if(|next_ref| next_ref.end <= reference.end)
+ .is_some()
+ {}
+ return Ok(Substitution::from_value(v));
+ }
+ }
let Some(ref fallback) = reference.fallback else {
return Err(());
diff --git a/servo/components/style/properties/mod.rs b/servo/components/style/properties/mod.rs
@@ -747,14 +747,7 @@ fn parse_non_custom_property_declaration_value_into<'i>(
};
input.reset(&start);
- input.look_for_arbitrary_substitution_functions(
- if static_prefs::pref!("layout.css.attr.enabled") {
- &["var", "env", "attr"]
- } else {
- &["var", "env"]
- },
- );
-
+ input.look_for_arbitrary_substitution_functions(&["var", "env"]);
let err = match parse_entirely_into(declarations, input) {
Ok(()) => {
input.seen_arbitrary_substitution_functions();
