Auto merge of #16224 - servo:valid-fontface, r=upsuper

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

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<_>`.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16224)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-04-03 03:53:09 -05:00 committed by GitHub
commit fac0d17fd6
9 changed files with 240 additions and 197 deletions

View file

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

View file

@ -2,16 +2,13 @@
* 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/. */
// 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]
interface CSSFontFaceRule : CSSRule {
// attribute DOMString family;
// attribute DOMString src;
// attribute DOMString style;
// attribute DOMString weight;
// attribute DOMString stretch;
// attribute DOMString unicodeRange;
// attribute DOMString variant;
// attribute DOMString featureSettings;
// [SameObject, PutForwards=cssText] readonly attribute CSSStyleDeclaration style;
};

View file

@ -12,11 +12,11 @@
use computed_values::{font_style, font_weight, font_stretch};
use computed_values::font_family::FamilyName;
use cssparser::{AtRuleParser, DeclarationListParser, DeclarationParser, Parser};
#[cfg(feature = "gecko")] use gecko_bindings::structs::CSSFontFaceDescriptors;
#[cfg(feature = "gecko")] use cssparser::UnicodeRange;
use parser::{ParserContext, log_css_error, Parse};
use shared_lock::{SharedRwLockReadGuard, ToCssWithGuard};
use std::fmt;
use std::iter;
use style_traits::{ToCss, OneOrMoreCommaSeparated};
use values::specified::url::SpecifiedUrl;
@ -74,14 +74,12 @@ impl ToCss for UrlSource {
/// Parse the block inside a `@font-face` rule.
///
/// Note that the prelude parsing code lives in the `stylesheets` module.
pub fn parse_font_face_block(context: &ParserContext, input: &mut Parser)
-> Result<FontFaceData, ()> {
let mut rule = FontFaceData::initial();
pub fn parse_font_face_block(context: &ParserContext, input: &mut Parser) -> FontFaceRuleData {
let mut rule = FontFaceRuleData::empty();
{
let parser = FontFaceRuleParser {
context: context,
rule: &mut rule,
missing: MissingDescriptors::new(),
};
let mut iter = DeclarationListParser::new(input, parser);
while let Some(declaration) = iter.next() {
@ -92,24 +90,27 @@ pub fn parse_font_face_block(context: &ParserContext, input: &mut Parser)
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.
#[cfg(feature = "servo")]
#[derive(Clone, Debug)]
#[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
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
/// sources which don't list any format hint, or the ones which list at
/// least "truetype" or "opentype".
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 {
let hints = &url_source.format_hints;
// We support only opentype fonts and truetype is an alias for
@ -125,7 +126,8 @@ impl FontFaceData {
}
}
impl iter::Iterator for EffectiveSources {
#[cfg(feature = "servo")]
impl Iterator for EffectiveSources {
type Item = Source;
fn next(&mut self) -> Option<Source> {
self.0.pop()
@ -134,8 +136,7 @@ impl iter::Iterator for EffectiveSources {
struct FontFaceRuleParser<'a, 'b: 'a> {
context: &'a ParserContext<'b>,
rule: &'a mut FontFaceData,
missing: MissingDescriptors,
rule: &'a mut FontFaceRuleData,
}
/// Default methods reject all at rules.
@ -172,82 +173,53 @@ impl Parse for Source {
}
}
macro_rules! font_face_descriptors {
macro_rules! font_face_descriptors_common {
(
mandatory descriptors = [
$( #[$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, )*
]
$( #[$doc: meta] $name: tt $ident: ident / $gecko_ident: ident: $ty: ty, )*
) => {
/// Data inside a `@font-face` rule.
///
/// https://drafts.csswg.org/css-fonts/#font-face-rule
#[derive(Debug, PartialEq, Eq)]
pub struct FontFaceData {
pub struct FontFaceRuleData {
$(
#[$m_doc]
pub $m_ident: $m_ty,
)*
$(
#[$o_doc]
pub $o_ident: $o_ty,
#[$doc]
pub $ident: Option<$ty>,
)*
}
struct MissingDescriptors {
impl FontFaceRuleData {
fn empty() -> Self {
FontFaceRuleData {
$(
$m_ident: bool,
)*
}
impl MissingDescriptors {
fn new() -> Self {
MissingDescriptors {
$(
$m_ident: true,
$ident: None,
)*
}
}
fn any(&self) -> bool {
/// Convert to Gecko types
#[cfg(feature = "gecko")]
pub fn set_descriptors(&self, descriptors: &mut CSSFontFaceDescriptors) {
$(
self.$m_ident
)||*
if let Some(ref value) = self.$ident {
descriptors.$gecko_ident.set_from(value)
}
)*
// Leave unset descriptors to eCSSUnit_Null,
// FontFaceSet::FindOrCreateUserFontEntryFromFontFace does the defaulting
// to initial values.
}
}
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.
fn to_css<W>(&self, _guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
where W: fmt::Write {
dest.write_str("@font-face {\n")?;
$(
dest.write_str(concat!(" ", $m_name, ": "))?;
ToCss::to_css(&self.$m_ident, dest)?;
dest.write_str(";\n")?;
)*
$(
// 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)?;
if let Some(ref value) = self.$ident {
dest.write_str(concat!(" ", $name, ": "))?;
ToCss::to_css(value, dest)?;
dest.write_str(";\n")?;
}
)*
@ -261,13 +233,14 @@ macro_rules! font_face_descriptors {
fn parse_value(&mut self, name: &str, input: &mut Parser) -> Result<(), ()> {
match_ignore_ascii_case! { name,
$(
$m_name => {
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)?,
$name => {
// DeclarationParser also calls parse_entirely
// so wed normally not need to,
// but in this case we do because we set the value as a side effect
// rather than returning it.
let value = input.parse_entirely(|i| Parse::parse(self.context, i))?;
self.rule.$ident = Some(value)
}
)*
_ => return Err(())
}
@ -277,33 +250,85 @@ macro_rules! font_face_descriptors {
}
}
macro_rules! font_face_descriptors {
(
mandatory descriptors = [
$( #[$m_doc: meta] $m_name: tt $m_ident: ident / $m_gecko_ident: ident: $m_ty: ty, )*
]
optional descriptors = [
$( #[$o_doc: meta] $o_name: tt $o_ident: ident / $o_gecko_ident: ident: $o_ty: ty =
$o_initial: expr, )*
]
) => {
font_face_descriptors_common! {
$( #[$m_doc] $m_name $m_ident / $m_gecko_ident: $m_ty, )*
$( #[$o_doc] $o_name $o_ident / $o_gecko_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 doesnt 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,
#[cfg(feature = "gecko")]
font_face_descriptors! {
mandatory descriptors = [
/// The name of this font face
"font-family" family: FamilyName = FamilyName {
name: atom!(""),
quoted: true,
},
"font-family" family / mFamily: FamilyName,
/// The alternative sources for this font face.
"src" sources: Vec<Source> = Vec::new(),
"src" sources / mSrc: Vec<Source>,
]
optional descriptors = [
/// The style of this font face
"font-style" style: font_style::T = font_style::T::normal,
"font-style" style / mStyle: font_style::T = font_style::T::normal,
/// The weight of this font face
"font-weight" weight: font_weight::T = font_weight::T::Weight400 /* normal */,
"font-weight" weight / mWeight: font_weight::T = font_weight::T::Weight400 /* normal */,
/// The stretch of this font face
"font-stretch" stretch: font_stretch::T = font_stretch::T::normal,
"font-stretch" stretch / mStretch: font_stretch::T = font_stretch::T::normal,
/// The ranges of code points outside of which this font face should not be used.
"unicode-range" unicode_range: Vec<UnicodeRange> = vec![
"unicode-range" unicode_range / mUnicodeRange: Vec<UnicodeRange> = vec![
UnicodeRange { start: 0, end: 0x10FFFF }
],
// FIXME: add font-feature-settings, font-language-override, and font-display
]
}
@ -311,13 +336,10 @@ font_face_descriptors! {
font_face_descriptors! {
mandatory descriptors = [
/// The name of this font face
"font-family" family: FamilyName = FamilyName {
name: atom!(""),
quoted: true,
},
"font-family" family / mFamily: FamilyName,
/// The alternative sources for this font face.
"src" sources: Vec<Source> = Vec::new(),
"src" sources / mSrc: Vec<Source>,
]
optional descriptors = [
]

View file

@ -4,9 +4,13 @@
//! Bindings for CSS Rule objects
use font_face::{FontFaceData, Source};
use computed_values::{font_style, font_weight, font_stretch};
use computed_values::font_family::FamilyName;
use cssparser::UnicodeRange;
use font_face::{FontFaceRuleData, Source};
use gecko_bindings::bindings;
use gecko_bindings::structs::{self, CSSFontFaceDescriptors, nsCSSFontFaceRule};
use gecko_bindings::structs::{self, nsCSSFontFaceRule, nsCSSValue};
use gecko_bindings::sugar::ns_css_value::ToNsCssValue;
use gecko_bindings::sugar::refptr::{RefPtr, UniqueRefPtr};
use shared_lock::{ToCssWithGuard, SharedRwLockReadGuard};
use std::fmt;
@ -14,34 +18,46 @@ use std::fmt;
/// A @font-face rule
pub type FontFaceRule = RefPtr<nsCSSFontFaceRule>;
fn set_font_face_descriptors(descriptors: &mut CSSFontFaceDescriptors,
data: FontFaceData) {
// font-family
descriptors.mFamily.set_string_from_atom(&data.family.name);
macro_rules! map_enum {
($target:ident = ($data:ident: $prop:ident) {
$($servo:ident => $gecko:ident,)+
}) => {{
use computed_values::$prop::T;
descriptors.$target.set_enum(match data.$data {
$( T::$servo => structs::$gecko as i32, )+
})
}}
impl ToNsCssValue for FamilyName {
fn convert(&self, nscssvalue: &mut nsCSSValue) {
nscssvalue.set_string_from_atom(&self.name)
}
}
// font-style
map_enum!(mStyle = (style: font_style) {
impl ToNsCssValue for font_weight::T {
fn convert(&self, nscssvalue: &mut nsCSSValue) {
nscssvalue.set_integer(*self as i32)
}
}
macro_rules! map_enum {
(
$(
$prop:ident {
$($servo:ident => $gecko:ident,)+
}
)+
) => {
$(
impl ToNsCssValue for $prop::T {
fn convert(&self, nscssvalue: &mut nsCSSValue) {
nscssvalue.set_enum(match *self {
$( $prop::T::$servo => structs::$gecko as i32, )+
})
}
}
)+
}
}
map_enum! {
font_style {
normal => NS_FONT_STYLE_NORMAL,
italic => NS_FONT_STYLE_ITALIC,
oblique => NS_FONT_STYLE_OBLIQUE,
});
}
// font-weight
descriptors.mWeight.set_integer(data.weight as i32);
// font-stretch
map_enum!(mStretch = (stretch: font_stretch) {
font_stretch {
normal => NS_FONT_STRETCH_NORMAL,
ultra_condensed => NS_FONT_STRETCH_ULTRA_CONDENSED,
extra_condensed => NS_FONT_STRETCH_EXTRA_CONDENSED,
@ -51,10 +67,12 @@ fn set_font_face_descriptors(descriptors: &mut CSSFontFaceDescriptors,
expanded => NS_FONT_STRETCH_EXPANDED,
extra_expanded => NS_FONT_STRETCH_EXTRA_EXPANDED,
ultra_expanded => NS_FONT_STRETCH_ULTRA_EXPANDED,
});
}
}
// src
let src_len = data.sources.iter().fold(0, |acc, src| {
impl ToNsCssValue for Vec<Source> {
fn convert(&self, nscssvalue: &mut nsCSSValue) {
let src_len = self.iter().fold(0, |acc, src| {
acc + match *src {
// Each format hint takes one position in the array of mSrc.
Source::Url(ref url) => url.format_hints.len() + 1,
@ -62,11 +80,11 @@ fn set_font_face_descriptors(descriptors: &mut CSSFontFaceDescriptors,
}
});
let mut target_srcs =
descriptors.mSrc.set_array(src_len as i32).as_mut_slice().iter_mut();
nscssvalue.set_array(src_len as i32).as_mut_slice().iter_mut();
macro_rules! next { () => {
target_srcs.next().expect("Length of target_srcs should be enough")
} }
for src in data.sources.iter() {
for src in self.iter() {
match *src {
Source::Url(ref url) => {
next!().set_url(&url.url);
@ -80,31 +98,27 @@ fn set_font_face_descriptors(descriptors: &mut CSSFontFaceDescriptors,
}
}
debug_assert!(target_srcs.next().is_none(), "Should have filled all slots");
}
}
// unicode-range
let target_ranges = descriptors.mUnicodeRange
.set_array((data.unicode_range.len() * 2) as i32)
impl ToNsCssValue for Vec<UnicodeRange> {
fn convert(&self, nscssvalue: &mut nsCSSValue) {
let target_ranges = nscssvalue
.set_array((self.len() * 2) as i32)
.as_mut_slice().chunks_mut(2);
for (range, target) in data.unicode_range.iter().zip(target_ranges) {
for (range, target) in self.iter().zip(target_ranges) {
target[0].set_integer(range.start as i32);
target[1].set_integer(range.end as i32);
}
// The following three descriptors are not implemented yet.
// font-feature-settings
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 {
fn from(data: FontFaceData) -> FontFaceRule {
impl From<FontFaceRuleData> for FontFaceRule {
fn from(data: FontFaceRuleData) -> FontFaceRule {
let mut result = unsafe {
UniqueRefPtr::from_addrefed(bindings::Gecko_CSSFontFaceRule_Create())
};
set_font_face_descriptors(&mut result.mDecl.mDescriptors, data);
data.set_descriptors(&mut result.mDecl.mDescriptors);
result.get()
}
}

View file

@ -6,7 +6,7 @@
mod ns_com_ptr;
mod ns_css_shadow_array;
mod ns_css_value;
pub mod ns_css_value;
mod ns_style_auto_array;
pub mod ns_style_coord;
mod ns_t_array;

View file

@ -163,6 +163,11 @@ impl nsCSSValue {
unsafe { bindings::Gecko_CSSValue_SetArray(self, len) }
unsafe { self.mValue.mArray.as_mut().as_mut() }.unwrap()
}
/// Generic set from any value that implements the ToNsCssValue trait.
pub fn set_from<T: ToNsCssValue>(&mut self, value: &T) {
value.convert(self)
}
}
impl Drop for nsCSSValue {
@ -210,3 +215,9 @@ impl IndexMut<usize> for nsCSSValue_Array {
&mut self.as_mut_slice()[i]
}
}
/// Generic conversion to nsCSSValue
pub trait ToNsCssValue {
/// Convert
fn convert(&self, nscssvalue: &mut nsCSSValue);
}

View file

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

View file

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

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