Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-07-24 15:50:21 +01:00
Code Issues Projects Releases Packages Wiki Activity

Make the parser accept @font-face rules without font-family or src.

Browse source 
Fix #16165.

Also, it turns out that the CSSFontFaceRule IDL specified in the
css-fonts spec is not web-compatible.
Instead browsers implement a .style attribute like in CSSStyleRule:
https://github.com/w3c/csswg-drafts/issues/825

This in turn requires preserving data about which descriptors
were set or not (distinguishing unset from set to a value that happens
to be the initial value),
so this commit also makes every field `Option<_>`.
This commit is contained in:
Simon Sapin 2017-04-01 08:20:35 +02:00
parent 15fc9f3bbf
commit 61812d4d9d
7 changed files with 169 additions and 168 deletions
Download patch file Download diff file Expand all files Collapse all files

28
components/layout_thread/lib.rs
View file

@ -355,20 +355,24 @@ fn add_font_face_rules(stylesheet: &Stylesheet,
outstanding_web_fonts_counter: &Arc<AtomicUsize>) { outstanding_web_fonts_counter: &Arc<AtomicUsize>) {
if opts::get().load_webfonts_synchronously { if opts::get().load_webfonts_synchronously {
let (sender, receiver) = ipc::channel().unwrap(); let (sender, receiver) = ipc::channel().unwrap();
stylesheet.effective_font_face_rules(&device, guard, |font_face| { stylesheet.effective_font_face_rules(&device, guard, |rule| {
let effective_sources = font_face.effective_sources(); if let Some(font_face) = rule.font_face() {
font_cache_thread.add_web_font(font_face.family.clone(), let effective_sources = font_face.effective_sources();
effective_sources, font_cache_thread.add_web_font(font_face.family().clone(),
sender.clone()); effective_sources,
receiver.recv().unwrap(); sender.clone());
receiver.recv().unwrap();
}
}) })
} else { } else {
stylesheet.effective_font_face_rules(&device, guard, |font_face| { stylesheet.effective_font_face_rules(&device, guard, |rule| {
let effective_sources = font_face.effective_sources(); if let Some(font_face) = rule.font_face() {
outstanding_web_fonts_counter.fetch_add(1, Ordering::SeqCst); let effective_sources = font_face.effective_sources();
font_cache_thread.add_web_font(font_face.family.clone(), outstanding_web_fonts_counter.fetch_add(1, Ordering::SeqCst);
effective_sources, font_cache_thread.add_web_font(font_face.family().clone(),
(*font_cache_sender).clone()); effective_sources,
(*font_cache_sender).clone());
}
}) })
} }
} }

15
components/script/dom/webidls/CSSFontFaceRule.webidl
View file

@ -2,16 +2,13 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this * License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
// https://drafts.csswg.org/css-fonts/#cssfontfacerule // https://drafts.csswg.org/css-fonts/#cssfontfacerule is unfortunately not web-compatible:
// https://github.com/w3c/csswg-drafts/issues/825
// https://www.w3.org/TR/2000/REC-DOM-Level-2-Style-20001113/css.html#CSS-CSSFontFaceRule ,
// plus extended attributes matching CSSStyleRule
[Exposed=Window] [Exposed=Window]
interface CSSFontFaceRule : CSSRule { interface CSSFontFaceRule : CSSRule {
// attribute DOMString family; // [SameObject, PutForwards=cssText] readonly attribute CSSStyleDeclaration style;
// attribute DOMString src;
// attribute DOMString style;
// attribute DOMString weight;
// attribute DOMString stretch;
// attribute DOMString unicodeRange;
// attribute DOMString variant;
// attribute DOMString featureSettings;
}; };

172
components/style/font_face.rs
View file

@ -16,7 +16,6 @@ use cssparser::{AtRuleParser, DeclarationListParser, DeclarationParser, Parser};
use parser::{ParserContext, log_css_error, Parse}; use parser::{ParserContext, log_css_error, Parse};
use shared_lock::{SharedRwLockReadGuard, ToCssWithGuard}; use shared_lock::{SharedRwLockReadGuard, ToCssWithGuard};
use std::fmt; use std::fmt;
use std::iter;
use style_traits::{ToCss, OneOrMoreCommaSeparated}; use style_traits::{ToCss, OneOrMoreCommaSeparated};
use values::specified::url::SpecifiedUrl; use values::specified::url::SpecifiedUrl;
@ -74,14 +73,12 @@ impl ToCss for UrlSource {
/// Parse the block inside a `@font-face` rule. /// Parse the block inside a `@font-face` rule.
/// ///
/// Note that the prelude parsing code lives in the `stylesheets` module. /// Note that the prelude parsing code lives in the `stylesheets` module.
pub fn parse_font_face_block(context: &ParserContext, input: &mut Parser) pub fn parse_font_face_block(context: &ParserContext, input: &mut Parser) -> FontFaceRuleData {
-> Result<FontFaceData, ()> { let mut rule = FontFaceRuleData::empty();
let mut rule = FontFaceData::initial();
{ {
let parser = FontFaceRuleParser { let parser = FontFaceRuleParser {
context: context, context: context,
rule: &mut rule, rule: &mut rule,
missing: MissingDescriptors::new(),
}; };
let mut iter = DeclarationListParser::new(input, parser); let mut iter = DeclarationListParser::new(input, parser);
while let Some(declaration) = iter.next() { while let Some(declaration) = iter.next() {
@ -92,24 +89,27 @@ pub fn parse_font_face_block(context: &ParserContext, input: &mut Parser)
log_css_error(iter.input, pos, &*message, context); log_css_error(iter.input, pos, &*message, context);
} }
} }
if iter.parser.missing.any() {
return Err(())
}
} }
Ok(rule) rule
} }
/// A @font-face rule that is known to have font-family and src declarations.
#[cfg(feature = "servo")]
pub struct FontFace<'a>(&'a FontFaceRuleData);
/// A list of effective sources that we send over through IPC to the font cache. /// A list of effective sources that we send over through IPC to the font cache.
#[cfg(feature = "servo")]
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
#[cfg_attr(feature = "servo", derive(Deserialize, Serialize))] #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
pub struct EffectiveSources(Vec<Source>); pub struct EffectiveSources(Vec<Source>);
impl FontFaceData { #[cfg(feature = "servo")]
impl<'a> FontFace<'a> {
/// Returns the list of effective sources for that font-face, that is the /// Returns the list of effective sources for that font-face, that is the
/// sources which don't list any format hint, or the ones which list at /// sources which don't list any format hint, or the ones which list at
/// least "truetype" or "opentype". /// least "truetype" or "opentype".
pub fn effective_sources(&self) -> EffectiveSources { pub fn effective_sources(&self) -> EffectiveSources {
EffectiveSources(self.sources.iter().rev().filter(|source| { EffectiveSources(self.sources().iter().rev().filter(|source| {
if let Source::Url(ref url_source) = **source { if let Source::Url(ref url_source) = **source {
let hints = &url_source.format_hints; let hints = &url_source.format_hints;
// We support only opentype fonts and truetype is an alias for // We support only opentype fonts and truetype is an alias for
@ -125,7 +125,8 @@ impl FontFaceData {
} }
} }
impl iter::Iterator for EffectiveSources { #[cfg(feature = "servo")]
impl Iterator for EffectiveSources {
type Item = Source; type Item = Source;
fn next(&mut self) -> Option<Source> { fn next(&mut self) -> Option<Source> {
self.0.pop() self.0.pop()
@ -134,8 +135,7 @@ impl iter::Iterator for EffectiveSources {
struct FontFaceRuleParser<'a, 'b: 'a> { struct FontFaceRuleParser<'a, 'b: 'a> {
context: &'a ParserContext<'b>, context: &'a ParserContext<'b>,
rule: &'a mut FontFaceData, rule: &'a mut FontFaceRuleData,
missing: MissingDescriptors,
} }
/// Default methods reject all at rules. /// Default methods reject all at rules.
@ -172,82 +172,40 @@ impl Parse for Source {
} }
} }
macro_rules! font_face_descriptors { macro_rules! font_face_descriptors_common {
( (
mandatory descriptors = [ $( #[$doc: meta] $name: tt $ident: ident: $ty: ty, )*
$( #[$m_doc: meta] $m_name: tt $m_ident: ident: $m_ty: ty = $m_initial: expr, )*
]
optional descriptors = [
$( #[$o_doc: meta] $o_name: tt $o_ident: ident: $o_ty: ty = $o_initial: expr, )*
]
) => { ) => {
/// Data inside a `@font-face` rule. /// Data inside a `@font-face` rule.
/// ///
/// https://drafts.csswg.org/css-fonts/#font-face-rule /// https://drafts.csswg.org/css-fonts/#font-face-rule
#[derive(Debug, PartialEq, Eq)] #[derive(Debug, PartialEq, Eq)]
pub struct FontFaceData { pub struct FontFaceRuleData {
$( $(
#[$m_doc] #[$doc]
pub $m_ident: $m_ty, pub $ident: Option<$ty>,
)*
$(
#[$o_doc]
pub $o_ident: $o_ty,
)* )*
} }
struct MissingDescriptors { impl FontFaceRuleData {
$( fn empty() -> Self {
$m_ident: bool, FontFaceRuleData {
)*
}
impl MissingDescriptors {
fn new() -> Self {
MissingDescriptors {
$( $(
$m_ident: true, $ident: None,
)*
}
}
fn any(&self) -> bool {
$(
self.$m_ident
)||*
}
}
impl FontFaceData {
fn initial() -> Self {
FontFaceData {
$(
$m_ident: $m_initial,
)*
$(
$o_ident: $o_initial,
)* )*
} }
} }
} }
impl ToCssWithGuard for FontFaceData { impl ToCssWithGuard for FontFaceRuleData {
// Serialization of FontFaceRule is not specced. // Serialization of FontFaceRule is not specced.
fn to_css<W>(&self, _guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result fn to_css<W>(&self, _guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
where W: fmt::Write { where W: fmt::Write {
dest.write_str("@font-face {\n")?; dest.write_str("@font-face {\n")?;
$( $(
dest.write_str(concat!(" ", $m_name, ": "))?; if let Some(ref value) = self.$ident {
ToCss::to_css(&self.$m_ident, dest)?; dest.write_str(concat!(" ", $name, ": "))?;
dest.write_str(";\n")?; ToCss::to_css(value, dest)?;
)*
$(
// Because of parse_font_face_block,
// this condition is always true for "src" and "font-family".
// But it can be false for other descriptors.
if self.$o_ident != $o_initial {
dest.write_str(concat!(" ", $o_name, ": "))?;
ToCss::to_css(&self.$o_ident, dest)?;
dest.write_str(";\n")?; dest.write_str(";\n")?;
} }
)* )*
@ -261,13 +219,7 @@ macro_rules! font_face_descriptors {
fn parse_value(&mut self, name: &str, input: &mut Parser) -> Result<(), ()> { fn parse_value(&mut self, name: &str, input: &mut Parser) -> Result<(), ()> {
match_ignore_ascii_case! { name, match_ignore_ascii_case! { name,
$( $(
$m_name => { $name => self.rule.$ident = Some(Parse::parse(self.context, input)?),
self.rule.$m_ident = Parse::parse(self.context, input)?;
self.missing.$m_ident = false
},
)*
$(
$o_name => self.rule.$o_ident = Parse::parse(self.context, input)?,
)* )*
_ => return Err(()) _ => return Err(())
} }
@ -277,18 +229,67 @@ macro_rules! font_face_descriptors {
} }
} }
macro_rules! font_face_descriptors {
(
mandatory descriptors = [
$( #[$m_doc: meta] $m_name: tt $m_ident: ident: $m_ty: ty, )*
]
optional descriptors = [
$( #[$o_doc: meta] $o_name: tt $o_ident: ident: $o_ty: ty = $o_initial: expr, )*
]
) => {
font_face_descriptors_common! {
$( #[$m_doc] $m_name $m_ident: $m_ty, )*
$( #[$o_doc] $o_name $o_ident: $o_ty, )*
}
impl FontFaceRuleData {
/// Per https://github.com/w3c/csswg-drafts/issues/1133 an @font-face rule
/// is valid as far as the CSS parser is concerned even if it doesn’t have
/// a font-family or src declaration.
///
/// However both are required for the rule to represent an actual font face.
#[cfg(feature = "servo")]
pub fn font_face(&self) -> Option<FontFace> {
if $( self.$m_ident.is_some() )&&* {
Some(FontFace(self))
} else {
None
}
}
}
#[cfg(feature = "servo")]
impl<'a> FontFace<'a> {
$(
#[$m_doc]
pub fn $m_ident(&self) -> &$m_ty {
self.0 .$m_ident.as_ref().unwrap()
}
)*
$(
#[$o_doc]
pub fn $o_ident(&self) -> $o_ty {
if let Some(ref value) = self.0 .$o_ident {
value.clone()
} else {
$o_initial
}
}
)*
}
}
}
/// css-name rust_identifier: Type = initial_value, /// css-name rust_identifier: Type = initial_value,
#[cfg(feature = "gecko")] #[cfg(feature = "gecko")]
font_face_descriptors! { font_face_descriptors! {
mandatory descriptors = [ mandatory descriptors = [
/// The name of this font face /// The name of this font face
"font-family" family: FamilyName = FamilyName { "font-family" family: FamilyName,
name: atom!(""),
quoted: true,
},
/// The alternative sources for this font face. /// The alternative sources for this font face.
"src" sources: Vec<Source> = Vec::new(), "src" sources: Vec<Source>,
] ]
optional descriptors = [ optional descriptors = [
/// The style of this font face /// The style of this font face
@ -304,6 +305,8 @@ font_face_descriptors! {
"unicode-range" unicode_range: Vec<UnicodeRange> = vec![ "unicode-range" unicode_range: Vec<UnicodeRange> = vec![
UnicodeRange { start: 0, end: 0x10FFFF } UnicodeRange { start: 0, end: 0x10FFFF }
], ],
// FIXME: add font-feature-settings, font-language-override, and font-display
] ]
} }
@ -311,13 +314,10 @@ font_face_descriptors! {
font_face_descriptors! { font_face_descriptors! {
mandatory descriptors = [ mandatory descriptors = [
/// The name of this font face /// The name of this font face
"font-family" family: FamilyName = FamilyName { "font-family" family: FamilyName,
name: atom!(""),
quoted: true,
},
/// The alternative sources for this font face. /// The alternative sources for this font face.
"src" sources: Vec<Source> = Vec::new(), "src" sources: Vec<Source>,
] ]
optional descriptors = [ optional descriptors = [
] ]

95
components/style/gecko/rules.rs
View file

@ -4,7 +4,7 @@
//! Bindings for CSS Rule objects //! Bindings for CSS Rule objects
use font_face::{FontFaceData, Source}; use font_face::{FontFaceRuleData, Source};
use gecko_bindings::bindings; use gecko_bindings::bindings;
use gecko_bindings::structs::{self, CSSFontFaceDescriptors, nsCSSFontFaceRule}; use gecko_bindings::structs::{self, CSSFontFaceDescriptors, nsCSSFontFaceRule};
use gecko_bindings::sugar::refptr::{RefPtr, UniqueRefPtr}; use gecko_bindings::sugar::refptr::{RefPtr, UniqueRefPtr};
@ -15,18 +15,22 @@ use std::fmt;
pub type FontFaceRule = RefPtr<nsCSSFontFaceRule>; pub type FontFaceRule = RefPtr<nsCSSFontFaceRule>;
fn set_font_face_descriptors(descriptors: &mut CSSFontFaceDescriptors, fn set_font_face_descriptors(descriptors: &mut CSSFontFaceDescriptors,
data: FontFaceData) { data: FontFaceRuleData) {
// font-family // font-family
descriptors.mFamily.set_string_from_atom(&data.family.name); if let Some(ref family) = data.family {
descriptors.mFamily.set_string_from_atom(&family.name);
}
macro_rules! map_enum { macro_rules! map_enum {
($target:ident = ($data:ident: $prop:ident) { ($target:ident = ($data:ident: $prop:ident) {
$($servo:ident => $gecko:ident,)+ $($servo:ident => $gecko:ident,)+
}) => {{ }) => {{
use computed_values::$prop::T; if let Some(ref value) = data.$data {
descriptors.$target.set_enum(match data.$data { use computed_values::$prop::T;
$( T::$servo => structs::$gecko as i32, )+ descriptors.$target.set_enum(match *value {
}) $( T::$servo => structs::$gecko as i32, )+
})
}
}} }}
} }
@ -38,7 +42,9 @@ fn set_font_face_descriptors(descriptors: &mut CSSFontFaceDescriptors,
}); });
// font-weight // font-weight
descriptors.mWeight.set_integer(data.weight as i32); if let Some(weight) = data.weight {
descriptors.mWeight.set_integer(weight as i32);
}
// font-stretch // font-stretch
map_enum!(mStretch = (stretch: font_stretch) { map_enum!(mStretch = (stretch: font_stretch) {
@ -54,53 +60,52 @@ fn set_font_face_descriptors(descriptors: &mut CSSFontFaceDescriptors,
}); });
// src // src
let src_len = data.sources.iter().fold(0, |acc, src| { if let Some(ref sources) = data.sources {
acc + match *src { let src_len = sources.iter().fold(0, |acc, src| {
// Each format hint takes one position in the array of mSrc. acc + match *src {
Source::Url(ref url) => url.format_hints.len() + 1, // Each format hint takes one position in the array of mSrc.
Source::Local(_) => 1, Source::Url(ref url) => url.format_hints.len() + 1,
} Source::Local(_) => 1,
}); }
let mut target_srcs = });
descriptors.mSrc.set_array(src_len as i32).as_mut_slice().iter_mut(); let mut target_srcs =
macro_rules! next { () => { descriptors.mSrc.set_array(src_len as i32).as_mut_slice().iter_mut();
target_srcs.next().expect("Length of target_srcs should be enough") macro_rules! next { () => {
} } target_srcs.next().expect("Length of target_srcs should be enough")
for src in data.sources.iter() { } }
match *src { for src in sources.iter() {
Source::Url(ref url) => { match *src {
next!().set_url(&url.url); Source::Url(ref url) => {
for hint in url.format_hints.iter() { next!().set_url(&url.url);
next!().set_font_format(&hint); for hint in url.format_hints.iter() {
next!().set_font_format(&hint);
}
}
Source::Local(ref family) => {
next!().set_local_font(&family.name);
} }
} }
Source::Local(ref family) => {
next!().set_local_font(&family.name);
}
} }
debug_assert!(target_srcs.next().is_none(), "Should have filled all slots");
} }
debug_assert!(target_srcs.next().is_none(), "Should have filled all slots");
// unicode-range // unicode-range
let target_ranges = descriptors.mUnicodeRange if let Some(ref unicode_range) = data.unicode_range {
.set_array((data.unicode_range.len() * 2) as i32) let target_ranges = descriptors.mUnicodeRange
.as_mut_slice().chunks_mut(2); .set_array((unicode_range.len() * 2) as i32)
for (range, target) in data.unicode_range.iter().zip(target_ranges) { .as_mut_slice().chunks_mut(2);
target[0].set_integer(range.start as i32); for (range, target) in unicode_range.iter().zip(target_ranges) {
target[1].set_integer(range.end as i32); target[0].set_integer(range.start as i32);
target[1].set_integer(range.end as i32);
}
} }
// The following three descriptors are not implemented yet. // Leave unset descriptors to eCSSUnit_Null,
// font-feature-settings // FontFaceSet::FindOrCreateUserFontEntryFromFontFace does the defaulting to initial values.
descriptors.mFontFeatureSettings.set_normal();
// font-language-override
descriptors.mFontLanguageOverride.set_normal();
// font-display
descriptors.mDisplay.set_enum(structs::NS_FONT_DISPLAY_AUTO as i32);
} }
impl From<FontFaceData> for FontFaceRule { impl From<FontFaceRuleData> for FontFaceRule {
fn from(data: FontFaceData) -> FontFaceRule { fn from(data: FontFaceRuleData) -> FontFaceRule {
let mut result = unsafe { let mut result = unsafe {
UniqueRefPtr::from_addrefed(bindings::Gecko_CSSFontFaceRule_Create()) UniqueRefPtr::from_addrefed(bindings::Gecko_CSSFontFaceRule_Create())
}; };

6
components/style/stylesheets.rs
View file

@ -12,7 +12,7 @@ use cssparser::{AtRuleType, RuleListParser, SourcePosition, Token, parse_one_rul
use cssparser::ToCss as ParserToCss; use cssparser::ToCss as ParserToCss;
use error_reporting::ParseErrorReporter; use error_reporting::ParseErrorReporter;
#[cfg(feature = "servo")] #[cfg(feature = "servo")]
use font_face::FontFaceData; use font_face::FontFaceRuleData;
use font_face::parse_font_face_block; use font_face::parse_font_face_block;
#[cfg(feature = "gecko")] #[cfg(feature = "gecko")]
pub use gecko::rules::FontFaceRule; pub use gecko::rules::FontFaceRule;
@ -557,7 +557,7 @@ impl ToCssWithGuard for StyleRule {
/// A @font-face rule /// A @font-face rule
#[cfg(feature = "servo")] #[cfg(feature = "servo")]
pub type FontFaceRule = FontFaceData; pub type FontFaceRule = FontFaceRuleData;
impl Stylesheet { impl Stylesheet {
/// Updates an empty stylesheet from a given string of text. /// Updates an empty stylesheet from a given string of text.
@ -1012,7 +1012,7 @@ impl<'a, 'b> AtRuleParser for NestedRuleParser<'a, 'b> {
match prelude { match prelude {
AtRulePrelude::FontFace => { AtRulePrelude::FontFace => {
Ok(CssRule::FontFace(Arc::new(self.shared_lock.wrap( Ok(CssRule::FontFace(Arc::new(self.shared_lock.wrap(
parse_font_face_block(self.context, input)?.into())))) parse_font_face_block(self.context, input).into()))))
} }
AtRulePrelude::Media(media_queries) => { AtRulePrelude::Media(media_queries) => {
Ok(CssRule::Media(Arc::new(self.shared_lock.wrap(MediaRule { Ok(CssRule::Media(Arc::new(self.shared_lock.wrap(MediaRule {

13
tests/unit/gfx/font_cache_thread.rs
View file

@ -5,7 +5,7 @@
use gfx::font_cache_thread::FontCacheThread; use gfx::font_cache_thread::FontCacheThread;
use ipc_channel::ipc; use ipc_channel::ipc;
use style::computed_values::font_family::FamilyName; use style::computed_values::font_family::FamilyName;
use style::font_face::{FontFaceData, Source}; use style::font_face::{FontFaceRuleData, Source};
#[test] #[test]
fn test_local_web_font() { fn test_local_web_font() {
@ -20,12 +20,15 @@ fn test_local_web_font() {
name: From::from("test font face"), name: From::from("test font face"),
quoted: true, quoted: true,
}; };
let font_face_rule = FontFaceData { let font_face_rule = FontFaceRuleData {
family: family_name.clone(), family: Some(family_name.clone()),
sources: vec![Source::Local(variant_name)], sources: Some(vec![Source::Local(variant_name)]),
}; };
font_cache_thread.add_web_font(family_name, font_face_rule.effective_sources(), out_chan); font_cache_thread.add_web_font(
family_name,
font_face_rule.font_face().unwrap().effective_sources(),
out_chan);
assert_eq!(out_receiver.recv().unwrap(), ()); assert_eq!(out_receiver.recv().unwrap(), ());
} }

8
tests/wpt/metadata-css/css-conditional-3_dev/html/001.htm.ini
View file

@ -1,8 +0,0 @@
[001.htm]
type: testharness
[Inserting @font-face inside @supports works]
expected: FAIL
[Inserting an @supports inside a style rule should fail]
expected: FAIL