Bug 1431031: Make the font-size calc() code do what it means to do.
It makes no sense to pass a custom base size of zero in presence of rem, ex, or
ch units.
MozReview-Commit-ID: 7ZZwRzQKREX
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -2500,19 +2500,22 @@ fn static_assert() {
self.gecko.mFontSizeKeyword = structs::NS_STYLE_FONT_SIZE_NO_KEYWORD as u8;
self.gecko.mFontSizeFactor = 1.;
self.gecko.mFontSizeOffset = 0;
}
}
/// Set font size, taking into account scriptminsize and scriptlevel
/// Returns Some(size) if we have to recompute the script unconstrained size
- pub fn apply_font_size(&mut self, v: FontSize,
- parent: &Self,
- device: &Device) -> Option<NonNegativeLength> {
+ pub fn apply_font_size(
+ &mut self,
+ v: FontSize,
+ parent: &Self,
+ device: &Device,
+ ) -> Option<NonNegativeLength> {
let (adjusted_size, adjusted_unconstrained_size) =
self.calculate_script_level_size(parent, device);
// In this case, we have been unaffected by scriptminsize, ignore it
if parent.gecko.mSize == parent.gecko.mScriptUnconstrainedSize &&
adjusted_size == adjusted_unconstrained_size {
self.set_font_size(v);
self.fixup_font_min_size(device);
None
--- a/servo/components/style/values/specified/font.rs
+++ b/servo/components/style/values/specified/font.rs
@@ -588,25 +588,35 @@ impl FontSize {
base_size.resolve(context).scale_by(pc.0).into()
}
FontSize::Length(LengthOrPercentage::Calc(ref calc)) => {
let parent = context.style().get_parent_font().clone_font_size();
// if we contain em/% units and the parent was keyword derived, this is too
// Extract the ratio/offset and compose it
if (calc.em.is_some() || calc.percentage.is_some()) && parent.keyword_info.is_some() {
let ratio = calc.em.unwrap_or(0.) + calc.percentage.map_or(0., |pc| pc.0);
- // Compute it, but shave off the font-relative part (em, %)
- // This will mean that other font-relative units like ex and ch will be computed against
- // the old font even when the font changes. There's no particular "right answer" for what
- // to do here -- Gecko recascades as if the font had changed, we instead track the changes
- // and reapply, which means that we carry over old computed ex/ch values whilst Gecko
- // recomputes new ones. This is enough of an edge case to not really matter.
- let abs = calc.to_computed_value_zoomed(context, FontBaseSize::Custom(Au(0).into()))
- .length_component().into();
- info = parent.keyword_info.map(|i| i.compose(ratio, abs));
+ // Compute it, but shave off the font-relative part (em, %).
+ //
+ // This will mean that other font-relative units like ex and
+ // ch will be computed against the old parent font even when
+ // the font changes.
+ //
+ // There's no particular "right answer" for what to do here,
+ // Gecko recascades as if the font had changed, we instead
+ // track the changes and reapply, which means that we carry
+ // over old computed ex/ch values whilst Gecko recomputes
+ // new ones.
+ //
+ // This is enough of an edge case to not really matter.
+ let abs = calc.to_computed_value_zoomed(
+ context,
+ FontBaseSize::InheritedStyleButStripEmUnits,
+ ).length_component();
+
+ info = parent.keyword_info.map(|i| i.compose(ratio, abs.into()));
}
let calc = calc.to_computed_value_zoomed(context, base_size);
calc.to_used_value(Some(base_size.resolve(context))).unwrap().into()
}
FontSize::Keyword(i) => {
// As a specified keyword, this is keyword derived
info = Some(i);
i.to_computed_value(context)
--- a/servo/components/style/values/specified/length.rs
+++ b/servo/components/style/values/specified/length.rs
@@ -66,74 +66,93 @@ pub enum FontRelativeLength {
/// A "rem" value: https://drafts.csswg.org/css-values/#rem
#[css(dimension)]
Rem(CSSFloat)
}
/// A source to resolve font-relative units against
#[derive(Clone, Copy, Debug, PartialEq)]
pub enum FontBaseSize {
- /// Use the font-size of the current element
+ /// Use the font-size of the current element.
CurrentStyle,
- /// Use the inherited font-size
+ /// Use the inherited font-size.
InheritedStyle,
- /// Use a custom base size
+ /// Use the inherited font-size, but strip em units.
+ ///
+ /// FIXME(emilio): This is very complex, and should go away.
+ InheritedStyleButStripEmUnits,
+ /// Use a custom base size.
+ ///
+ /// FIXME(emilio): This is very dubious, and only used for MathML.
Custom(Au),
}
impl FontBaseSize {
/// Calculate the actual size for a given context
pub fn resolve(&self, context: &Context) -> Au {
match *self {
FontBaseSize::Custom(size) => size,
FontBaseSize::CurrentStyle => context.style().get_font().clone_font_size().size(),
+ FontBaseSize::InheritedStyleButStripEmUnits |
FontBaseSize::InheritedStyle => context.style().get_parent_font().clone_font_size().size(),
}
}
}
impl FontRelativeLength {
/// Computes the font-relative length.
pub fn to_computed_value(&self, context: &Context, base_size: FontBaseSize) -> CSSPixelLength {
use std::f32;
let (reference_size, length) = self.reference_font_size_and_length(context, base_size);
let pixel = (length * reference_size.to_f32_px()).min(f32::MAX).max(f32::MIN);
CSSPixelLength::new(pixel)
}
- /// Return reference font size. We use the base_size flag to pass a different size
- /// for computing font-size and unconstrained font-size.
- /// This returns a pair, the first one is the reference font size, and the second one is the
- /// unpacked relative length.
+ /// Return reference font size.
+ ///
+ /// We use the base_size flag to pass a different size for computing
+ /// font-size and unconstrained font-size.
+ ///
+ /// This returns a pair, the first one is the reference font size, and the
+ /// second one is the unpacked relative length.
fn reference_font_size_and_length(
&self,
context: &Context,
base_size: FontBaseSize,
) -> (Au, CSSFloat) {
- fn query_font_metrics(context: &Context, reference_font_size: Au) -> FontMetricsQueryResult {
- context.font_metrics_provider.query(context.style().get_font(),
- reference_font_size,
- context.style().writing_mode,
- context.in_media_query,
- context.device())
+ fn query_font_metrics(
+ context: &Context,
+ reference_font_size: Au,
+ ) -> FontMetricsQueryResult {
+ context.font_metrics_provider.query(
+ context.style().get_font(),
+ reference_font_size,
+ context.style().writing_mode,
+ context.in_media_query,
+ context.device(),
+ )
}
let reference_font_size = base_size.resolve(context);
-
match *self {
FontRelativeLength::Em(length) => {
if context.for_non_inherited_property.is_some() {
if matches!(base_size, FontBaseSize::CurrentStyle) {
context.rule_cache_conditions.borrow_mut()
.set_font_size_dependency(
reference_font_size.into()
);
}
}
- (reference_font_size, length)
+
+ if matches!(base_size, FontBaseSize::InheritedStyleButStripEmUnits) {
+ (Au(0), length)
+ } else {
+ (reference_font_size, length)
+ }
},
FontRelativeLength::Ex(length) => {
if context.for_non_inherited_property.is_some() {
context.rule_cache_conditions.borrow_mut().set_uncacheable();
}
let reference_size = match query_font_metrics(context, reference_font_size) {
FontMetricsQueryResult::Available(metrics) => {
metrics.x_height
@@ -177,17 +196,17 @@ impl FontRelativeLength {
}
};
(reference_size, length)
}
FontRelativeLength::Rem(length) => {
// https://drafts.csswg.org/css-values/#rem:
//
// When specified on the font-size property of the root
- // element, the rem units refer to the property’s initial
+ // element, the rem units refer to the property's initial
// value.
//
let reference_size = if context.is_root_element || context.in_media_query {
reference_font_size
} else {
context.device().root_font_size()
};
(reference_size, length)
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -146378,16 +146378,28 @@
[
"/css/css-values/reference/200-200-green.html",
"=="
]
],
{}
]
],
+ "css/css-values/calc-ch-ex-lang.html": [
+ [
+ "/css/css-values/calc-ch-ex-lang.html",
+ [
+ [
+ "/css/css-values/calc-ch-ex-lang-ref.html",
+ "=="
+ ]
+ ],
+ {}
+ ]
+ ],
"css/css-values/calc-in-calc.html": [
[
"/css/css-values/calc-in-calc.html",
[
[
"/css/css-values/reference/all-green.html",
"=="
]
@@ -146438,16 +146450,28 @@
[
"/css/css-values/reference/all-green.html",
"=="
]
],
{}
]
],
+ "css/css-values/calc-rem-lang.html": [
+ [
+ "/css/css-values/calc-rem-lang.html",
+ [
+ [
+ "/css/css-values/calc-rem-lang-ref.html",
+ "=="
+ ]
+ ],
+ {}
+ ]
+ ],
"css/css-values/ch-unit-001.html": [
[
"/css/css-values/ch-unit-001.html",
[
[
"/css/css-values/reference/ch-unit-001-ref.html",
"=="
]
@@ -253663,16 +253687,26 @@
{}
]
],
"css/css-values/OWNERS": [
[
{}
]
],
+ "css/css-values/calc-ch-ex-lang-ref.html": [
+ [
+ {}
+ ]
+ ],
+ "css/css-values/calc-rem-lang-ref.html": [
+ [
+ {}
+ ]
+ ],
"css/css-values/ex-calc-expression-001-ref.html": [
[
{}
]
],
"css/css-values/reference/200-200-green.html": [
[
{}
@@ -510085,16 +510119,24 @@
"css/css-values/attr-px-invalid-fallback.html": [
"855fa8131c6fcf734982d427b0e46ab2be90ae80",
"reftest"
],
"css/css-values/attr-px-valid.html": [
"70627dbbfc5af5fb859fdf1362f0004b38c64e34",
"reftest"
],
+ "css/css-values/calc-ch-ex-lang-ref.html": [
+ "218e1cb6782b554de35aad70913cddb8e7d71da2",
+ "support"
+ ],
+ "css/css-values/calc-ch-ex-lang.html": [
+ "d15f42e237b281793ac808e06af92217d4f9593e",
+ "reftest"
+ ],
"css/css-values/calc-in-calc.html": [
"be08a1510714e8b4fbc4d35582db5708924d06b2",
"reftest"
],
"css/css-values/calc-in-media-queries-001.html": [
"f8fdd8373eaca7a03d6a089b4faf71825c8bfaf2",
"reftest"
],
@@ -510105,16 +510147,24 @@
"css/css-values/calc-invalid-range-clamping.html": [
"3352575826aa7e335ef41b9e4f3386deda4a54b0",
"reftest"
],
"css/css-values/calc-parenthesis-stack.html": [
"080551c1bee3d7bf54dda2c3d5b7e5a9fbd8aed6",
"reftest"
],
+ "css/css-values/calc-rem-lang-ref.html": [
+ "08bbc95f3078421a489e1e93cc7a4f035af40d5b",
+ "support"
+ ],
+ "css/css-values/calc-rem-lang.html": [
+ "6fa668d2bcaf01f5c4680e3e14a0e86160d1b5d5",
+ "reftest"
+ ],
"css/css-values/calc-unit-analysis.html": [
"c5fd567b4fa257ce53c48ebf8c444bf382459fec",
"testharness"
],
"css/css-values/ch-unit-001.html": [
"6a9d4e97823a3c1b2babf5dcc98595a4697c5eb4",
"reftest"
],
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-values/calc-ch-ex-lang-ref.html
@@ -0,0 +1,12 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>CSS Test Reference</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<style>
+div {
+ width: calc(1ex + 1ch + 1em);
+ height: calc(1ex + 1ch + 1em);
+ background: green;
+}
+</style>
+<div></div>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-values/calc-ch-ex-lang.html
@@ -0,0 +1,16 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>CSS Test: Calc in font-size with ch / ex units across lang changes</title>
+<link rel="help" href="https://drafts.csswg.org/css-values/#ch">
+<link rel="help" href="https://drafts.csswg.org/css-values/#ex">
+<link rel="help" href="https://drafts.csswg.org/css-values/#funcdef-calc">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1431031">
+<link rel="match" href="calc-ch-ex-lang-ref.html">
+<style>
+div[lang] {
+ font-size: calc(1ex + 1ch + 1em);
+}
+</style>
+<div lang="en">
+ <div style="width: 1em; height: 1em; background: green;"></div>
+</div>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-values/calc-rem-lang-ref.html
@@ -0,0 +1,6 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>CSS Test Reference</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<p>You should see a green box twice-the-initial-font-size wide.</p>
+<div style="width: 2em; height: 2em; background: green;"></div>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-values/calc-rem-lang.html
@@ -0,0 +1,17 @@
+<!doctype html>
+<html lang="en"><!-- The lang is important! -->
+<meta charset="utf-8">
+<title>CSS Test: Calc with rem and relative units on the root element</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="help" href="https://drafts.csswg.org/css-values/#rem">
+<link rel="help" href="https://drafts.csswg.org/css-values/#funcdef-calc">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1431031">
+<link rel="match" href="calc-rem-lang-ref.html">
+<style>
+ html {
+ font-size: calc(1rem + 1em);
+ }
+</style>
+<p style="font-size: initial">You should see a green box twice-the-initial-font-size wide.</p>
+<div style="width: 1em; height: 1em; background: green;"></div>
+</html>