Skip to content

Commit 58ee680

Browse files
author
agoloman
committed
Revert "Bug 2024601 - patch 6 - Also support env(), and add a couple tests for it. r=firefox-style-system-reviewers,dshin" for causing wr failures @at-supports-namespace-001.html.
This reverts commit c6c4fb5. Revert "Bug 2024601 - patch 5 - Provide an AttributeTracker when resolving style query expression values. r=firefox-style-system-reviewers,emilio" This reverts commit 85e1ee1. Revert "Bug 2024601 - patch 4 - Remove the special-case for simple var() references (without default value) in feature expression values. r=firefox-style-system-reviewers,emilio" This reverts commit 9ed150d. Revert "Bug 2024601 - patch 3 - Evaluate substitution functions in style query expression values. r=firefox-style-system-reviewers,emilio" This reverts commit d4728d9. Revert "Bug 2024601 - patch 2 - Add evaluation tests involving substitution functions. r=firefox-style-system-reviewers,emilio" This reverts commit a7d6b5c. Revert "Bug 2024601 - patch 1 - Parsing support for substitution functions as style query expression values. r=firefox-style-system-reviewers,emilio" This reverts commit 76ede91.
1 parent 69b54d8 commit 58ee680

11 files changed

Lines changed: 79 additions & 211 deletions

File tree

‎servo/components/style/custom_properties.rs‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2392,7 +2392,6 @@ impl<'a> Substitution<'a> {
23922392
}
23932393

23942394
/// Result of var(), env(), and attr() substitution.
2395-
#[derive(Debug)]
23962395
pub struct SubstitutionResult<'a> {
23972396
/// The resolved CSS string after substitution.
23982397
pub css: Cow<'a, str>,
@@ -2582,11 +2581,10 @@ fn substitute_one_reference<'a>(
25822581
},
25832582
|attr| {
25842583
let attr = if let AttributeType::Type(_) = &reference.attribute_data.kind {
2585-
if let Some(value) = substitution_functions.get_attr(&reference.name) {
2586-
value.to_variable_value().css
2587-
} else {
2588-
attr
2589-
}
2584+
substitution_functions
2585+
.get_attr(&reference.name)
2586+
.map(|v| v.to_variable_value())?
2587+
.css
25902588
} else {
25912589
attr
25922590
};

‎servo/components/style/media_queries/media_list.rs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ use super::{MediaQuery, Qualifier};
1010
use crate::context::QuirksMode;
1111
use crate::derives::*;
1212
use crate::device::Device;
13-
use crate::dom::AttributeTracker;
1413
use crate::error_reporting::ContextualParseError;
1514
use crate::parser::ParserContext;
1615
use crate::stylesheets::CustomMediaEvaluator;
@@ -101,9 +100,9 @@ impl MediaList {
101100
}
102101
KleeneValue::any(self.media_queries.iter(), |mq| {
103102
let mut query_match = if mq.media_type.matches(context.device().media_type()) {
104-
mq.condition.as_ref().map_or(KleeneValue::True, |c| {
105-
c.matches(context, custom, &mut AttributeTracker::new_dummy())
106-
})
103+
mq.condition
104+
.as_ref()
105+
.map_or(KleeneValue::True, |c| c.matches(context, custom))
107106
} else {
108107
KleeneValue::False
109108
};

‎servo/components/style/properties/mod.rs‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -734,15 +734,6 @@ impl ShorthandId {
734734
}
735735
}
736736

737-
/// Return the names of arbitrary substitution functions that are enabled.
738-
pub fn enabled_arbitrary_substitution_functions() -> &'static [&'static str] {
739-
if static_prefs::pref!("layout.css.attr.enabled") {
740-
&["var", "env", "attr"]
741-
} else {
742-
&["var", "env"]
743-
}
744-
}
745-
746737
fn parse_non_custom_property_declaration_value_into<'i>(
747738
declarations: &mut SourcePropertyDeclaration,
748739
context: &ParserContext,
@@ -774,7 +765,13 @@ fn parse_non_custom_property_declaration_value_into<'i>(
774765
};
775766

776767
input.reset(&start);
777-
input.look_for_arbitrary_substitution_functions(enabled_arbitrary_substitution_functions());
768+
input.look_for_arbitrary_substitution_functions(
769+
if static_prefs::pref!("layout.css.attr.enabled") {
770+
&["var", "env", "attr"]
771+
} else {
772+
&["var", "env"]
773+
},
774+
);
778775

779776
let err = match parse_entirely_into(declarations, input) {
780777
Ok(()) => {

‎servo/components/style/queries/condition.rs‎

Lines changed: 26 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -245,26 +245,18 @@ impl StyleQuery {
245245
}
246246
}
247247

248-
fn matches(
249-
&self,
250-
ctx: &computed::Context,
251-
attribute_tracker: &mut AttributeTracker,
252-
) -> KleeneValue {
248+
fn matches(&self, ctx: &computed::Context) -> KleeneValue {
253249
ctx.builder
254250
.add_flags(ComputedValueFlags::DEPENDS_ON_CONTAINER_STYLE_QUERY);
255251
match *self {
256-
StyleQuery::Feature(ref f) => f.matches(ctx, attribute_tracker),
257-
StyleQuery::Not(ref c) => !c.matches(ctx, attribute_tracker),
258-
StyleQuery::InParens(ref c) => c.matches(ctx, attribute_tracker),
252+
StyleQuery::Feature(ref f) => f.matches(ctx),
253+
StyleQuery::Not(ref c) => !c.matches(ctx),
254+
StyleQuery::InParens(ref c) => c.matches(ctx),
259255
StyleQuery::Operation(ref conditions, op) => {
260256
debug_assert!(!conditions.is_empty(), "We never create an empty op");
261257
match op {
262-
Operator::And => KleeneValue::any_false(conditions.iter(), |c| {
263-
c.matches(ctx, attribute_tracker)
264-
}),
265-
Operator::Or => {
266-
KleeneValue::any(conditions.iter(), |c| c.matches(ctx, attribute_tracker))
267-
},
258+
Operator::And => KleeneValue::any_false(conditions.iter(), |c| c.matches(ctx)),
259+
Operator::Or => KleeneValue::any(conditions.iter(), |c| c.matches(ctx)),
268260
}
269261
},
270262
StyleQuery::GeneralEnclosed(_) => KleeneValue::Unknown,
@@ -333,14 +325,10 @@ impl StyleFeature {
333325
Ok(Self::Plain(StyleFeaturePlain::parse(context, input)?))
334326
}
335327

336-
fn matches(
337-
&self,
338-
ctx: &computed::Context,
339-
attribute_tracker: &mut AttributeTracker,
340-
) -> KleeneValue {
328+
fn matches(&self, ctx: &computed::Context) -> KleeneValue {
341329
match self {
342-
Self::Plain(plain) => plain.matches(ctx, attribute_tracker),
343-
Self::Range(range) => range.evaluate(ctx, attribute_tracker),
330+
Self::Plain(plain) => plain.matches(ctx),
331+
Self::Range(range) => range.evaluate(ctx),
344332
}
345333
}
346334
}
@@ -413,7 +401,6 @@ impl StyleFeaturePlain {
413401
registration: &PropertyDescriptors,
414402
stylist: &Stylist,
415403
ctx: &computed::Context,
416-
attribute_tracker: &mut AttributeTracker,
417404
current_value: Option<&ComputedRegisteredValue>,
418405
) -> bool {
419406
let substitution_functions = custom_properties::ComputedSubstitutionFunctions::new(
@@ -426,7 +413,8 @@ impl StyleFeaturePlain {
426413
&substitution_functions,
427414
stylist,
428415
ctx,
429-
attribute_tracker,
416+
// FIXME: do we need to pass a real AttributeTracker for the query?
417+
&mut AttributeTracker::new_dummy(),
430418
) {
431419
Ok(sub) => sub,
432420
Err(_) => return current_value.is_none(),
@@ -451,11 +439,7 @@ impl StyleFeaturePlain {
451439
computed.as_ref() == current_value
452440
}
453441

454-
fn matches(
455-
&self,
456-
ctx: &computed::Context,
457-
attribute_tracker: &mut AttributeTracker,
458-
) -> KleeneValue {
442+
fn matches(&self, ctx: &computed::Context) -> KleeneValue {
459443
// FIXME(emilio): Confirm this is the right style to query.
460444
let stylist = ctx
461445
.builder
@@ -473,14 +457,7 @@ impl StyleFeaturePlain {
473457
} else if v.has_references() {
474458
// If there are --var() references in the query value,
475459
// try to substitute them before comparing to current.
476-
Self::substitute_and_compare(
477-
v,
478-
registration,
479-
stylist,
480-
ctx,
481-
attribute_tracker,
482-
current_value,
483-
)
460+
Self::substitute_and_compare(v, registration, stylist, ctx, current_value)
484461
} else {
485462
custom_properties::compute_variable_value(&v, registration, ctx).as_ref()
486463
== current_value
@@ -822,27 +799,26 @@ impl QueryCondition {
822799
&self,
823800
context: &computed::Context,
824801
custom: &mut CustomMediaEvaluator,
825-
attribute_tracker: &mut AttributeTracker,
826802
) -> KleeneValue {
827803
match *self {
828804
Self::Custom(ref f) => custom.matches(f, context),
829805
Self::Feature(ref f) => f.matches(context),
830806
Self::GeneralEnclosed(ref str, ref url_data) => {
831-
self.matches_general(&str, url_data, context, custom, attribute_tracker)
807+
self.matches_general(&str, url_data, context, custom)
832808
},
833-
Self::InParens(ref c) => c.matches(context, custom, attribute_tracker),
834-
Self::Not(ref c) => !c.matches(context, custom, attribute_tracker),
835-
Self::Style(ref c) => c.matches(context, attribute_tracker),
809+
Self::InParens(ref c) => c.matches(context, custom),
810+
Self::Not(ref c) => !c.matches(context, custom),
811+
Self::Style(ref c) => c.matches(context),
836812
Self::MozPref(ref c) => c.matches(context),
837813
Self::Operation(ref conditions, op) => {
838814
debug_assert!(!conditions.is_empty(), "We never create an empty op");
839815
match op {
840-
Operator::And => KleeneValue::any_false(conditions.iter(), |c| {
841-
c.matches(context, custom, attribute_tracker)
842-
}),
843-
Operator::Or => KleeneValue::any(conditions.iter(), |c| {
844-
c.matches(context, custom, attribute_tracker)
845-
}),
816+
Operator::And => {
817+
KleeneValue::any_false(conditions.iter(), |c| c.matches(context, custom))
818+
},
819+
Operator::Or => {
820+
KleeneValue::any(conditions.iter(), |c| c.matches(context, custom))
821+
},
846822
}
847823
},
848824
}
@@ -856,7 +832,6 @@ impl QueryCondition {
856832
url_data: &UrlExtraData,
857833
context: &computed::Context,
858834
custom: &mut CustomMediaEvaluator,
859-
attribute_tracker: &mut AttributeTracker,
860835
) -> KleeneValue {
861836
// This only applies (currently, at least) to container queries.
862837
if !context.in_container_query {
@@ -897,7 +872,8 @@ impl QueryCondition {
897872
&substitution_functions,
898873
stylist,
899874
context,
900-
attribute_tracker,
875+
// FIXME: do we need to pass a real AttributeTracker for the query?
876+
&mut AttributeTracker::new_dummy(),
901877
) {
902878
Ok(sub) => sub,
903879
Err(_) => return KleeneValue::Unknown,
@@ -928,7 +904,7 @@ impl QueryCondition {
928904
// If the result is still GeneralEnclosed, the query is unknown.
929905
KleeneValue::Unknown
930906
},
931-
Ok(query) => query.matches(context, custom, attribute_tracker),
907+
Ok(query) => query.matches(context, custom),
932908
Err(_) => KleeneValue::Unknown,
933909
};
934910

0 commit comments

Comments
 (0)