From a9e473c6e8ca7b006d05aeb9a4d4dfa1aefaa994 Mon Sep 17 00:00:00 2001 From: Jonathan Kew Date: Sun, 14 Apr 2019 11:06:41 +0000 Subject: [PATCH 01/27] style: Allow full-width and/or full-size-kana values of text-transform to be combined with a case transformation. Differential Revision: https://phabricator.services.mozilla.com/D27402 --- components/style/properties/data.py | 1 + .../longhands/inherited_text.mako.rs | 7 +- components/style/values/computed/mod.rs | 1 + components/style/values/computed/text.rs | 1 + components/style/values/specified/mod.rs | 1 + components/style/values/specified/text.rs | 169 ++++++++++++++++++ 6 files changed, 176 insertions(+), 4 deletions(-) diff --git a/components/style/properties/data.py b/components/style/properties/data.py index 407cc5fcdb0..b20a8deaaea 100644 --- a/components/style/properties/data.py +++ b/components/style/properties/data.py @@ -347,6 +347,7 @@ class Longhand(object): "TextAlign", "TextDecorationLine", "TextEmphasisPosition", + "TextTransform", "TouchAction", "TransformStyle", "UserSelect", diff --git a/components/style/properties/longhands/inherited_text.mako.rs b/components/style/properties/longhands/inherited_text.mako.rs index f61c8754b5e..cdd7df93256 100644 --- a/components/style/properties/longhands/inherited_text.mako.rs +++ b/components/style/properties/longhands/inherited_text.mako.rs @@ -19,11 +19,10 @@ ${helpers.predefined_type( // CSS Text Module Level 3 -// TODO(pcwalton): `full-width` -${helpers.single_keyword( +${helpers.predefined_type( "text-transform", - "none capitalize uppercase lowercase", - extra_gecko_values="full-width full-size-kana", + "TextTransform", + "computed::TextTransform::none()", animation_value_type="discrete", flags="APPLIES_TO_FIRST_LETTER APPLIES_TO_FIRST_LINE APPLIES_TO_PLACEHOLDER", spec="https://drafts.csswg.org/css-text/#propdef-text-transform", diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index 3b32f8fecd7..78384ea6b94 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -87,6 +87,7 @@ pub use self::transform::{TransformOrigin, TransformStyle, Translate}; pub use self::ui::CursorImage; pub use self::ui::{Cursor, MozForceBrokenImageIcon, UserSelect}; pub use super::specified::{BorderStyle, TextDecorationLine}; +pub use super::specified::TextTransform; pub use super::{Auto, Either, None_}; pub use app_units::Au; diff --git a/components/style/values/computed/text.rs b/components/style/values/computed/text.rs index 2fdb1e48c4f..b88ba850536 100644 --- a/components/style/values/computed/text.rs +++ b/components/style/values/computed/text.rs @@ -21,6 +21,7 @@ use style_traits::{CssWriter, ToCss}; pub use crate::values::specified::TextAlignKeyword as TextAlign; pub use crate::values::specified::{OverflowWrap, WordBreak}; pub use crate::values::specified::{TextDecorationLine, TextEmphasisPosition}; +pub use crate::values::specified::TextTransform; /// A computed value for the `initial-letter` property. pub type InitialLetter = GenericInitialLetter; diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index 64a76ccc582..64a45231c03 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -83,6 +83,7 @@ pub use self::table::XSpan; pub use self::text::{InitialLetter, LetterSpacing, LineHeight, TextAlign}; pub use self::text::{OverflowWrap, TextEmphasisPosition, TextEmphasisStyle, WordBreak}; pub use self::text::{TextAlignKeyword, TextDecorationLine, TextOverflow, WordSpacing}; +pub use self::text::TextTransform; pub use self::time::Time; pub use self::transform::{Rotate, Scale, Transform}; pub use self::transform::{TransformOrigin, TransformStyle, Translate}; diff --git a/components/style/values/specified/text.rs b/components/style/values/specified/text.rs index 9d4c6ae649c..bf4c5b288e1 100644 --- a/components/style/values/specified/text.rs +++ b/components/style/values/specified/text.rs @@ -351,6 +351,175 @@ impl TextDecorationLine { } } +#[derive( + Clone, + Copy, + Debug, + Eq, + MallocSizeOf, + PartialEq, + SpecifiedValueInfo, + ToComputedValue, + ToResolvedValue, + ToShmem)] +#[repr(C)] +/// Specified value of the text-transform property, stored in two parts: +/// the case-related transforms (mutually exclusive, only one may be in effect), and others (non-exclusive). +pub struct TextTransform { + /// Case transform, if any. + pub case_: TextTransformCase, + /// Non-case transforms. + pub other_: TextTransformOther, +} + +impl TextTransform { + #[inline] + /// Returns the initial value of text-transform + pub fn none() -> Self { + TextTransform { + case_: TextTransformCase::None, + other_: TextTransformOther::empty(), + } + } + #[inline] + /// Returns whether the value is 'none' + pub fn is_none(&self) -> bool { + self.case_ == TextTransformCase::None && self.other_.is_empty() + } +} + +impl Parse for TextTransform { + fn parse<'i, 't>( + _context: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result> { + let mut result = TextTransform::none(); + + // Case keywords are mutually exclusive; other transforms may co-occur. + loop { + let location = input.current_source_location(); + let ident = match input.next() { + Ok(&Token::Ident(ref ident)) => ident, + Ok(other) => return Err(location.new_unexpected_token_error(other.clone())), + Err(..) => break, + }; + + match_ignore_ascii_case! { ident, + "none" if result.is_none() => { + return Ok(result); + }, + "uppercase" if result.case_ == TextTransformCase::None => { + result.case_ = TextTransformCase::Uppercase + }, + "lowercase" if result.case_ == TextTransformCase::None => { + result.case_ = TextTransformCase::Lowercase + }, + "capitalize" if result.case_ == TextTransformCase::None => { + result.case_ = TextTransformCase::Capitalize + }, + "full-width" if !result.other_.intersects(TextTransformOther::FULL_WIDTH) => { + result.other_.insert(TextTransformOther::FULL_WIDTH) + }, + "full-size-kana" if !result.other_.intersects(TextTransformOther::FULL_SIZE_KANA) => { + result.other_.insert(TextTransformOther::FULL_SIZE_KANA) + } + _ => return Err(location.new_custom_error( + SelectorParseErrorKind::UnexpectedIdent(ident.clone()) + )), + } + } + + if result.is_none() { + Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) + } else { + Ok(result) + } + } +} + +impl ToCss for TextTransform { + fn to_css(&self, dest: &mut CssWriter) -> fmt::Result + where + W: Write, + { + if self.is_none() { + return dest.write_str("none"); + } + + if self.case_ != TextTransformCase::None { + self.case_.to_css(dest)?; + if !self.other_.is_empty() { + dest.write_str(" ")?; + } + } + + self.other_.to_css(dest) + } +} + +#[derive( + Clone, + Copy, + Debug, + Eq, + MallocSizeOf, + PartialEq, + SpecifiedValueInfo, + ToComputedValue, + ToCss, + ToResolvedValue, + ToShmem)] +#[repr(C)] +/// Specified keyword values for case transforms in the text-transform property. (These are exclusive.) +pub enum TextTransformCase { + /// No case transform. + None, + /// All uppercase. + Uppercase, + /// All lowercase. + Lowercase, + /// Capitalize each word. + Capitalize, +} + +bitflags! { + #[derive(MallocSizeOf, SpecifiedValueInfo, ToComputedValue, ToResolvedValue, ToShmem)] + #[value_info(other_values = "none,full-width,full-size-kana")] + #[repr(C)] + /// Specified keyword values for non-case transforms in the text-transform property. (Non-exclusive.) + pub struct TextTransformOther: u8 { + /// full-width + const FULL_WIDTH = 1 << 0; + /// full-size-kana + const FULL_SIZE_KANA = 1 << 1; + } +} + +impl ToCss for TextTransformOther { + fn to_css(&self, dest: &mut CssWriter) -> fmt::Result + where + W: Write, + { + let mut writer = SequenceWriter::new(dest, " "); + let mut any = false; + macro_rules! maybe_write { + ($ident:ident => $str:expr) => { + if self.contains(TextTransformOther::$ident) { + writer.raw_item($str)?; + any = true; + } + }; + } + + maybe_write!(FULL_WIDTH => "full-width"); + maybe_write!(FULL_SIZE_KANA => "full-size-kana"); + + debug_assert!(any || self.is_empty()); + + Ok(()) + } +} + /// Specified value of text-align keyword value. #[derive( Clone, From 498a163cdfbdcb260fb0149cb5103fe1cd6f3e34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 15 Apr 2019 20:11:45 +0000 Subject: [PATCH 02/27] style: The counters code should use atoms rather than strings. Servo already atomizes the counter names, it makes no sense to copy the string rather than bumping the refcount. Differential Revision: https://phabricator.services.mozilla.com/D27061 --- components/style/properties/gecko.mako.rs | 36 ++++++++++++-------- components/style/values/generics/counters.rs | 14 +++++--- 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 81e1ee4307d..95afdd56a79 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -4455,7 +4455,7 @@ clip-path fn set_counter_function( data: &mut nsStyleContentData, content_type: StyleContentType, - name: &CustomIdent, + name: CustomIdent, sep: &str, style: CounterStyleOrNone, ) { @@ -4464,7 +4464,9 @@ clip-path let counter_func = unsafe { bindings::Gecko_SetCounterFunction(data, content_type).as_mut().unwrap() }; - counter_func.mIdent.assign(name.0.as_slice()); + counter_func.mIdent.set_move(unsafe { + RefPtr::from_addrefed(name.0.into_addrefed()) + }); if content_type == StyleContentType::Counters { counter_func.mSeparator.assign_str(sep); } @@ -4493,14 +4495,14 @@ clip-path Gecko_ClearAndResizeStyleContents(&mut self.gecko, items.len() as u32); } - for (i, item) in items.into_iter().enumerate() { + for (i, item) in items.into_vec().into_iter().enumerate() { // NB: Gecko compares the mString value if type is not image // or URI independently of whatever gets there. In the quote // cases, they set it to null, so do the same here. unsafe { *self.gecko.mContents[i].mContent.mString.as_mut() = ptr::null_mut(); } - match *item { + match item { ContentItem::String(ref value) => { self.gecko.mContents[i].mType = StyleContentType::String; unsafe { @@ -4536,22 +4538,22 @@ clip-path => self.gecko.mContents[i].mType = StyleContentType::NoOpenQuote, ContentItem::NoCloseQuote => self.gecko.mContents[i].mType = StyleContentType::NoCloseQuote, - ContentItem::Counter(ref name, ref style) => { + ContentItem::Counter(name, style) => { set_counter_function( &mut self.gecko.mContents[i], StyleContentType::Counter, - &name, + name, "", - style.clone(), + style, ); } - ContentItem::Counters(ref name, ref sep, ref style) => { + ContentItem::Counters(name, sep, style) => { set_counter_function( &mut self.gecko.mContents[i], StyleContentType::Counters, - &name, + name, &sep, - style.clone(), + style, ); } ContentItem::Url(ref url) => { @@ -4627,7 +4629,9 @@ clip-path StyleContentType::Counter | StyleContentType::Counters => { let gecko_function = unsafe { &**gecko_content.mContent.mCounters.as_ref() }; - let ident = CustomIdent(Atom::from(&*gecko_function.mIdent)); + let ident = CustomIdent(unsafe { + Atom::from_raw(gecko_function.mIdent.mRawPtr) + }); let style = CounterStyleOrNone::from_gecko_value(&gecko_function.mCounterStyle); let style = match style { @@ -4664,8 +4668,10 @@ clip-path ) { unsafe { bindings::Gecko_ClearAndResizeCounter${counter_property}s(&mut self.gecko, v.len() as u32); - for (i, ref pair) in v.iter().enumerate() { - self.gecko.m${counter_property}s[i].mCounter.assign(pair.name.0.as_slice()); + for (i, pair) in v.0.into_vec().into_iter().enumerate() { + self.gecko.m${counter_property}s[i].mCounter.set_move( + RefPtr::from_addrefed(pair.name.0.into_addrefed()) + ); self.gecko.m${counter_property}s[i].mValue = pair.value; } } @@ -4690,7 +4696,9 @@ clip-path longhands::counter_${counter_property.lower()}::computed_value::T::new( self.gecko.m${counter_property}s.iter().map(|ref gecko_counter| { CounterPair { - name: CustomIdent(Atom::from(gecko_counter.mCounter.to_string())), + name: CustomIdent(unsafe { + Atom::from_raw(gecko_counter.mCounter.mRawPtr) + }), value: gecko_counter.mValue, } }).collect() diff --git a/components/style/values/generics/counters.rs b/components/style/values/generics/counters.rs index 17ac687a670..fbb6927b9f1 100644 --- a/components/style/values/generics/counters.rs +++ b/components/style/values/generics/counters.rs @@ -45,7 +45,7 @@ pub struct CounterPair { ToResolvedValue, ToShmem, )] -pub struct CounterIncrement(Counters); +pub struct CounterIncrement(pub Counters); impl CounterIncrement { /// Returns a new value for `counter-increment`. @@ -77,7 +77,7 @@ impl Deref for CounterIncrement { ToResolvedValue, ToShmem, )] -pub struct CounterSetOrReset(Counters); +pub struct CounterSetOrReset(pub Counters); impl CounterSetOrReset { /// Returns a new value for `counter-set` / `counter-reset`. @@ -102,6 +102,7 @@ impl Deref for CounterSetOrReset { #[derive( Clone, Debug, + Default, MallocSizeOf, PartialEq, SpecifiedValueInfo, @@ -112,10 +113,13 @@ impl Deref for CounterSetOrReset { )] pub struct Counters(#[css(iterable, if_empty = "none")] Box<[CounterPair]>); -impl Default for Counters { +impl Counters { + /// Move out the Box into a vector. This could just return the Box<>, but + /// Vec<> is a bit more convenient because Box<[T]> doesn't implement + /// IntoIter: https://github.com/rust-lang/rust/issues/59878 #[inline] - fn default() -> Self { - Counters(vec![].into_boxed_slice()) + pub fn into_vec(self) -> Vec> { + self.0.into_vec() } } From 09d497db3d05773d7cf1b33b7bb261bab1b9413e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 16 Apr 2019 13:16:56 +0000 Subject: [PATCH 03/27] style: Refactor the selector parser to make implementing ::part() easier. ::slotted() is already weird in the sense that it supports a pseudo-element afterwards (so ::slotted(*)::before is valid for example). ::part() is weirder because you are supposed to allow stuff like ::part(foo):hover, ::part(foo):hover::before, etc. In order to avoid making the already-complex parse_compound_selector more complex, shuffle stuff so that we pass the progress of our current compound selector around, and is the parsing code for each selector which decides whether it's ok to parse at the given point. Differential Revision: https://phabricator.services.mozilla.com/D27158 --- components/selectors/parser.rs | 353 +++++++++++----------- components/style/gecko/pseudo_element.rs | 12 +- components/style/gecko/selector_parser.rs | 19 +- 3 files changed, 191 insertions(+), 193 deletions(-) diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index 0515eaca193..afb7280508d 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -30,17 +30,10 @@ pub trait PseudoElement: Sized + ToCss { /// Whether the pseudo-element supports a given state selector to the right /// of it. - fn supports_pseudo_class( - &self, - _pseudo_class: &::NonTSPseudoClass, - ) -> bool { - false - } + fn accepts_state_pseudo_classes(&self) -> bool { false } /// Whether this pseudo-element is valid after a ::slotted(..) pseudo. - fn valid_after_slotted(&self) -> bool { - false - } + fn valid_after_slotted(&self) -> bool { false } } /// A trait that represents a pseudo-class. @@ -50,6 +43,11 @@ pub trait NonTSPseudoClass: Sized + ToCss { /// Whether this pseudo-class is :active or :hover. fn is_active_or_hover(&self) -> bool; + + /// Whether this pseudo-class belongs to: + /// + /// https://drafts.csswg.org/selectors-4/#useraction-pseudos + fn is_user_action_state(&self) -> bool; } /// Returns a Cow::Borrowed if `s` is already ASCII lowercase, and a @@ -64,6 +62,55 @@ fn to_ascii_lowercase(s: &str) -> Cow { } } +bitflags! { + /// Flags that indicate at which point of parsing a selector are we. + struct SelectorParsingState: u8 { + /// Whether we're inside a negation. If we're inside a negation, we're + /// not allowed to add another negation or such, for example. + const INSIDE_NEGATION = 1 << 0; + /// Whether we've parsed an ::slotted() pseudo-element already. + /// + /// If so, then we can only parse a subset of pseudo-elements, and + /// whatever comes after them if so. + const AFTER_SLOTTED = 1 << 1; + /// Whether we've parsed a pseudo-element (as in, an + /// `Impl::PseudoElement` thus not accounting for `::slotted`) already. + /// + /// If so, then other pseudo-elements and most other selectors are + /// disallowed. + const AFTER_PSEUDO_ELEMENT = 1 << 2; + /// Whether we've parsed a non-stateful pseudo-element (again, as-in + /// `Impl::PseudoElement`) already. If so, then other pseudo-classes are + /// disallowed. If this flag is set, `AFTER_PSEUDO_ELEMENT` must be set + /// as well. + const AFTER_NON_STATEFUL_PSEUDO_ELEMENT = 1 << 3; + /// Whether we are after any of the pseudo-like things. + const AFTER_PSEUDO = Self::AFTER_SLOTTED.bits | Self::AFTER_PSEUDO_ELEMENT.bits; + } +} + +impl SelectorParsingState { + #[inline] + fn allows_functional_pseudo_classes(self) -> bool { + !self.intersects(SelectorParsingState::AFTER_PSEUDO) + } + + #[inline] + fn allows_slotted(self) -> bool { + !self.intersects(SelectorParsingState::AFTER_PSEUDO) + } + + #[inline] + fn allows_non_functional_pseudo_classes(self) -> bool { + !self.intersects(SelectorParsingState::AFTER_SLOTTED | SelectorParsingState::AFTER_NON_STATEFUL_PSEUDO_ELEMENT) + } + + #[inline] + fn allows_tree_structural_pseudo_classes(self) -> bool { + !self.intersects(SelectorParsingState::AFTER_PSEUDO) + } +} + pub type SelectorParseError<'i> = ParseError<'i, SelectorParseErrorKind<'i>>; #[derive(Clone, Debug, PartialEq)] @@ -76,6 +123,7 @@ pub enum SelectorParseErrorKind<'i> { NonCompoundSelector, NonPseudoElementAfterSlotted, InvalidPseudoElementAfterSlotted, + InvalidState, UnexpectedTokenInAttributeSelector(Token<'i>), PseudoElementExpectedColon(Token<'i>), PseudoElementExpectedIdent(Token<'i>), @@ -1369,9 +1417,9 @@ where 'outer_loop: loop { // Parse a sequence of simple selectors. match parse_compound_selector(parser, input, &mut builder)? { - Some((has_pseudo, slot)) => { - has_pseudo_element = has_pseudo; - slotted = slot; + Some(state) => { + has_pseudo_element = state.intersects(SelectorParsingState::AFTER_PSEUDO_ELEMENT); + slotted = state.intersects(SelectorParsingState::AFTER_SLOTTED); }, None => { return Err(input.new_custom_error(if builder.has_combinators() { @@ -1848,7 +1896,7 @@ where Err(e) => return Err(e.into()), }; if !is_type_sel { - match parse_one_simple_selector(parser, input, /* inside_negation = */ true)? { + match parse_one_simple_selector(parser, input, SelectorParsingState::INSIDE_NEGATION)? { Some(SimpleSelectorParseResult::SimpleSelector(s)) => { sequence.push(s); }, @@ -1875,14 +1923,11 @@ where /// /// `Err(())` means invalid selector. /// `Ok(None)` is an empty selector -/// -/// The booleans represent whether a pseudo-element has been parsed, and whether -/// ::slotted() has been parsed, respectively. fn parse_compound_selector<'i, 't, P, Impl>( parser: &P, input: &mut CssParser<'i, 't>, builder: &mut SelectorBuilder, -) -> Result, ParseError<'i, P::Error>> +) -> Result, ParseError<'i, P::Error>> where P: Parser<'i, Impl = Impl>, Impl: SelectorImpl, @@ -1901,122 +1946,44 @@ where empty = false; } - let mut pseudo = false; - let mut slot = false; + let mut state = SelectorParsingState::empty(); loop { let parse_result = - match parse_one_simple_selector(parser, input, /* inside_negation = */ false)? { + match parse_one_simple_selector(parser, input, state)? { None => break, Some(result) => result, }; empty = false; - let slotted_selector; - let pseudo_element; - match parse_result { SimpleSelectorParseResult::SimpleSelector(s) => { builder.push_simple_selector(s); - continue; - }, - SimpleSelectorParseResult::PseudoElement(p) => { - slotted_selector = None; - pseudo_element = Some(p); }, SimpleSelectorParseResult::SlottedPseudo(selector) => { - slotted_selector = Some(selector); - let maybe_pseudo = - parse_one_simple_selector(parser, input, /* inside_negation = */ false)?; - - pseudo_element = match maybe_pseudo { - None => None, - Some(SimpleSelectorParseResult::PseudoElement(pseudo)) => { - if !pseudo.valid_after_slotted() { - return Err(input.new_custom_error( - SelectorParseErrorKind::InvalidPseudoElementAfterSlotted, - )); - } - Some(pseudo) - }, - Some(SimpleSelectorParseResult::SimpleSelector(..)) | - Some(SimpleSelectorParseResult::SlottedPseudo(..)) => { - return Err(input.new_custom_error( - SelectorParseErrorKind::NonPseudoElementAfterSlotted, - )); - }, - }; + state.insert(SelectorParsingState::AFTER_SLOTTED); + if !builder.is_empty() { + builder.push_combinator(Combinator::SlotAssignment); + } + builder.push_simple_selector(Component::Slotted(selector)); + }, + SimpleSelectorParseResult::PseudoElement(p) => { + state.insert(SelectorParsingState::AFTER_PSEUDO_ELEMENT); + if !p.accepts_state_pseudo_classes() { + state.insert(SelectorParsingState::AFTER_NON_STATEFUL_PSEUDO_ELEMENT); + } + if !builder.is_empty() { + builder.push_combinator(Combinator::PseudoElement); + } + builder.push_simple_selector(Component::PseudoElement(p)); }, } - - debug_assert!(slotted_selector.is_some() || pseudo_element.is_some()); - // Try to parse state to the right of the pseudo-element. - // - // There are only 3 allowable state selectors that can go on - // pseudo-elements as of right now. - let mut state_selectors = SmallVec::<[Component; 3]>::new(); - if let Some(ref p) = pseudo_element { - loop { - let location = input.current_source_location(); - match input.next_including_whitespace() { - Ok(&Token::Colon) => {}, - Ok(&Token::WhiteSpace(_)) | Err(_) => break, - Ok(t) => { - let e = SelectorParseErrorKind::PseudoElementExpectedColon(t.clone()); - return Err(location.new_custom_error(e)); - }, - } - - let location = input.current_source_location(); - // TODO(emilio): Functional pseudo-classes too? - // We don't need it for now. - let name = match input.next_including_whitespace()? { - &Token::Ident(ref name) => name.clone(), - t => { - return Err(location.new_custom_error( - SelectorParseErrorKind::NoIdentForPseudo(t.clone()), - )); - }, - }; - - let pseudo_class = P::parse_non_ts_pseudo_class(parser, location, name.clone())?; - if !p.supports_pseudo_class(&pseudo_class) { - return Err(input.new_custom_error( - SelectorParseErrorKind::UnsupportedPseudoClassOrElement(name), - )); - } - state_selectors.push(Component::NonTSPseudoClass(pseudo_class)); - } - } - - if let Some(slotted) = slotted_selector { - slot = true; - if !builder.is_empty() { - builder.push_combinator(Combinator::SlotAssignment); - } - builder.push_simple_selector(Component::Slotted(slotted)); - } - - if let Some(p) = pseudo_element { - pseudo = true; - if !builder.is_empty() { - builder.push_combinator(Combinator::PseudoElement); - } - - builder.push_simple_selector(Component::PseudoElement(p)); - - for state_selector in state_selectors.drain() { - builder.push_simple_selector(state_selector); - } - } - - break; } if empty { // An empty selector is invalid. Ok(None) } else { - Ok(Some((pseudo, slot))) + Ok(Some(state)) } } @@ -2024,12 +1991,18 @@ fn parse_functional_pseudo_class<'i, 't, P, Impl>( parser: &P, input: &mut CssParser<'i, 't>, name: CowRcStr<'i>, - inside_negation: bool, + state: SelectorParsingState, ) -> Result, ParseError<'i, P::Error>> where P: Parser<'i, Impl = Impl>, Impl: SelectorImpl, { + if !state.allows_functional_pseudo_classes() { + return Err(input.new_custom_error( + SelectorParseErrorKind::InvalidState + )); + } + debug_assert!(state.allows_tree_structural_pseudo_classes()); match_ignore_ascii_case! { &name, "nth-child" => return Ok(parse_nth_pseudo_class(input, Component::NthChild)?), "nth-of-type" => return Ok(parse_nth_pseudo_class(input, Component::NthOfType)?), @@ -2037,11 +2010,12 @@ where "nth-last-of-type" => return Ok(parse_nth_pseudo_class(input, Component::NthLastOfType)?), "host" => return Ok(Component::Host(Some(parse_inner_compound_selector(parser, input)?))), "not" => { - if inside_negation { + if state.intersects(SelectorParsingState::INSIDE_NEGATION) { return Err(input.new_custom_error( SelectorParseErrorKind::UnexpectedIdent("not".into()) )); } + debug_assert!(state.is_empty()); return parse_negation(parser, input) }, _ => {} @@ -2080,37 +2054,52 @@ pub fn is_css2_pseudo_element(name: &str) -> bool { fn parse_one_simple_selector<'i, 't, P, Impl>( parser: &P, input: &mut CssParser<'i, 't>, - inside_negation: bool, + state: SelectorParsingState, ) -> Result>, ParseError<'i, P::Error>> where P: Parser<'i, Impl = Impl>, Impl: SelectorImpl, { let start = input.state(); - // FIXME: remove clone() when lifetimes are non-lexical - match input.next_including_whitespace().map(|t| t.clone()) { - Ok(Token::IDHash(id)) => { + let token = match input.next_including_whitespace().map(|t| t.clone()) { + Ok(t) => t, + Err(..) => { + input.reset(&start); + return Ok(None); + } + }; + + Ok(Some(match token { + Token::IDHash(id) => { + if state.intersects(SelectorParsingState::AFTER_PSEUDO) { + return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState)); + } let id = Component::ID(id.as_ref().into()); - Ok(Some(SimpleSelectorParseResult::SimpleSelector(id))) + SimpleSelectorParseResult::SimpleSelector(id) }, - Ok(Token::Delim('.')) => { + Token::Delim('.') => { + if state.intersects(SelectorParsingState::AFTER_PSEUDO) { + return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState)); + } let location = input.current_source_location(); - match *input.next_including_whitespace()? { - Token::Ident(ref class) => { - let class = Component::Class(class.as_ref().into()); - Ok(Some(SimpleSelectorParseResult::SimpleSelector(class))) - }, + let class = match *input.next_including_whitespace()? { + Token::Ident(ref class) => class, ref t => { let e = SelectorParseErrorKind::ClassNeedsIdent(t.clone()); - Err(location.new_custom_error(e)) + return Err(location.new_custom_error(e)) }, + }; + let class = Component::Class(class.as_ref().into()); + SimpleSelectorParseResult::SimpleSelector(class) + }, + Token::SquareBracketBlock => { + if state.intersects(SelectorParsingState::AFTER_PSEUDO) { + return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState)); } - }, - Ok(Token::SquareBracketBlock) => { let attr = input.parse_nested_block(|input| parse_attribute_selector(parser, input))?; - Ok(Some(SimpleSelectorParseResult::SimpleSelector(attr))) + SimpleSelectorParseResult::SimpleSelector(attr) }, - Ok(Token::Colon) => { + Token::Colon => { let location = input.current_source_location(); let (is_single_colon, next_token) = match input.next_including_whitespace()?.clone() { Token::Colon => (false, input.next_including_whitespace()?.clone()), @@ -2127,69 +2116,83 @@ where let is_pseudo_element = !is_single_colon || P::pseudo_element_allows_single_colon(&name); if is_pseudo_element { - let parse_result = if is_functional { + if state.intersects(SelectorParsingState::AFTER_PSEUDO_ELEMENT) { + return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState)); + } + let pseudo_element = if is_functional { if P::parse_slotted(parser) && name.eq_ignore_ascii_case("slotted") { + if !state.allows_slotted() { + return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState)); + } let selector = input.parse_nested_block(|input| { parse_inner_compound_selector(parser, input) })?; - SimpleSelectorParseResult::SlottedPseudo(selector) - } else { - let selector = input.parse_nested_block(|input| { - P::parse_functional_pseudo_element(parser, name, input) - })?; - SimpleSelectorParseResult::PseudoElement(selector) + return Ok(Some(SimpleSelectorParseResult::SlottedPseudo(selector))); } + input.parse_nested_block(|input| { + P::parse_functional_pseudo_element(parser, name, input) + })? } else { - SimpleSelectorParseResult::PseudoElement(P::parse_pseudo_element( - parser, location, name, - )?) + P::parse_pseudo_element(parser, location, name)? }; - Ok(Some(parse_result)) + + if state.intersects(SelectorParsingState::AFTER_SLOTTED) && !pseudo_element.valid_after_slotted() { + return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState)); + } + SimpleSelectorParseResult::PseudoElement(pseudo_element) } else { let pseudo_class = if is_functional { input.parse_nested_block(|input| { - parse_functional_pseudo_class(parser, input, name, inside_negation) + parse_functional_pseudo_class(parser, input, name, state) })? } else { - parse_simple_pseudo_class(parser, location, name)? + parse_simple_pseudo_class(parser, location, name, state)? }; - Ok(Some(SimpleSelectorParseResult::SimpleSelector( - pseudo_class, - ))) + SimpleSelectorParseResult::SimpleSelector(pseudo_class) } }, _ => { input.reset(&start); - Ok(None) + return Ok(None) }, - } + })) } fn parse_simple_pseudo_class<'i, P, Impl>( parser: &P, location: SourceLocation, name: CowRcStr<'i>, + state: SelectorParsingState, ) -> Result, ParseError<'i, P::Error>> where P: Parser<'i, Impl = Impl>, Impl: SelectorImpl, { - (match_ignore_ascii_case! { &name, - "first-child" => Ok(Component::FirstChild), - "last-child" => Ok(Component::LastChild), - "only-child" => Ok(Component::OnlyChild), - "root" => Ok(Component::Root), - "empty" => Ok(Component::Empty), - "scope" => Ok(Component::Scope), - "host" if P::parse_host(parser) => Ok(Component::Host(None)), - "first-of-type" => Ok(Component::FirstOfType), - "last-of-type" => Ok(Component::LastOfType), - "only-of-type" => Ok(Component::OnlyOfType), - _ => Err(()) - }) - .or_else(|()| { - P::parse_non_ts_pseudo_class(parser, location, name).map(Component::NonTSPseudoClass) - }) + if !state.allows_non_functional_pseudo_classes() { + return Err(location.new_custom_error(SelectorParseErrorKind::InvalidState)); + } + + if state.allows_tree_structural_pseudo_classes() { + match_ignore_ascii_case! { &name, + "first-child" => return Ok(Component::FirstChild), + "last-child" => return Ok(Component::LastChild), + "only-child" => return Ok(Component::OnlyChild), + "root" => return Ok(Component::Root), + "empty" => return Ok(Component::Empty), + "scope" => return Ok(Component::Scope), + "host" if P::parse_host(parser) => return Ok(Component::Host(None)), + "first-of-type" => return Ok(Component::FirstOfType), + "last-of-type" => return Ok(Component::LastOfType), + "only-of-type" => return Ok(Component::OnlyOfType), + _ => {}, + } + } + + let pseudo_class = P::parse_non_ts_pseudo_class(parser, location, name)?; + if state.intersects(SelectorParsingState::AFTER_PSEUDO_ELEMENT) && !pseudo_class.is_user_action_state() { + return Err(location.new_custom_error(SelectorParseErrorKind::InvalidState)); + } + Ok(Component::NonTSPseudoClass(pseudo_class)) } // NB: pub module in order to access the DummyParser @@ -2218,16 +2221,9 @@ pub mod tests { impl parser::PseudoElement for PseudoElement { type Impl = DummySelectorImpl; - fn supports_pseudo_class(&self, pc: &PseudoClass) -> bool { - match *pc { - PseudoClass::Hover => true, - PseudoClass::Active | PseudoClass::Lang(..) => false, - } - } + fn accepts_state_pseudo_classes(&self) -> bool { true } - fn valid_after_slotted(&self) -> bool { - true - } + fn valid_after_slotted(&self) -> bool { true } } impl parser::NonTSPseudoClass for PseudoClass { @@ -2237,6 +2233,11 @@ pub mod tests { fn is_active_or_hover(&self) -> bool { matches!(*self, PseudoClass::Active | PseudoClass::Hover) } + + #[inline] + fn is_user_action_state(&self) -> bool { + self.is_active_or_hover() + } } impl ToCss for PseudoClass { @@ -2789,11 +2790,11 @@ pub mod tests { specificity(0, 2, 1) | HAS_PSEUDO_BIT, )])) ); - assert!(parse("::before:hover:active").is_err()); + assert!(parse("::before:hover:lang(foo)").is_err()); assert!(parse("::before:hover .foo").is_err()); assert!(parse("::before .foo").is_err()); assert!(parse("::before ~ bar").is_err()); - assert!(parse("::before:active").is_err()); + assert!(parse("::before:active").is_ok()); // https://github.com/servo/servo/issues/15335 assert!(parse(":: before").is_err()); diff --git a/components/style/gecko/pseudo_element.rs b/components/style/gecko/pseudo_element.rs index 479c12b9cde..7538b785858 100644 --- a/components/style/gecko/pseudo_element.rs +++ b/components/style/gecko/pseudo_element.rs @@ -11,7 +11,7 @@ use crate::gecko_bindings::structs::{self, PseudoStyleType}; use crate::properties::longhands::display::computed_value::T as Display; use crate::properties::{ComputedValues, PropertyFlags}; -use crate::selector_parser::{NonTSPseudoClass, PseudoElementCascadeType, SelectorImpl}; +use crate::selector_parser::{PseudoElementCascadeType, SelectorImpl}; use crate::str::{starts_with_ignore_ascii_case, string_as_ascii_lowercase}; use crate::string_cache::Atom; use crate::values::serialize_atom_identifier; @@ -30,6 +30,7 @@ impl ::selectors::parser::PseudoElement for PseudoElement { // ::slotted() should support all tree-abiding pseudo-elements, see // https://drafts.csswg.org/css-scoping/#slotted-pseudo // https://drafts.csswg.org/css-pseudo-4/#treelike + #[inline] fn valid_after_slotted(&self) -> bool { matches!( *self, @@ -40,12 +41,9 @@ impl ::selectors::parser::PseudoElement for PseudoElement { ) } - fn supports_pseudo_class(&self, pseudo_class: &NonTSPseudoClass) -> bool { - if !self.supports_user_action_state() { - return false; - } - - return pseudo_class.is_safe_user_action_state(); + #[inline] + fn accepts_state_pseudo_classes(&self) -> bool { + self.supports_user_action_state() } } diff --git a/components/style/gecko/selector_parser.rs b/components/style/gecko/selector_parser.rs index 41de05c33b9..36558b9f5db 100644 --- a/components/style/gecko/selector_parser.rs +++ b/components/style/gecko/selector_parser.rs @@ -184,16 +184,6 @@ impl NonTSPseudoClass { } } - /// - /// - /// We intentionally skip the link-related ones. - pub fn is_safe_user_action_state(&self) -> bool { - matches!( - *self, - NonTSPseudoClass::Hover | NonTSPseudoClass::Active | NonTSPseudoClass::Focus - ) - } - /// Get the state flag associated with a pseudo-class, if any. pub fn state_flag(&self) -> ElementState { macro_rules! flag { @@ -279,6 +269,15 @@ impl ::selectors::parser::NonTSPseudoClass for NonTSPseudoClass { fn is_active_or_hover(&self) -> bool { matches!(*self, NonTSPseudoClass::Active | NonTSPseudoClass::Hover) } + + /// We intentionally skip the link-related ones. + #[inline] + fn is_user_action_state(&self) -> bool { + matches!( + *self, + NonTSPseudoClass::Hover | NonTSPseudoClass::Active | NonTSPseudoClass::Focus + ) + } } /// The dummy struct we use to implement our selector parsing. From 477bda81d34dbfc1709d7d81775fdb28e1f791d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 16 Apr 2019 17:34:50 +0200 Subject: [PATCH 04/27] style: Unify a bit Servo and Gecko code in properties.mako.rs. Mostly removing #[cfg] statements, so npotb. --- components/style/properties/gecko.mako.rs | 3 --- components/style/properties/properties.mako.rs | 6 ++---- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 95afdd56a79..2dde6af2a57 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -1104,9 +1104,6 @@ impl ${style_struct.gecko_struct_name} { } result } - pub fn get_gecko(&self) -> &${style_struct.gecko_ffi_name} { - &self.gecko - } } impl Drop for ${style_struct.gecko_struct_name} { fn drop(&mut self) { diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 5f44a259641..9943c3509e6 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -27,7 +27,7 @@ use crate::context::QuirksMode; #[cfg(feature = "servo")] use crate::logical_geometry::LogicalMargin; #[cfg(feature = "servo")] use crate::computed_values; use crate::logical_geometry::WritingMode; -#[cfg(feature = "gecko")] use malloc_size_of::{MallocSizeOf, MallocSizeOfOps}; +use malloc_size_of::{MallocSizeOf, MallocSizeOfOps}; use crate::media_queries::Device; use crate::parser::ParserContext; use crate::properties::longhands::system_font::SystemFont; @@ -376,7 +376,6 @@ impl PartialEq for PropertyDeclaration { } } -#[cfg(feature = "gecko")] impl MallocSizeOf for PropertyDeclaration { #[inline] fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize { @@ -3745,8 +3744,7 @@ pub fn adjust_border_width(style: &mut StyleBuilder) { } /// An identifier for a given alias property. -#[derive(Clone, Copy, Eq, PartialEq)] -#[cfg_attr(feature = "servo", derive(MallocSizeOf))] +#[derive(Clone, Copy, Eq, PartialEq, MallocSizeOf)] #[repr(u16)] pub enum AliasId { % for i, property in enumerate(data.all_aliases()): From 098eb300ac759a4db5cdedae0c9db1c5c005538c Mon Sep 17 00:00:00 2001 From: Mats Palmgren Date: Thu, 18 Apr 2019 01:16:03 +0200 Subject: [PATCH 05/27] style: [cssom][css-grid] 'grid-auto-flow: row dense' should serialize to 'dense' since 'row' is implied. Differential Revision: https://phabricator.services.mozilla.com/D28058 --- components/style/values/specified/position.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/components/style/values/specified/position.rs b/components/style/values/specified/position.rs index f75af3f5585..c0ac56c9822 100644 --- a/components/style/values/specified/position.rs +++ b/components/style/values/specified/position.rs @@ -460,6 +460,11 @@ pub enum AutoFlow { Column, } +/// If `dense` is specified, `row` is implied. +fn is_row_dense(autoflow: &AutoFlow, dense: &bool) -> bool { + *autoflow == AutoFlow::Row && *dense +} + #[derive( Clone, Copy, @@ -477,6 +482,7 @@ pub enum AutoFlow { /// specifying exactly how auto-placed items get flowed into the grid pub struct GridAutoFlow { /// Specifiy how auto-placement algorithm fills each `row` or `column` in turn + #[css(contextual_skip_if = "is_row_dense")] pub autoflow: AutoFlow, /// Specify use `dense` packing algorithm or not #[css(represents_keyword)] From c0b17cc84406e51549955d46b225277a14495e23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 19 Apr 2019 04:20:31 +0000 Subject: [PATCH 06/27] style: Don't keep two list of stylesheets in ServoStyleSet. Just one set of stylesheets is enough. While at it, unify SheetType and Origin. Differential Revision: https://phabricator.services.mozilla.com/D27564 --- components/style/gecko/conversions.rs | 14 ++------------ components/style/stylesheet_set.rs | 7 +++++++ components/style/stylesheets/origin.rs | 11 +++++------ components/style/stylist.rs | 12 ++++++++++++ 4 files changed, 26 insertions(+), 18 deletions(-) diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs index 43438438a40..44dbc1bb04b 100644 --- a/components/style/gecko/conversions.rs +++ b/components/style/gecko/conversions.rs @@ -13,9 +13,9 @@ use crate::gecko::values::GeckoStyleCoordConvertible; use crate::gecko_bindings::bindings; use crate::gecko_bindings::structs::{self, nsStyleCoord_CalcValue, Matrix4x4Components}; -use crate::gecko_bindings::structs::{nsStyleImage, nsresult, SheetType}; +use crate::gecko_bindings::structs::{nsStyleImage, nsresult}; use crate::gecko_bindings::sugar::ns_style_coord::{CoordData, CoordDataMut, CoordDataValue}; -use crate::stylesheets::{Origin, RulesMutateError}; +use crate::stylesheets::RulesMutateError; use crate::values::computed::image::LineDirection; use crate::values::computed::transform::Matrix3D; use crate::values::computed::url::ComputedImageUrl; @@ -818,16 +818,6 @@ impl From for nsresult { } } -impl From for SheetType { - fn from(other: Origin) -> Self { - match other { - Origin::UserAgent => SheetType::Agent, - Origin::Author => SheetType::Doc, - Origin::User => SheetType::User, - } - } -} - impl TrackSize { /// Return TrackSize from given two nsStyleCoord pub fn from_gecko_style_coords(gecko_min: &T, gecko_max: &T) -> Self { diff --git a/components/style/stylesheet_set.rs b/components/style/stylesheet_set.rs index d2c3ad0f88e..5e72f7a219f 100644 --- a/components/style/stylesheet_set.rs +++ b/components/style/stylesheet_set.rs @@ -468,7 +468,14 @@ where .fold(0, |s, (item, _)| s + item.len()) } + /// Returns the count of stylesheets for a given origin. + #[inline] + pub fn sheet_count(&self, origin: Origin) -> usize { + self.collections.borrow_for_origin(&origin).len() + } + /// Returns the `index`th stylesheet in the set for the given origin. + #[inline] pub fn get(&self, origin: Origin, index: usize) -> Option<&S> { self.collections.borrow_for_origin(&origin).get(index) } diff --git a/components/style/stylesheets/origin.rs b/components/style/stylesheets/origin.rs index 783b8f26a8b..a65b61fca13 100644 --- a/components/style/stylesheets/origin.rs +++ b/components/style/stylesheets/origin.rs @@ -10,18 +10,17 @@ use std::ops::BitOrAssign; /// Each style rule has an origin, which determines where it enters the cascade. /// /// -#[derive(Clone, Copy, Debug, Eq, PartialEq, ToShmem)] +#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, ToShmem)] #[repr(u8)] -#[cfg_attr(feature = "servo", derive(MallocSizeOf))] pub enum Origin { /// - UserAgent = 1 << 0, + UserAgent = 0x1, /// - User = 1 << 1, + User = 0x2, /// - Author = 1 << 2, + Author = 0x4, } impl Origin { @@ -59,7 +58,7 @@ impl Origin { bitflags! { /// A set of origins. This is equivalent to Gecko's OriginFlags. - #[cfg_attr(feature = "servo", derive(MallocSizeOf))] + #[derive(MallocSizeOf)] pub struct OriginSet: u8 { /// const ORIGIN_USER_AGENT = Origin::UserAgent as u8; diff --git a/components/style/stylist.rs b/components/style/stylist.rs index ece14e9896f..ad7f9fd8585 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -591,6 +591,18 @@ impl Stylist { .remove_stylesheet(Some(&self.device), sheet, guard) } + /// Appends a new stylesheet to the current set. + #[inline] + pub fn sheet_count(&self, origin: Origin) -> usize { + self.stylesheets.sheet_count(origin) + } + + /// Appends a new stylesheet to the current set. + #[inline] + pub fn sheet_at(&self, origin: Origin, index: usize) -> Option<&StylistSheet> { + self.stylesheets.get(origin, index) + } + /// Returns whether for any of the applicable style rule data a given /// condition is true. pub fn any_applicable_rule_data(&self, element: E, mut f: F) -> bool From 52026f602b9c15f4a854512f6d79a447dc85e048 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 19 Apr 2019 04:41:17 +0000 Subject: [PATCH 07/27] style: Don't allow to parse XUL tree pseudo-elements with a single colon. Now that they're not exposed to the web we can remove this special case. Differential Revision: https://phabricator.services.mozilla.com/D28071 --- components/selectors/parser.rs | 10 ++-------- components/style/gecko/selector_parser.rs | 5 ----- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index afb7280508d..e3afab57e16 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -191,12 +191,6 @@ pub trait Parser<'i> { type Impl: SelectorImpl; type Error: 'i + From>; - /// Whether the name is a pseudo-element that can be specified with - /// the single colon syntax in addition to the double-colon syntax. - fn pseudo_element_allows_single_colon(name: &str) -> bool { - is_css2_pseudo_element(name) - } - /// Whether to parse the `::slotted()` pseudo-element. fn parse_slotted(&self) -> bool { false @@ -2038,7 +2032,7 @@ where /// Returns whether the name corresponds to a CSS2 pseudo-element that /// can be specified with the single colon syntax (in addition to the /// double-colon syntax, which can be used for all pseudo-elements). -pub fn is_css2_pseudo_element(name: &str) -> bool { +fn is_css2_pseudo_element(name: &str) -> bool { // ** Do not add to this list! ** match_ignore_ascii_case! { name, "before" | "after" | "first-line" | "first-letter" => true, @@ -2114,7 +2108,7 @@ where }, }; let is_pseudo_element = - !is_single_colon || P::pseudo_element_allows_single_colon(&name); + !is_single_colon || is_css2_pseudo_element(&name); if is_pseudo_element { if state.intersects(SelectorParsingState::AFTER_PSEUDO_ELEMENT) { return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState)); diff --git a/components/style/gecko/selector_parser.rs b/components/style/gecko/selector_parser.rs index 36558b9f5db..ec0db9286ec 100644 --- a/components/style/gecko/selector_parser.rs +++ b/components/style/gecko/selector_parser.rs @@ -351,11 +351,6 @@ impl<'a, 'i> ::selectors::Parser<'i> for SelectorParser<'a> { self.parse_slotted() } - fn pseudo_element_allows_single_colon(name: &str) -> bool { - // FIXME: -moz-tree check should probably be ascii-case-insensitive. - ::selectors::parser::is_css2_pseudo_element(name) || name.starts_with("-moz-tree-") - } - fn parse_non_ts_pseudo_class( &self, location: SourceLocation, From 50312e14576a9b8317372b44cb48a97e41464812 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 22 Apr 2019 19:45:06 +0000 Subject: [PATCH 08/27] style: Drop unused user-agent cascade datas when not holding the cache lock. We want to drop the cascade data memory as soon as possible, so bug 1544546 introduced an UpdateStylistIfNeeded call from ShellDetachedFromDocument. Unfortunately, this call can reenter into the global user-agent cascade data if some of the CSS values kept alive by the cascade data keep alive an SVG document, see the stack on this bug for an example. Make sure to drop the user-agent cascade datas when not holding the cache lock to avoid this situation. Before bug 1535788, we just destroyed the stylist, so we kept holding a reference from the cache, and that reference will be dropped sometime later when other document updated their user-agent stylesheets (if they happened not to match the cache of course). Seems to me this doesn't ended up happening in our automation, but it could happen in the wild, in theory at least. It's nice that Rust made this a safe deadlock caught by our tests rather than a very subtle and infrequent memory corruption. The relevant SVG documents are probably the rules: https://searchfox.org/mozilla-central/rev/d80f0a570736dce76a2eb184fb65517462089e8a/layout/style/res/forms.css#1050 Differential Revision: https://phabricator.services.mozilla.com/D28299 --- components/style/stylist.rs | 49 +++++++++++++++++++++++++------------ 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/components/style/stylist.rs b/components/style/stylist.rs index ad7f9fd8585..a22d850468e 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -48,7 +48,8 @@ use selectors::visitor::SelectorVisitor; use selectors::NthIndexCache; use servo_arc::{Arc, ArcBorrow}; use smallbitvec::SmallBitVec; -use std::ops; +use smallvec::SmallVec; +use std::{mem, ops}; use std::sync::Mutex; use style_traits::viewport::ViewportConstraints; @@ -128,15 +129,28 @@ impl UserAgentCascadeDataCache { Ok(new_data) } - fn expire_unused(&mut self) { - // is_unique() returns false for static references, but we never have - // static references to UserAgentCascadeDatas. If we did, it may not - // make sense to put them in the cache in the first place. - self.entries.retain(|e| !e.is_unique()) + /// Returns all the cascade datas that are not being used (that is, that are + /// held alive just by this cache). + /// + /// We return them instead of dropping in place because some of them may + /// keep alive some other documents (like the SVG documents kept alive by + /// URL references), and thus we don't want to drop them while locking the + /// cache to not deadlock. + fn take_unused(&mut self) -> SmallVec<[Arc; 3]> { + let mut unused = SmallVec::new(); + for i in (0..self.entries.len()).rev() { + // is_unique() returns false for static references, but we never + // have static references to UserAgentCascadeDatas. If we did, it + // may not make sense to put them in the cache in the first place. + if self.entries[i].is_unique() { + unused.push(self.entries.remove(i)); + } + } + unused } - fn clear(&mut self) { - self.entries.clear(); + fn take_all(&mut self) -> Vec> { + mem::replace(&mut self.entries, Vec::new()) } #[cfg(feature = "gecko")] @@ -254,13 +268,18 @@ impl DocumentCascadeData { // First do UA sheets. { if flusher.flush_origin(Origin::UserAgent).dirty() { - let mut ua_cache = UA_CASCADE_DATA_CACHE.lock().unwrap(); let origin_sheets = flusher.origin_sheets(Origin::UserAgent); - let ua_cascade_data = - ua_cache.lookup(origin_sheets, device, quirks_mode, guards.ua_or_user)?; - ua_cache.expire_unused(); - debug!("User agent data cache size {:?}", ua_cache.len()); - self.user_agent = ua_cascade_data; + let _unused_cascade_datas = { + let mut ua_cache = UA_CASCADE_DATA_CACHE.lock().unwrap(); + self.user_agent = ua_cache.lookup( + origin_sheets, + device, + quirks_mode, + guards.ua_or_user, + )?; + debug!("User agent data cache size {:?}", ua_cache.len()); + ua_cache.take_unused() + }; } } @@ -1368,7 +1387,7 @@ impl Stylist { /// Shutdown the static data that this module stores. pub fn shutdown() { - UA_CASCADE_DATA_CACHE.lock().unwrap().clear() + let _entries = UA_CASCADE_DATA_CACHE.lock().unwrap().take_all(); } } From e5b5cd78a9cf09eb9c5e8c82f8004b1f4ed0fbf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 23 Apr 2019 16:43:15 +0000 Subject: [PATCH 09/27] style: Remove support for XBL resources. So much unsound code going away :-) Differential Revision: https://phabricator.services.mozilla.com/D28380 --- components/selectors/context.rs | 4 +- components/style/author_styles.rs | 12 +-- components/style/dom.rs | 45 ++--------- components/style/gecko/data.rs | 5 -- components/style/gecko/wrapper.rs | 76 +------------------ .../element/state_and_attributes.rs | 12 +-- components/style/rule_collector.rs | 41 +--------- components/style/sharing/mod.rs | 21 ----- components/style/stylesheet_set.rs | 2 +- components/style/stylist.rs | 12 +-- 10 files changed, 22 insertions(+), 208 deletions(-) diff --git a/components/selectors/context.rs b/components/selectors/context.rs index 3ecd4dd82d5..95caa4055d7 100644 --- a/components/selectors/context.rs +++ b/components/selectors/context.rs @@ -279,13 +279,13 @@ where /// Runs F with a given shadow host which is the root of the tree whose /// rules we're matching. #[inline] - pub fn with_shadow_host(&mut self, host: Option, f: F) -> R + pub fn with_shadow_host(&mut self, host: E, f: F) -> R where E: Element, F: FnOnce(&mut Self) -> R, { let original_host = self.current_host.take(); - self.current_host = host.map(|h| h.opaque()); + self.current_host = Some(host.opaque()); let result = f(self); self.current_host = original_host; result diff --git a/components/style/author_styles.rs b/components/style/author_styles.rs index eff64ed6c1e..58d9bda423a 100644 --- a/components/style/author_styles.rs +++ b/components/style/author_styles.rs @@ -3,7 +3,7 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ //! A set of author stylesheets and their computed representation, such as the -//! ones used for ShadowRoot and XBL. +//! ones used for ShadowRoot. use crate::context::QuirksMode; use crate::dom::TElement; @@ -17,7 +17,7 @@ use crate::stylesheets::StylesheetInDocument; use crate::stylist::CascadeData; /// A set of author stylesheets and their computed representation, such as the -/// ones used for ShadowRoot and XBL. +/// ones used for ShadowRoot. #[derive(MallocSizeOf)] pub struct AuthorStyles where @@ -28,9 +28,6 @@ where pub stylesheets: AuthorStylesheetSet, /// The actual cascade data computed from the stylesheets. pub data: CascadeData, - /// The quirks mode of the last stylesheet flush, used because XBL sucks and - /// we should really fix it, see bug 1406875. - pub quirks_mode: QuirksMode, } impl AuthorStyles @@ -43,7 +40,6 @@ where Self { stylesheets: AuthorStylesheetSet::new(), data: CascadeData::new(), - quirks_mode: QuirksMode::NoQuirks, } } @@ -65,10 +61,6 @@ where .stylesheets .flush::(/* host = */ None, /* snapshot_map = */ None); - if flusher.sheets.dirty() { - self.quirks_mode = quirks_mode; - } - // Ignore OOM. let _ = self .data diff --git a/components/style/dom.rs b/components/style/dom.rs index 967b978bad4..9a81e2ec774 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -761,7 +761,7 @@ pub trait TElement: /// Returns the anonymous content for the current element's XBL binding, /// given if any. /// - /// This is used in Gecko for XBL and shadow DOM. + /// This is used in Gecko for XBL. fn xbl_binding_anonymous_content(&self) -> Option { None } @@ -772,11 +772,6 @@ pub trait TElement: /// The shadow root which roots the subtree this element is contained in. fn containing_shadow(&self) -> Option<::ConcreteShadowRoot>; - /// XBL hack for style sharing. :( - fn has_same_xbl_proto_binding_as(&self, _other: Self) -> bool { - true - } - /// Return the element which we can use to look up rules in the selector /// maps. /// @@ -792,56 +787,34 @@ pub trait TElement: } } - /// Implements Gecko's `nsBindingManager::WalkRules`. - /// - /// Returns whether to cut off the binding inheritance, that is, whether - /// document rules should _not_ apply. - fn each_xbl_cascade_data<'a, F>(&self, _: F) -> bool - where - Self: 'a, - F: FnMut(&'a CascadeData, QuirksMode), - { - false - } - /// Executes the callback for each applicable style rule data which isn't /// the main document's data (which stores UA / author rules). /// /// The element passed to the callback is the containing shadow host for the - /// data if it comes from Shadow DOM, None if it comes from XBL. + /// data if it comes from Shadow DOM. /// /// Returns whether normal document author rules should apply. fn each_applicable_non_document_style_rule_data<'a, F>(&self, mut f: F) -> bool where Self: 'a, - F: FnMut(&'a CascadeData, QuirksMode, Option), + F: FnMut(&'a CascadeData, Self), { use rule_collector::containing_shadow_ignoring_svg_use; - let mut doc_rules_apply = !self.each_xbl_cascade_data(|data, quirks_mode| { - f(data, quirks_mode, None); - }); + let mut doc_rules_apply = self.matches_user_and_author_rules(); // Use the same rules to look for the containing host as we do for rule // collection. if let Some(shadow) = containing_shadow_ignoring_svg_use(*self) { doc_rules_apply = false; if let Some(data) = shadow.style_data() { - f( - data, - self.as_node().owner_doc().quirks_mode(), - Some(shadow.host()), - ); + f(data, shadow.host()); } } if let Some(shadow) = self.shadow_root() { if let Some(data) = shadow.style_data() { - f( - data, - self.as_node().owner_doc().quirks_mode(), - Some(shadow.host()), - ); + f(data, shadow.host()); } } @@ -850,11 +823,7 @@ pub trait TElement: // Slots can only have assigned nodes when in a shadow tree. let shadow = slot.containing_shadow().unwrap(); if let Some(data) = shadow.style_data() { - f( - data, - self.as_node().owner_doc().quirks_mode(), - Some(shadow.host()), - ); + f(data, shadow.host()); } current = slot.assigned_slot(); } diff --git a/components/style/gecko/data.rs b/components/style/gecko/data.rs index ead7d88502b..0ce43bca46d 100644 --- a/components/style/gecko/data.rs +++ b/components/style/gecko/data.rs @@ -145,11 +145,6 @@ impl PerDocumentStyleData { /// Create a `PerDocumentStyleData`. pub fn new(document: *const structs::Document) -> Self { let device = Device::new(document); - - // FIXME(emilio, tlin): How is this supposed to work with XBL? This is - // right now not always honored, see bug 1405543... - // - // Should we just force XBL Stylists to be NoQuirks? let quirks_mode = device.document().mCompatMode; PerDocumentStyleData(AtomicRefCell::new(PerDocumentStyleDataImpl { diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 0fa354fb139..c295b795fdc 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -172,15 +172,7 @@ impl<'lr> TShadowRoot for GeckoShadowRoot<'lr> { Self: 'a, { let author_styles = unsafe { self.0.mServoStyles.mPtr.as_ref()? }; - let author_styles = AuthorStyles::::from_ffi(author_styles); - - debug_assert!( - author_styles.quirks_mode == self.as_node().owner_doc().quirks_mode() || - author_styles.stylesheets.is_empty() || - author_styles.stylesheets.dirty() - ); - Some(&author_styles.data) } @@ -536,11 +528,6 @@ impl<'lb> GeckoXBLBinding<'lb> { self.0.mContent.raw::() } - #[inline] - fn inherits_style(&self) -> bool { - unsafe { bindings::Gecko_XBLBinding_InheritsStyle(self.0) } - } - // This duplicates the logic in Gecko's // nsBindingManager::GetBindingWithContent. fn binding_with_content(&self) -> Option { @@ -552,22 +539,6 @@ impl<'lb> GeckoXBLBinding<'lb> { binding = binding.base_binding()?; } } - - fn each_xbl_cascade_data(&self, f: &mut F) - where - F: FnMut(&'lb CascadeData, QuirksMode), - { - if let Some(base) = self.base_binding() { - base.each_xbl_cascade_data(f); - } - - let data = unsafe { bindings::Gecko_XBLBinding_GetRawServoStyles(self.0).as_ref() }; - - if let Some(data) = data { - let data: &'lb _ = AuthorStyles::::from_ffi(data); - f(&data.data, data.quirks_mode) - } - } } /// A simple wrapper over a non-null Gecko `Element` pointer. @@ -1250,14 +1221,6 @@ impl<'le> TElement for GeckoElement<'le> { } } - fn has_same_xbl_proto_binding_as(&self, other: Self) -> bool { - match (self.xbl_binding(), other.xbl_binding()) { - (None, None) => true, - (Some(a), Some(b)) => a.0.mPrototypeBinding == b.0.mPrototypeBinding, - _ => false, - } - } - fn each_anonymous_content_child(&self, mut f: F) where F: FnMut(Self), @@ -1436,7 +1399,7 @@ impl<'le> TElement for GeckoElement<'le> { #[inline] fn matches_user_and_author_rules(&self) -> bool { - !self.is_in_native_anonymous_subtree() + !self.rule_hash_target().is_in_native_anonymous_subtree() } #[inline] @@ -1599,43 +1562,6 @@ impl<'le> TElement for GeckoElement<'le> { self.may_have_animations() && unsafe { Gecko_ElementHasCSSTransitions(self.0) } } - fn each_xbl_cascade_data<'a, F>(&self, mut f: F) -> bool - where - 'le: 'a, - F: FnMut(&'a CascadeData, QuirksMode), - { - // Walk the binding scope chain, starting with the binding attached to - // our content, up till we run out of scopes or we get cut off. - // - // If we are a NAC pseudo-element, we want to get rules from our - // rule_hash_target, that is, our originating element. - let mut current = Some(self.rule_hash_target()); - while let Some(element) = current { - if let Some(binding) = element.xbl_binding() { - binding.each_xbl_cascade_data(&mut f); - - // If we're not looking at our original element, allow the - // binding to cut off style inheritance. - if element != *self && !binding.inherits_style() { - // Go no further; we're not inheriting style from - // anything above here. - break; - } - } - - if element.is_root_of_native_anonymous_subtree() { - // Deliberately cut off style inheritance here. - break; - } - - current = element.xbl_binding_parent(); - } - - // If current has something, this means we cut off inheritance at some - // point in the loop. - current.is_some() - } - fn xbl_binding_anonymous_content(&self) -> Option> { self.xbl_binding_with_content() .map(|b| unsafe { GeckoNode::from_content(&*b.anon_content()) }) diff --git a/components/style/invalidation/element/state_and_attributes.rs b/components/style/invalidation/element/state_and_attributes.rs index e4555bb9a70..a49bb6306c5 100644 --- a/components/style/invalidation/element/state_and_attributes.rs +++ b/components/style/invalidation/element/state_and_attributes.rs @@ -224,8 +224,8 @@ where let mut shadow_rule_datas = SmallVec::<[_; 3]>::new(); let matches_document_author_rules = - element.each_applicable_non_document_style_rule_data(|data, quirks_mode, host| { - shadow_rule_datas.push((data, quirks_mode, host.map(|h| h.opaque()))) + element.each_applicable_non_document_style_rule_data(|data, host| { + shadow_rule_datas.push((data, host.opaque())) }); let invalidated_self = { @@ -258,12 +258,8 @@ where } } - for &(ref data, quirks_mode, ref host) in &shadow_rule_datas { - // FIXME(emilio): Replace with assert / remove when we figure - // out what to do with the quirks mode mismatches - // (that is, when bug 1406875 is properly fixed). - collector.matching_context.set_quirks_mode(quirks_mode); - collector.matching_context.current_host = host.clone(); + for &(ref data, ref host) in &shadow_rule_datas { + collector.matching_context.current_host = Some(host.clone()); collector.collect_dependencies_in_invalidation_map(data.invalidation_map()); } diff --git a/components/style/rule_collector.rs b/components/style/rule_collector.rs index ba602ddca83..e87a86c5e47 100644 --- a/components/style/rule_collector.rs +++ b/components/style/rule_collector.rs @@ -98,7 +98,7 @@ where flags_setter: &'a mut F, ) -> Self { let rule_hash_target = element.rule_hash_target(); - let matches_user_and_author_rules = rule_hash_target.matches_user_and_author_rules(); + let matches_user_and_author_rules = element.matches_user_and_author_rules(); // Gecko definitely has pseudo-elements with style attributes, like // ::-moz-color-swatch. @@ -198,7 +198,7 @@ where let rules = &mut self.rules; let flags_setter = &mut self.flags_setter; let shadow_cascade_order = self.shadow_cascade_order; - self.context.with_shadow_host(Some(shadow_host), |context| { + self.context.with_shadow_host(shadow_host, |context| { map.get_all_matching_rules( element, rule_hash_target, @@ -303,42 +303,6 @@ where self.collect_stylist_rules(Origin::Author); } - fn collect_xbl_rules(&mut self) { - let element = self.element; - let cut_xbl_binding_inheritance = - element.each_xbl_cascade_data(|cascade_data, quirks_mode| { - let map = match cascade_data.normal_rules(self.pseudo_element) { - Some(m) => m, - None => return, - }; - - // NOTE(emilio): This is needed because the XBL stylist may - // think it has a different quirks mode than the document. - let mut matching_context = MatchingContext::new( - self.context.matching_mode(), - self.context.bloom_filter, - self.context.nth_index_cache.as_mut().map(|s| &mut **s), - quirks_mode, - ); - matching_context.pseudo_element_matching_fn = - self.context.pseudo_element_matching_fn; - - // SameTreeAuthorNormal instead of InnerShadowNormal to - // preserve behavior, though that's kinda fishy... - map.get_all_matching_rules( - self.element, - self.rule_hash_target, - self.rules, - &mut matching_context, - self.flags_setter, - CascadeLevel::SameTreeAuthorNormal, - self.shadow_cascade_order, - ); - }); - - self.matches_document_author_rules &= !cut_xbl_binding_inheritance; - } - fn collect_style_attribute_and_animation_rules(&mut self) { if let Some(sa) = self.style_attribute { self.rules @@ -396,7 +360,6 @@ where self.collect_host_rules(); self.collect_slotted_rules(); self.collect_normal_rules_from_containing_shadow_tree(); - self.collect_xbl_rules(); self.collect_document_author_rules(); self.collect_style_attribute_and_animation_rules(); } diff --git a/components/style/sharing/mod.rs b/components/style/sharing/mod.rs index 0675359ed77..3ce2a717dc8 100644 --- a/components/style/sharing/mod.rs +++ b/components/style/sharing/mod.rs @@ -727,27 +727,6 @@ impl StyleSharingCache { return None; } - // Note that in theory we shouldn't need this XBL check. However, XBL is - // absolutely broken in all sorts of ways. - // - // A style change that changes which XBL binding applies to an element - // arrives there, with the element still having the old prototype - // binding attached. And thus we try to match revalidation selectors - // with the old XBL binding, because we can't look at the new ones of - // course. And that causes us to revalidate with the wrong selectors and - // hit assertions. - // - // Other than this, we don't need anything else like the containing XBL - // binding parent or what not, since two elements with different XBL - // bindings will necessarily end up with different style. - if !target - .element - .has_same_xbl_proto_binding_as(candidate.element) - { - trace!("Miss: Different proto bindings"); - return None; - } - // If the elements are not assigned to the same slot they could match // different ::slotted() rules in the slot scope. // diff --git a/components/style/stylesheet_set.rs b/components/style/stylesheet_set.rs index 5e72f7a219f..d8c21d5494c 100644 --- a/components/style/stylesheet_set.rs +++ b/components/style/stylesheet_set.rs @@ -546,7 +546,7 @@ where } } -/// The set of stylesheets effective for a given XBL binding or Shadow Root. +/// The set of stylesheets effective for a given Shadow Root. #[derive(MallocSizeOf)] pub struct AuthorStylesheetSet where diff --git a/components/style/stylist.rs b/components/style/stylist.rs index a22d850468e..08f3cee0464 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -636,7 +636,7 @@ impl Stylist { let mut maybe = false; let doc_author_rules_apply = - element.each_applicable_non_document_style_rule_data(|data, _, _| { + element.each_applicable_non_document_style_rule_data(|data, _| { maybe = maybe || f(&*data); }); @@ -1072,12 +1072,6 @@ impl Stylist { /// Returns whether, given a media feature change, any previously-applicable /// style has become non-applicable, or vice-versa for each origin, using /// `device`. - /// - /// Passing `device` is needed because this is used for XBL in Gecko, which - /// can be stale in various ways, so we need to pass the device of the - /// document itself, which is what is kept up-to-date. - /// - /// Arguably XBL should use something more lightweight than a Stylist. pub fn media_features_change_changed_style( &self, guards: &StylesheetGuards, @@ -1261,11 +1255,11 @@ impl Stylist { let mut results = SmallBitVec::new(); let matches_document_rules = - element.each_applicable_non_document_style_rule_data(|data, quirks_mode, host| { + element.each_applicable_non_document_style_rule_data(|data, host| { matching_context.with_shadow_host(host, |matching_context| { data.selectors_for_cache_revalidation.lookup( element, - quirks_mode, + self.quirks_mode, |selector_and_hashes| { results.push(matches_selector( &selector_and_hashes.selector, From 07c0c1e53f8679e7fc890b1fba69cf9c065594e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 23 Apr 2019 13:13:11 +0000 Subject: [PATCH 10/27] style: Allow CSS wide-keywords in custom property fallback. Differential Revision: https://phabricator.services.mozilla.com/D28349 --- .../style/properties/properties.mako.rs | 136 ++++++++++-------- 1 file changed, 77 insertions(+), 59 deletions(-) diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 9943c3509e6..e7f9c72413e 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -1085,6 +1085,8 @@ impl LonghandId { } } + // TODO(emilio): Should we use a function table like CASCADE_PROPERTY does + // to avoid blowing up code-size here? fn parse_value<'i, 't>( &self, context: &ParserContext, @@ -1532,64 +1534,7 @@ impl UnparsedValue { quirks_mode: QuirksMode, environment: &::custom_properties::CssEnvironment, ) -> PropertyDeclaration { - crate::custom_properties::substitute( - &self.css, - self.first_token_type, - custom_properties, - environment, - ).ok().and_then(|css| { - // As of this writing, only the base URL is used for property - // values. - // - // NOTE(emilio): we intentionally pase `None` as the rule type here. - // If something starts depending on it, it's probably a bug, since - // it'd change how values are parsed depending on whether we're in a - // @keyframes rule or not, for example... So think twice about - // whether you want to do this! - // - // FIXME(emilio): ParsingMode is slightly fishy... - let context = ParserContext::new( - Origin::Author, - &self.url_data, - None, - ParsingMode::DEFAULT, - quirks_mode, - None, - None, - ); - - let mut input = ParserInput::new(&css); - Parser::new(&mut input).parse_entirely(|input| { - match self.from_shorthand { - None => longhand_id.parse_value(&context, input), - Some(ShorthandId::All) => { - // No need to parse the 'all' shorthand as anything other than a CSS-wide - // keyword, after variable substitution. - Err(input.new_custom_error(SelectorParseErrorKind::UnexpectedIdent("all".into()))) - } - % for shorthand in data.shorthands_except_all(): - Some(ShorthandId::${shorthand.camel_case}) => { - shorthands::${shorthand.ident}::parse_value(&context, input) - .map(|longhands| { - match longhand_id { - % for property in shorthand.sub_properties: - LonghandId::${property.camel_case} => { - PropertyDeclaration::${property.camel_case}( - longhands.${property.ident} - ) - } - % endfor - _ => unreachable!() - } - }) - } - % endfor - } - }) - .ok() - }) - .unwrap_or_else(|| { - // Invalid at computed-value time. + let invalid_at_computed_value_time = || { let keyword = if longhand_id.inherited() { CSSWideKeyword::Inherit } else { @@ -1599,7 +1544,80 @@ impl UnparsedValue { id: longhand_id, keyword, }) - }) + }; + + let css = match crate::custom_properties::substitute( + &self.css, + self.first_token_type, + custom_properties, + environment, + ) { + Ok(css) => css, + Err(..) => return invalid_at_computed_value_time(), + }; + + // As of this writing, only the base URL is used for property + // values. + // + // NOTE(emilio): we intentionally pase `None` as the rule type here. + // If something starts depending on it, it's probably a bug, since + // it'd change how values are parsed depending on whether we're in a + // @keyframes rule or not, for example... So think twice about + // whether you want to do this! + // + // FIXME(emilio): ParsingMode is slightly fishy... + let context = ParserContext::new( + Origin::Author, + &self.url_data, + None, + ParsingMode::DEFAULT, + quirks_mode, + None, + None, + ); + + let mut input = ParserInput::new(&css); + let mut input = Parser::new(&mut input); + input.skip_whitespace(); // Unnecessary for correctness, but may help try() rewind less. + if let Ok(keyword) = input.try(CSSWideKeyword::parse) { + return PropertyDeclaration::CSSWideKeyword(WideKeywordDeclaration { + id: longhand_id, + keyword, + }); + } + + let declaration = input.parse_entirely(|input| { + match self.from_shorthand { + None => longhand_id.parse_value(&context, input), + Some(ShorthandId::All) => { + // No need to parse the 'all' shorthand as anything other + // than a CSS-wide keyword, after variable substitution. + Err(input.new_custom_error(SelectorParseErrorKind::UnexpectedIdent("all".into()))) + } + % for shorthand in data.shorthands_except_all(): + Some(ShorthandId::${shorthand.camel_case}) => { + shorthands::${shorthand.ident}::parse_value(&context, input) + .map(|longhands| { + match longhand_id { + % for property in shorthand.sub_properties: + LonghandId::${property.camel_case} => { + PropertyDeclaration::${property.camel_case}( + longhands.${property.ident} + ) + } + % endfor + _ => unreachable!() + } + }) + } + % endfor + } + }); + + match declaration { + Ok(decl) => decl, + Err(..) => invalid_at_computed_value_time(), + } } } From 9636ef7be7981eb2541510186822e1385f362f8c Mon Sep 17 00:00:00 2001 From: Brian Birtles Date: Fri, 26 Apr 2019 00:47:40 +0000 Subject: [PATCH 11/27] style: Allow animating the 'all' property from Web Animations. Differential Revision: https://phabricator.services.mozilla.com/D28763 --- components/style/properties/data.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/components/style/properties/data.py b/components/style/properties/data.py index b20a8deaaea..7a96402c4a1 100644 --- a/components/style/properties/data.py +++ b/components/style/properties/data.py @@ -400,8 +400,6 @@ class Shorthand(object): and allowed_in_keyframe_block != "False" def get_animatable(self): - if self.ident == "all": - return False for sub in self.sub_properties: if sub.animatable: return True From bec81dc9c2da399ece93405b38dc8593f6cf45df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 29 Apr 2019 08:04:22 +0000 Subject: [PATCH 12/27] style: Fix :-moz-svg-use-shadow-tree-root pseudo-class. Turns out removing the pseudo-class and such ends up not being quite as trivial as I initially thought, or possible at all, since the fact that the is a is observable via selectors, added a test for that. Differential Revision: https://phabricator.services.mozilla.com/D29131 --- components/style/gecko/wrapper.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index c295b795fdc..079dde2f513 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -865,10 +865,11 @@ impl<'le> GeckoElement<'le> { if !self.as_node().is_in_shadow_tree() { return false; } - match self.containing_shadow_host() { - Some(e) => e.is_svg_element() && e.local_name() == &*local_name!("use"), - None => false, + if !self.parent_node_is_shadow_root() { + return false; } + let host = self.containing_shadow_host().unwrap(); + host.is_svg_element() && host.local_name() == &*local_name!("use") } fn css_transitions_info(&self) -> FxHashMap> { From e40500622ee0712dda741cf560f374e532d535f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 30 Apr 2019 19:48:27 +0000 Subject: [PATCH 13/27] style: Add a pref for a simpler -moz- gradient parsing. This won't reintroduce any of the regressions that were triggered by our previous attempts to turn off -moz prefixed gradients, and lets us massively simplify the gradient code, if it sticks. Differential Revision: https://phabricator.services.mozilla.com/D29346 --- components/style/values/specified/image.rs | 52 +++++++++++----------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/components/style/values/specified/image.rs b/components/style/values/specified/image.rs index 572db0ca1eb..84571a43ad5 100644 --- a/components/style/values/specified/image.rs +++ b/components/style/values/specified/image.rs @@ -8,6 +8,8 @@ //! [image]: https://drafts.csswg.org/css-images/#image-values use crate::custom_properties::SpecifiedValue; +#[cfg(feature = "gecko")] +use crate::gecko_bindings::structs; use crate::parser::{Parse, ParserContext}; #[cfg(feature = "gecko")] use crate::values::computed::{Context, Position as ComputedPosition, ToComputedValue}; @@ -266,16 +268,6 @@ impl Parse for Gradient { }, }; - #[cfg(feature = "gecko")] - { - use crate::gecko_bindings::structs; - if compat_mode == CompatMode::Moz && - !unsafe { structs::StaticPrefs_sVarCache_layout_css_prefixes_gradients } - { - return Err(input.new_custom_error(StyleParseErrorKind::UnexpectedFunction(func))); - } - } - let (kind, items) = input.parse_nested_block(|i| { let shape = match shape { Shape::Linear => GradientKind::parse_linear(context, i, &mut compat_mode)?, @@ -548,6 +540,16 @@ impl Gradient { } } +#[inline] +fn simple_moz_gradient() -> bool { + #[cfg(feature = "gecko")] + unsafe { + return structs::StaticPrefs_sVarCache_layout_css_simple_moz_gradient_enabled; + } + #[cfg(not(feature = "gecko"))] + return false; +} + impl GradientKind { /// Parses a linear gradient. /// CompatMode can change during `-moz-` prefixed gradient parsing if it come across a `to` keyword. @@ -583,16 +585,6 @@ impl GradientKind { }); (shape, position.ok(), None, None) }, - CompatMode::WebKit => { - let position = input.try(|i| Position::parse(context, i)); - let shape = input.try(|i| { - if position.is_ok() { - i.expect_comma()?; - } - EndingShape::parse(context, i, *compat_mode) - }); - (shape, position.ok(), None, None) - }, // The syntax of `-moz-` prefixed radial gradient is: // -moz-radial-gradient( // [ [ || ]? [ ellipse | [ | ]{2} ] , | @@ -603,7 +595,7 @@ impl GradientKind { // where = closest-corner | closest-side | farthest-corner | farthest-side | // cover | contain // and = [ | ]? - CompatMode::Moz => { + CompatMode::Moz if !simple_moz_gradient() => { let mut position = input.try(|i| LegacyPosition::parse(context, i)); let angle = input.try(|i| Angle::parse(context, i)).ok(); if position.is_err() { @@ -619,6 +611,16 @@ impl GradientKind { (shape, None, angle, position.ok()) }, + _ => { + let position = input.try(|i| Position::parse(context, i)); + let shape = input.try(|i| { + if position.is_ok() { + i.expect_comma()?; + } + EndingShape::parse(context, i, *compat_mode) + }); + (shape, position.ok(), None, None) + }, }; if shape.is_ok() || position.is_some() || angle.is_some() || moz_position.is_some() { @@ -631,7 +633,7 @@ impl GradientKind { #[cfg(feature = "gecko")] { - if *compat_mode == CompatMode::Moz { + if *compat_mode == CompatMode::Moz && !simple_moz_gradient() { // If this form can be represented in Modern mode, then convert the compat_mode to Modern. if angle.is_none() { *compat_mode = CompatMode::Modern; @@ -751,7 +753,7 @@ impl LineDirection { input: &mut Parser<'i, 't>, compat_mode: &mut CompatMode, ) -> Result> { - let mut _angle = if *compat_mode == CompatMode::Moz { + let mut _angle = if *compat_mode == CompatMode::Moz && !simple_moz_gradient() { input.try(|i| Angle::parse(context, i)).ok() } else { // Gradients allow unitless zero angles as an exception, see: @@ -784,7 +786,7 @@ impl LineDirection { #[cfg(feature = "gecko")] { // `-moz-` prefixed linear gradient can be both Angle and Position. - if *compat_mode == CompatMode::Moz { + if *compat_mode == CompatMode::Moz && !simple_moz_gradient(){ let position = i.try(|i| LegacyPosition::parse(context, i)).ok(); if _angle.is_none() { _angle = i.try(|i| Angle::parse(context, i)).ok(); @@ -874,7 +876,7 @@ impl EndingShape { } // -moz- prefixed radial gradient doesn't allow EndingShape's Length or LengthPercentage // to come before shape keyword. Otherwise it conflicts with . - if compat_mode != CompatMode::Moz { + if compat_mode != CompatMode::Moz || simple_moz_gradient() { if let Ok(length) = input.try(|i| Length::parse(context, i)) { if let Ok(y) = input.try(|i| LengthPercentage::parse(context, i)) { if compat_mode == CompatMode::Modern { From 8123007717a60e57ee13d98ae5df23e42d47aa53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 30 Apr 2019 20:37:54 +0000 Subject: [PATCH 14/27] style: Use rust types for gradient stops. This doesn't clean up all that much, yet, but it's a step in the right direction. Differential Revision: https://phabricator.services.mozilla.com/D29168 --- components/style/gecko/conversions.rs | 45 +++------------------- components/style/values/computed/image.rs | 2 +- components/style/values/generics/image.rs | 30 +++++++++++++-- components/style/values/specified/image.rs | 38 +++++++++--------- 4 files changed, 52 insertions(+), 63 deletions(-) diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs index 44dbc1bb04b..7b73f00e73e 100644 --- a/components/style/gecko/conversions.rs +++ b/components/style/gecko/conversions.rs @@ -24,7 +24,7 @@ use crate::values::computed::{Integer, LengthPercentage}; use crate::values::computed::{Length, Percentage, TextAlign}; use crate::values::generics::box_::VerticalAlign; use crate::values::generics::grid::{TrackListValue, TrackSize}; -use crate::values::generics::image::{CompatMode, GradientItem, Image as GenericImage}; +use crate::values::generics::image::{CompatMode, Image as GenericImage}; use crate::values::generics::rect::Rect; use crate::Zero; use app_units::Au; @@ -154,7 +154,6 @@ impl nsStyleImage { // FIXME(emilio): This is really complex, we should use cbindgen for this. fn set_gradient(&mut self, gradient: Gradient) { - use self::structs::nsStyleCoord; use self::structs::NS_STYLE_GRADIENT_SIZE_CLOSEST_CORNER as CLOSEST_CORNER; use self::structs::NS_STYLE_GRADIENT_SIZE_CLOSEST_SIDE as CLOSEST_SIDE; use self::structs::NS_STYLE_GRADIENT_SIZE_FARTHEST_CORNER as FARTHEST_CORNER; @@ -329,26 +328,9 @@ impl nsStyleImage { }, }; - for (index, item) in gradient.items.iter().enumerate() { - // NB: stops are guaranteed to be none in the gecko side by - // default. - + for (index, item) in gradient.items.into_iter().enumerate() { let gecko_stop = unsafe { &mut (*gecko_gradient).mStops[index] }; - let mut coord = nsStyleCoord::null(); - - match *item { - GradientItem::ColorStop(ref stop) => { - gecko_stop.mColor = stop.color.into(); - gecko_stop.mIsInterpolationHint = false; - coord.set(stop.position); - }, - GradientItem::InterpolationHint(hint) => { - gecko_stop.mIsInterpolationHint = true; - coord.set(Some(hint)); - }, - } - - gecko_stop.mLocation.move_from(coord); + *gecko_stop = item; } unsafe { @@ -419,7 +401,7 @@ impl nsStyleImage { use self::structs::NS_STYLE_GRADIENT_SIZE_FARTHEST_CORNER as FARTHEST_CORNER; use self::structs::NS_STYLE_GRADIENT_SIZE_FARTHEST_SIDE as FARTHEST_SIDE; use crate::values::computed::position::Position; - use crate::values::generics::image::{Circle, ColorStop, Ellipse}; + use crate::values::generics::image::{Circle, Ellipse}; use crate::values::generics::image::{EndingShape, GradientKind, ShapeExtent}; let gecko_gradient = bindings::Gecko_GetGradientImageValue(self) @@ -531,24 +513,7 @@ impl nsStyleImage { }, }; - let items = gecko_gradient - .mStops - .iter() - .map(|ref stop| { - if stop.mIsInterpolationHint { - GradientItem::InterpolationHint( - LengthPercentage::from_gecko_style_coord(&stop.mLocation) - .expect("mLocation could not convert to LengthPercentage"), - ) - } else { - GradientItem::ColorStop(ColorStop { - color: stop.mColor.into(), - position: LengthPercentage::from_gecko_style_coord(&stop.mLocation), - }) - } - }) - .collect(); - + let items = gecko_gradient.mStops.iter().cloned().collect(); let compat_mode = if gecko_gradient.mMozLegacySyntax { CompatMode::Moz } else if gecko_gradient.mLegacySyntax { diff --git a/components/style/values/computed/image.rs b/components/style/values/computed/image.rs index 4070e51815a..70ecad6a04e 100644 --- a/components/style/values/computed/image.rs +++ b/components/style/values/computed/image.rs @@ -55,7 +55,7 @@ pub enum LineDirection { pub type EndingShape = generic::EndingShape; /// A computed gradient item. -pub type GradientItem = generic::GradientItem; +pub type GradientItem = generic::GenericGradientItem; /// A computed color stop. pub type ColorStop = generic::ColorStop; diff --git a/components/style/values/generics/image.rs b/components/style/values/generics/image.rs index 71d6a1b3849..1442ce604a2 100644 --- a/components/style/values/generics/image.rs +++ b/components/style/values/generics/image.rs @@ -135,13 +135,23 @@ pub enum ShapeExtent { #[derive( Clone, Copy, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToCss, ToResolvedValue, ToShmem, )] -pub enum GradientItem { - /// A color stop. - ColorStop(ColorStop), +#[repr(C, u8)] +pub enum GenericGradientItem { + /// A simple color stop, without position. + SimpleColorStop(Color), + /// A complex color stop, with a position. + ComplexColorStop { + /// The color for the stop. + color: Color, + /// The position for the stop. + position: LengthPercentage, + }, /// An interpolation hint. InterpolationHint(LengthPercentage), } +pub use self::GenericGradientItem as GradientItem; + /// A color stop. /// #[derive( @@ -154,6 +164,20 @@ pub struct ColorStop { pub position: Option, } +impl ColorStop { + /// Convert the color stop into an appropriate `GradientItem`. + #[inline] + pub fn into_item(self) -> GradientItem { + match self.position { + Some(position) => GradientItem::ComplexColorStop { + color: self.color, + position, + }, + None => GradientItem::SimpleColorStop(self.color), + } + } +} + /// Specified values for a paint worklet. /// #[cfg_attr(feature = "servo", derive(MallocSizeOf))] diff --git a/components/style/values/specified/image.rs b/components/style/values/specified/image.rs index 84571a43ad5..a387d8e07c2 100644 --- a/components/style/values/specified/image.rs +++ b/components/style/values/specified/image.rs @@ -484,24 +484,24 @@ impl Gradient { if reverse_stops { p.reverse(); } - Ok(generic::GradientItem::ColorStop(generic::ColorStop { - color: color, - position: Some(p.into()), - })) + Ok(generic::GradientItem::ComplexColorStop { + color, + position: p.into(), + }) }) }) .unwrap_or(vec![]); if items.is_empty() { items = vec![ - generic::GradientItem::ColorStop(generic::ColorStop { + generic::GradientItem::ComplexColorStop { color: Color::transparent().into(), - position: Some(Percentage::zero().into()), - }), - generic::GradientItem::ColorStop(generic::ColorStop { + position: Percentage::zero().into(), + }, + generic::GradientItem::ComplexColorStop { color: Color::transparent().into(), - position: Some(Percentage::hundred().into()), - }), + position: Percentage::hundred().into(), + }, ]; } else if items.len() == 1 { let first = items[0].clone(); @@ -510,12 +510,12 @@ impl Gradient { items.sort_by(|a, b| { match (a, b) { ( - &generic::GradientItem::ColorStop(ref a), - &generic::GradientItem::ColorStop(ref b), - ) => match (&a.position, &b.position) { + &generic::GradientItem::ComplexColorStop { position: ref a_position, .. }, + &generic::GradientItem::ComplexColorStop { position: ref b_position, .. }, + ) => match (a_position, b_position) { ( - &Some(LengthPercentage::Percentage(a)), - &Some(LengthPercentage::Percentage(b)), + &LengthPercentage::Percentage(a), + &LengthPercentage::Percentage(b), ) => { return a.0.partial_cmp(&b.0).unwrap_or(Ordering::Equal); }, @@ -960,13 +960,13 @@ impl GradientItem { if let Ok(multi_position) = input.try(|i| LengthPercentage::parse(context, i)) { let stop_color = stop.color.clone(); - items.push(generic::GradientItem::ColorStop(stop)); - items.push(generic::GradientItem::ColorStop(ColorStop { + items.push(stop.into_item()); + items.push(ColorStop { color: stop_color, position: Some(multi_position), - })); + }.into_item()); } else { - items.push(generic::GradientItem::ColorStop(stop)); + items.push(stop.into_item()); } seen_stop = true; From c990c9623d8f204594e9fa12777fd595e1559bbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 1 May 2019 13:08:34 +0000 Subject: [PATCH 15/27] style: Use rust types for vertical-align. The previous commit removed the dependence on the discriminant value, so we don't need to keep discriminants different from text-align anymore. Differential Revision: https://phabricator.services.mozilla.com/D29361 --- components/style/gecko/conversions.rs | 21 -------- components/style/properties/gecko.mako.rs | 45 +---------------- components/style/values/generics/box.rs | 59 +++++++++++++++-------- components/style/values/specified/box.rs | 17 +------ components/style/values/specified/text.rs | 3 +- 5 files changed, 43 insertions(+), 102 deletions(-) diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs index 7b73f00e73e..4cd5722bdb0 100644 --- a/components/style/gecko/conversions.rs +++ b/components/style/gecko/conversions.rs @@ -22,7 +22,6 @@ use crate::values::computed::url::ComputedImageUrl; use crate::values::computed::{Angle, Gradient, Image}; use crate::values::computed::{Integer, LengthPercentage}; use crate::values::computed::{Length, Percentage, TextAlign}; -use crate::values::generics::box_::VerticalAlign; use crate::values::generics::grid::{TrackListValue, TrackSize}; use crate::values::generics::image::{CompatMode, Image as GenericImage}; use crate::values::generics::rect::Rect; @@ -875,26 +874,6 @@ where } } -impl VerticalAlign { - /// Converts an enumerated value coming from Gecko to a `VerticalAlign`. - pub fn from_gecko_keyword(value: u32) -> Self { - match value { - structs::NS_STYLE_VERTICAL_ALIGN_BASELINE => VerticalAlign::Baseline, - structs::NS_STYLE_VERTICAL_ALIGN_SUB => VerticalAlign::Sub, - structs::NS_STYLE_VERTICAL_ALIGN_SUPER => VerticalAlign::Super, - structs::NS_STYLE_VERTICAL_ALIGN_TOP => VerticalAlign::Top, - structs::NS_STYLE_VERTICAL_ALIGN_TEXT_TOP => VerticalAlign::TextTop, - structs::NS_STYLE_VERTICAL_ALIGN_MIDDLE => VerticalAlign::Middle, - structs::NS_STYLE_VERTICAL_ALIGN_BOTTOM => VerticalAlign::Bottom, - structs::NS_STYLE_VERTICAL_ALIGN_TEXT_BOTTOM => VerticalAlign::TextBottom, - structs::NS_STYLE_VERTICAL_ALIGN_MIDDLE_WITH_BASELINE => { - VerticalAlign::MozMiddleWithBaseline - }, - _ => panic!("unexpected enumerated value for vertical-align"), - } - } -} - impl TextAlign { /// Obtain a specified value from a Gecko keyword value /// diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 2dde6af2a57..b73bb073fa6 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -38,7 +38,7 @@ use crate::gecko_bindings::bindings::{Gecko_ResetFilters, Gecko_CopyFiltersFrom} use crate::gecko_bindings::structs; use crate::gecko_bindings::structs::nsCSSPropertyID; use crate::gecko_bindings::structs::mozilla::PseudoStyleType; -use crate::gecko_bindings::sugar::ns_style_coord::{CoordDataValue, CoordData, CoordDataMut}; +use crate::gecko_bindings::sugar::ns_style_coord::{CoordDataValue, CoordDataMut}; use crate::gecko_bindings::sugar::refptr::RefPtr; use crate::gecko::values::GeckoStyleCoordConvertible; use crate::gecko::values::round_border_to_device_pixels; @@ -2505,7 +2505,7 @@ fn static_assert() { } -<% skip_box_longhands= """display vertical-align +<% skip_box_longhands= """display animation-name animation-delay animation-duration animation-direction animation-fill-mode animation-play-state animation-iteration-count animation-timing-function @@ -2561,47 +2561,6 @@ fn static_assert() { ) %> ${impl_keyword('clear', 'mBreakType', clear_keyword)} - pub fn set_vertical_align(&mut self, v: longhands::vertical_align::computed_value::T) { - use crate::values::generics::box_::VerticalAlign; - let value = match v { - VerticalAlign::Baseline => structs::NS_STYLE_VERTICAL_ALIGN_BASELINE, - VerticalAlign::Sub => structs::NS_STYLE_VERTICAL_ALIGN_SUB, - VerticalAlign::Super => structs::NS_STYLE_VERTICAL_ALIGN_SUPER, - VerticalAlign::Top => structs::NS_STYLE_VERTICAL_ALIGN_TOP, - VerticalAlign::TextTop => structs::NS_STYLE_VERTICAL_ALIGN_TEXT_TOP, - VerticalAlign::Middle => structs::NS_STYLE_VERTICAL_ALIGN_MIDDLE, - VerticalAlign::Bottom => structs::NS_STYLE_VERTICAL_ALIGN_BOTTOM, - VerticalAlign::TextBottom => structs::NS_STYLE_VERTICAL_ALIGN_TEXT_BOTTOM, - VerticalAlign::MozMiddleWithBaseline => { - structs::NS_STYLE_VERTICAL_ALIGN_MIDDLE_WITH_BASELINE - }, - VerticalAlign::Length(length) => { - self.gecko.mVerticalAlign.set(length); - return; - }, - }; - self.gecko.mVerticalAlign.set_value(CoordDataValue::Enumerated(value)); - } - - pub fn clone_vertical_align(&self) -> longhands::vertical_align::computed_value::T { - use crate::values::computed::LengthPercentage; - use crate::values::generics::box_::VerticalAlign; - - let gecko = &self.gecko.mVerticalAlign; - match gecko.as_value() { - CoordDataValue::Enumerated(value) => VerticalAlign::from_gecko_keyword(value), - _ => { - VerticalAlign::Length( - LengthPercentage::from_gecko_style_coord(gecko).expect( - "expected for vertical-align", - ), - ) - }, - } - } - - <%call expr="impl_coord_copy('vertical_align', 'mVerticalAlign')"> - ${impl_style_coord("scroll_snap_points_x", "mScrollSnapPointsX")} ${impl_style_coord("scroll_snap_points_y", "mScrollSnapPointsY")} diff --git a/components/style/values/generics/box.rs b/components/style/values/generics/box.rs index 3e8f959f456..5d6e8e25031 100644 --- a/components/style/values/generics/box.rs +++ b/components/style/values/generics/box.rs @@ -6,6 +6,37 @@ use crate::values::animated::ToAnimatedZero; +#[derive( + Animate, + Clone, + ComputeSquaredDistance, + Copy, + Debug, + FromPrimitive, + MallocSizeOf, + Parse, + PartialEq, + SpecifiedValueInfo, + ToComputedValue, + ToCss, + ToResolvedValue, + ToShmem, +)] +#[repr(u8)] +#[allow(missing_docs)] +pub enum VerticalAlignKeyword { + Baseline, + Sub, + Super, + Top, + TextTop, + Middle, + Bottom, + TextBottom, + #[cfg(feature = "gecko")] + MozMiddleWithBaseline, +} + /// A generic value for the `vertical-align` property. #[derive( Animate, @@ -21,35 +52,21 @@ use crate::values::animated::ToAnimatedZero; ToResolvedValue, ToShmem, )] -pub enum VerticalAlign { - /// `baseline` - Baseline, - /// `sub` - Sub, - /// `super` - Super, - /// `top` - Top, - /// `text-top` - TextTop, - /// `middle` - Middle, - /// `bottom` - Bottom, - /// `text-bottom` - TextBottom, - /// `-moz-middle-with-baseline` - #[cfg(feature = "gecko")] - MozMiddleWithBaseline, +#[repr(C, u8)] +pub enum GenericVerticalAlign { + /// One of the vertical-align keywords. + Keyword(VerticalAlignKeyword), /// `` Length(LengthPercentage), } +pub use self::GenericVerticalAlign as VerticalAlign; + impl VerticalAlign { /// Returns `baseline`. #[inline] pub fn baseline() -> Self { - VerticalAlign::Baseline + VerticalAlign::Keyword(VerticalAlignKeyword::Baseline) } } diff --git a/components/style/values/specified/box.rs b/components/style/values/specified/box.rs index c90c270c59f..7b2c5a34648 100644 --- a/components/style/values/specified/box.rs +++ b/components/style/values/specified/box.rs @@ -10,7 +10,7 @@ use crate::properties::{LonghandId, PropertyDeclarationId, PropertyFlags}; use crate::properties::{PropertyId, ShorthandId}; use crate::values::generics::box_::AnimationIterationCount as GenericAnimationIterationCount; use crate::values::generics::box_::Perspective as GenericPerspective; -use crate::values::generics::box_::VerticalAlign as GenericVerticalAlign; +use crate::values::generics::box_::{GenericVerticalAlign, VerticalAlignKeyword}; use crate::values::specified::length::{LengthPercentage, NonNegativeLength}; use crate::values::specified::{AllowQuirks, Number}; use crate::values::{CustomIdent, KeyframesName}; @@ -280,20 +280,7 @@ impl Parse for VerticalAlign { return Ok(GenericVerticalAlign::Length(lp)); } - try_match_ident_ignore_ascii_case! { input, - "baseline" => Ok(GenericVerticalAlign::Baseline), - "sub" => Ok(GenericVerticalAlign::Sub), - "super" => Ok(GenericVerticalAlign::Super), - "top" => Ok(GenericVerticalAlign::Top), - "text-top" => Ok(GenericVerticalAlign::TextTop), - "middle" => Ok(GenericVerticalAlign::Middle), - "bottom" => Ok(GenericVerticalAlign::Bottom), - "text-bottom" => Ok(GenericVerticalAlign::TextBottom), - #[cfg(feature = "gecko")] - "-moz-middle-with-baseline" => { - Ok(GenericVerticalAlign::MozMiddleWithBaseline) - }, - } + Ok(GenericVerticalAlign::Keyword(VerticalAlignKeyword::parse(input)?)) } } diff --git a/components/style/values/specified/text.rs b/components/style/values/specified/text.rs index bf4c5b288e1..7f1b438ea24 100644 --- a/components/style/values/specified/text.rs +++ b/components/style/values/specified/text.rs @@ -563,8 +563,7 @@ pub enum TextAlignKeyword { } /// Specified value of text-align property. -#[cfg_attr(feature = "gecko", derive(MallocSizeOf))] -#[derive(Clone, Copy, Debug, Eq, Hash, Parse, PartialEq, SpecifiedValueInfo, ToCss, ToShmem)] +#[derive(Clone, Copy, Debug, Eq, Hash, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, ToCss, ToShmem)] pub enum TextAlign { /// Keyword value of text-align property. Keyword(TextAlignKeyword), From 9f73576f6a61b3f1866d5eb509b849f6ad5aea36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 29 Apr 2019 05:58:56 +0000 Subject: [PATCH 16/27] style: Check iterator length in SelectorIter::is_featureless_host_selector. I'm going to unconditionally generate the PseudoElement combinator, and this causes issues since we'll put the raw `::pseudo` selectors in the host bucket, which is obviously wrong. Differential Revision: https://phabricator.services.mozilla.com/D27528 --- components/selectors/parser.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index e3afab57e16..21459e4e46d 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -736,7 +736,8 @@ impl<'a, Impl: 'a + SelectorImpl> SelectorIter<'a, Impl> { /// combinators to the left. #[inline] pub(crate) fn is_featureless_host_selector(&mut self) -> bool { - self.all(|component| matches!(*component, Component::Host(..))) && + self.selector_length() > 0 && + self.all(|component| matches!(*component, Component::Host(..))) && self.next_sequence().is_none() } From a23ad3be501723e14a97bfb3d0f1c2b6ec21cdab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 1 May 2019 17:25:13 +0000 Subject: [PATCH 17/27] style: Add parsing support for ::part(). Disabled for now of course. This should be pretty uncontroversial I'd think. Differential Revision: https://phabricator.services.mozilla.com/D28060 --- components/malloc_size_of/lib.rs | 1 + components/selectors/builder.rs | 1 + components/selectors/matching.rs | 3 + components/selectors/parser.rs | 95 +++++++++++++++---- components/selectors/tree.rs | 5 + components/style/gecko/selector_parser.rs | 1 + components/style/gecko/wrapper.rs | 4 + .../invalidation/element/element_wrapper.rs | 4 + .../invalidation/element/invalidation_map.rs | 1 + .../style/invalidation/element/invalidator.rs | 8 +- 10 files changed, 106 insertions(+), 17 deletions(-) diff --git a/components/malloc_size_of/lib.rs b/components/malloc_size_of/lib.rs index b59e26391ef..a305f86f7cb 100644 --- a/components/malloc_size_of/lib.rs +++ b/components/malloc_size_of/lib.rs @@ -747,6 +747,7 @@ where Component::ExplicitUniversalType | Component::LocalName(..) | Component::ID(..) | + Component::Part(..) | Component::Class(..) | Component::AttributeInNoNamespaceExists { .. } | Component::AttributeInNoNamespace { .. } | diff --git a/components/selectors/builder.rs b/components/selectors/builder.rs index 72404de1084..73c1575371c 100644 --- a/components/selectors/builder.rs +++ b/components/selectors/builder.rs @@ -270,6 +270,7 @@ where Component::Combinator(..) => { unreachable!("Found combinator in simple selectors vector?"); }, + Component::Part(..) | Component::PseudoElement(..) | Component::LocalName(..) => { specificity.element_selectors += 1 }, diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs index f50dc8ddecf..aa200cc4b26 100644 --- a/components/selectors/matching.rs +++ b/components/selectors/matching.rs @@ -450,6 +450,7 @@ where element.containing_shadow_host() }, + Combinator::Part => element.containing_shadow_host(), Combinator::SlotAssignment => { debug_assert!( context.current_host.is_some(), @@ -517,6 +518,7 @@ where Combinator::Child | Combinator::Descendant | Combinator::SlotAssignment | + Combinator::Part | Combinator::PseudoElement => SelectorMatchingResult::NotMatchedGlobally, }; @@ -671,6 +673,7 @@ where match *selector { Component::Combinator(_) => unreachable!(), + Component::Part(ref part) => element.is_part(part), Component::Slotted(ref selector) => { // are never flattened tree slottables. !element.is_html_slot_element() && diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index 21459e4e46d..45cbb56236f 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -68,24 +68,30 @@ bitflags! { /// Whether we're inside a negation. If we're inside a negation, we're /// not allowed to add another negation or such, for example. const INSIDE_NEGATION = 1 << 0; - /// Whether we've parsed an ::slotted() pseudo-element already. + /// Whether we've parsed a ::slotted() pseudo-element already. /// /// If so, then we can only parse a subset of pseudo-elements, and /// whatever comes after them if so. const AFTER_SLOTTED = 1 << 1; + /// Whether we've parsed a ::part() pseudo-element already. + /// + /// If so, then we can only parse a subset of pseudo-elements, and + /// whatever comes after them if so. + const AFTER_PART = 1 << 2; /// Whether we've parsed a pseudo-element (as in, an - /// `Impl::PseudoElement` thus not accounting for `::slotted`) already. + /// `Impl::PseudoElement` thus not accounting for `::slotted` or + /// `::part`) already. /// /// If so, then other pseudo-elements and most other selectors are /// disallowed. - const AFTER_PSEUDO_ELEMENT = 1 << 2; + const AFTER_PSEUDO_ELEMENT = 1 << 3; /// Whether we've parsed a non-stateful pseudo-element (again, as-in /// `Impl::PseudoElement`) already. If so, then other pseudo-classes are /// disallowed. If this flag is set, `AFTER_PSEUDO_ELEMENT` must be set /// as well. - const AFTER_NON_STATEFUL_PSEUDO_ELEMENT = 1 << 3; + const AFTER_NON_STATEFUL_PSEUDO_ELEMENT = 1 << 4; /// Whether we are after any of the pseudo-like things. - const AFTER_PSEUDO = Self::AFTER_SLOTTED.bits | Self::AFTER_PSEUDO_ELEMENT.bits; + const AFTER_PSEUDO = Self::AFTER_PART.bits | Self::AFTER_SLOTTED.bits | Self::AFTER_PSEUDO_ELEMENT.bits; } } @@ -100,6 +106,14 @@ impl SelectorParsingState { !self.intersects(SelectorParsingState::AFTER_PSEUDO) } + // TODO(emilio): Should we allow other ::part()s after ::part()? + // + // See https://github.com/w3c/csswg-drafts/issues/3841 + #[inline] + fn allows_part(self) -> bool { + !self.intersects(SelectorParsingState::AFTER_PSEUDO) + } + #[inline] fn allows_non_functional_pseudo_classes(self) -> bool { !self.intersects(SelectorParsingState::AFTER_SLOTTED | SelectorParsingState::AFTER_NON_STATEFUL_PSEUDO_ELEMENT) @@ -156,6 +170,7 @@ macro_rules! with_all_bounds { type AttrValue: $($InSelector)*; type Identifier: $($InSelector)*; type ClassName: $($InSelector)*; + type PartName: $($InSelector)*; type LocalName: $($InSelector)* + Borrow; type NamespaceUrl: $($CommonBounds)* + Default + Borrow; type NamespacePrefix: $($InSelector)* + Default; @@ -196,6 +211,11 @@ pub trait Parser<'i> { false } + /// Whether to parse the `::part()` pseudo-element. + fn parse_part(&self) -> bool { + false + } + /// Whether to parse the `:host` pseudo-class. fn parse_host(&self) -> bool { false @@ -841,6 +861,9 @@ pub enum Combinator { /// Another combinator used for ::slotted(), which represent the jump from /// a node to its assigned slot. SlotAssignment, + /// Another combinator used for `::part()`, which represents the jump from + /// the part to the containing shadow host. + Part, } impl Combinator { @@ -934,8 +957,7 @@ pub enum Component { LastOfType, OnlyOfType, NonTSPseudoClass(#[shmem(field_bound)] Impl::NonTSPseudoClass), - /// The ::slotted() pseudo-element (which isn't actually a pseudo-element, - /// and probably should be a pseudo-class): + /// The ::slotted() pseudo-element: /// /// https://drafts.csswg.org/css-scoping/#slotted-pseudo /// @@ -947,6 +969,9 @@ pub enum Component { /// /// See https://github.com/w3c/csswg-drafts/issues/2158 Slotted(Selector), + /// The `::part` pseudo-element. + /// https://drafts.csswg.org/css-shadow-parts/#part + Part(#[shmem(field_bound)] Impl::PartName), /// The `:host` pseudo-class: /// /// https://drafts.csswg.org/css-scoping/#host-selector @@ -1196,7 +1221,8 @@ impl ToCss for Combinator { Combinator::Descendant => dest.write_str(" "), Combinator::NextSibling => dest.write_str(" + "), Combinator::LaterSibling => dest.write_str(" ~ "), - Combinator::PseudoElement => Ok(()), + Combinator::PseudoElement | + Combinator::Part | Combinator::SlotAssignment => Ok(()), } } @@ -1236,6 +1262,11 @@ impl ToCss for Component { selector.to_css(dest)?; dest.write_char(')') }, + Part(ref part_name) => { + dest.write_str("::part(")?; + display_to_css_identifier(part_name, dest)?; + dest.write_char(')') + }, PseudoElement(ref p) => p.to_css(dest), ID(ref s) => { dest.write_char('#')?; @@ -1407,15 +1438,12 @@ where { let mut builder = SelectorBuilder::default(); - let mut has_pseudo_element; - let mut slotted; + let mut has_pseudo_element = false; + let mut slotted = false; 'outer_loop: loop { // Parse a sequence of simple selectors. - match parse_compound_selector(parser, input, &mut builder)? { - Some(state) => { - has_pseudo_element = state.intersects(SelectorParsingState::AFTER_PSEUDO_ELEMENT); - slotted = state.intersects(SelectorParsingState::AFTER_SLOTTED); - }, + let state = match parse_compound_selector(parser, input, &mut builder)? { + Some(state) => state, None => { return Err(input.new_custom_error(if builder.has_combinators() { SelectorParseErrorKind::DanglingCombinator @@ -1425,7 +1453,11 @@ where }, }; - if has_pseudo_element || slotted { + if state.intersects(SelectorParsingState::AFTER_PSEUDO) { + has_pseudo_element = state.intersects(SelectorParsingState::AFTER_PSEUDO_ELEMENT); + slotted = state.intersects(SelectorParsingState::AFTER_SLOTTED); + let part = state.intersects(SelectorParsingState::AFTER_PART); + debug_assert!(has_pseudo_element || slotted || part); break; } @@ -1463,6 +1495,8 @@ where builder.push_combinator(combinator); } + // TODO(emilio): We'll have to flag part() somehow as well, but we need more + // bits! Ok(Selector(builder.build(has_pseudo_element, slotted))) } @@ -1553,6 +1587,7 @@ enum SimpleSelectorParseResult { SimpleSelector(Component), PseudoElement(Impl::PseudoElement), SlottedPseudo(Selector), + PartPseudo(Impl::PartName), } #[derive(Debug)] @@ -1899,6 +1934,7 @@ where return Err(input.new_custom_error(SelectorParseErrorKind::EmptyNegation)); }, Some(SimpleSelectorParseResult::PseudoElement(_)) | + Some(SimpleSelectorParseResult::PartPseudo(_)) | Some(SimpleSelectorParseResult::SlottedPseudo(_)) => { let e = SelectorParseErrorKind::NonSimpleSelectorInNegation; return Err(input.new_custom_error(e)); @@ -1955,6 +1991,11 @@ where SimpleSelectorParseResult::SimpleSelector(s) => { builder.push_simple_selector(s); }, + SimpleSelectorParseResult::PartPseudo(part_name) => { + state.insert(SelectorParsingState::AFTER_PART); + builder.push_combinator(Combinator::Part); + builder.push_simple_selector(Component::Part(part_name)); + }, SimpleSelectorParseResult::SlottedPseudo(selector) => { state.insert(SelectorParsingState::AFTER_SLOTTED); if !builder.is_empty() { @@ -2115,6 +2156,15 @@ where return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState)); } let pseudo_element = if is_functional { + if P::parse_part(parser) && name.eq_ignore_ascii_case("part") { + if !state.allows_part() { + return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState)); + } + let name = input.parse_nested_block(|input| { + Ok(input.expect_ident()?.as_ref().into()) + })?; + return Ok(Some(SimpleSelectorParseResult::PartPseudo(name))); + } if P::parse_slotted(parser) && name.eq_ignore_ascii_case("slotted") { if !state.allows_slotted() { return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState)); @@ -2298,6 +2348,7 @@ pub mod tests { type AttrValue = DummyAtom; type Identifier = DummyAtom; type ClassName = DummyAtom; + type PartName = DummyAtom; type LocalName = DummyAtom; type NamespaceUrl = DummyAtom; type NamespacePrefix = DummyAtom; @@ -2336,6 +2387,10 @@ pub mod tests { true } + fn parse_part(&self) -> bool { + true + } + fn parse_non_ts_pseudo_class( &self, location: SourceLocation, @@ -2910,6 +2965,14 @@ pub mod tests { assert!(parse("::slotted(div).foo").is_err()); assert!(parse("::slotted(div + bar)").is_err()); assert!(parse("::slotted(div) + foo").is_err()); + + assert!(parse("::part()").is_err()); + assert!(parse("::part(42)").is_err()); + // Though note https://github.com/w3c/csswg-drafts/issues/3502 + assert!(parse("::part(foo bar)").is_err()); + assert!(parse("::part(foo):hover").is_ok()); + assert!(parse("::part(foo) + bar").is_err()); + assert!(parse("div ::slotted(div)").is_ok()); assert!(parse("div + slot::slotted(div)").is_ok()); assert!(parse("div + slot::slotted(div.foo)").is_ok()); diff --git a/components/selectors/tree.rs b/components/selectors/tree.rs index badfca86ed6..b57876308a5 100644 --- a/components/selectors/tree.rs +++ b/components/selectors/tree.rs @@ -110,6 +110,11 @@ pub trait Element: Sized + Clone + Debug { case_sensitivity: CaseSensitivity, ) -> bool; + fn is_part( + &self, + name: &::PartName, + ) -> bool; + /// Returns whether this element matches `:empty`. /// /// That is, whether it does not contain any child element or any non-zero-length text node. diff --git a/components/style/gecko/selector_parser.rs b/components/style/gecko/selector_parser.rs index ec0db9286ec..18718446c1a 100644 --- a/components/style/gecko/selector_parser.rs +++ b/components/style/gecko/selector_parser.rs @@ -289,6 +289,7 @@ impl ::selectors::SelectorImpl for SelectorImpl { type AttrValue = Atom; type Identifier = Atom; type ClassName = Atom; + type PartName = Atom; type LocalName = Atom; type NamespacePrefix = Atom; type NamespaceUrl = Namespace; diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 079dde2f513..f88e30ab777 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -2259,6 +2259,10 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { case_sensitivity.eq_atom(element_id, id) } + fn is_part(&self, _name: &Atom) -> bool { + unimplemented!(); + } + #[inline(always)] fn has_class(&self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool { let attr = match self.get_class_attr() { diff --git a/components/style/invalidation/element/element_wrapper.rs b/components/style/invalidation/element/element_wrapper.rs index c794eb15c2c..114cdc9cd8a 100644 --- a/components/style/invalidation/element/element_wrapper.rs +++ b/components/style/invalidation/element/element_wrapper.rs @@ -340,6 +340,10 @@ where } } + fn is_part(&self, _name: &Atom) -> bool { + unimplemented!(); + } + fn has_class(&self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool { match self.snapshot() { Some(snapshot) if snapshot.has_attrs() => snapshot.has_class(name, case_sensitivity), diff --git a/components/style/invalidation/element/invalidation_map.rs b/components/style/invalidation/element/invalidation_map.rs index e4cc8115760..63d6eb6acc4 100644 --- a/components/style/invalidation/element/invalidation_map.rs +++ b/components/style/invalidation/element/invalidation_map.rs @@ -98,6 +98,7 @@ impl Dependency { // an eager pseudo, and return only Descendants here if not. Some(Combinator::PseudoElement) => DependencyInvalidationKind::ElementAndDescendants, Some(Combinator::SlotAssignment) => DependencyInvalidationKind::SlottedElements, + Some(Combinator::Part) => unimplemented!("Need to add invalidation for shadow parts"), } } } diff --git a/components/style/invalidation/element/invalidator.rs b/components/style/invalidation/element/invalidator.rs index 371c0771183..ec1548f4aa3 100644 --- a/components/style/invalidation/element/invalidator.rs +++ b/components/style/invalidation/element/invalidator.rs @@ -158,7 +158,10 @@ impl<'a> Invalidation<'a> { // We should be able to do better here! match self.selector.combinator_at_parse_order(self.offset - 1) { Combinator::Descendant | Combinator::LaterSibling | Combinator::PseudoElement => true, - Combinator::SlotAssignment | Combinator::NextSibling | Combinator::Child => false, + Combinator::Part | + Combinator::SlotAssignment | + Combinator::NextSibling | + Combinator::Child => false, } } @@ -171,6 +174,9 @@ impl<'a> Invalidation<'a> { Combinator::Child | Combinator::Descendant | Combinator::PseudoElement => { InvalidationKind::Descendant(DescendantInvalidationKind::Dom) }, + Combinator::Part => { + unimplemented!("Need to add invalidation for shadow parts"); + }, Combinator::SlotAssignment => { InvalidationKind::Descendant(DescendantInvalidationKind::Slotted) }, From 627514b7377cf8937c2bcc03d633bad38208454f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 1 May 2019 17:28:23 +0000 Subject: [PATCH 18/27] style: Implement selector-matching for ::part(). Also fairly straight-forward. This may get more complicated when we do part forwarding, if any. I've opened https://github.com/w3c/csswg-drafts/issues/3841 in what I think would be a cleaner model for forwarding. Differential Revision: https://phabricator.services.mozilla.com/D28063 --- components/style/gecko/snapshot.rs | 12 +++++++++++- components/style/gecko/snapshot_helpers.rs | 12 ++++++------ components/style/gecko/wrapper.rs | 17 ++++++++++++++--- .../invalidation/element/element_wrapper.rs | 11 +++++++++-- 4 files changed, 40 insertions(+), 12 deletions(-) diff --git a/components/style/gecko/snapshot.rs b/components/style/gecko/snapshot.rs index 6ac4003fa25..d294939baeb 100644 --- a/components/style/gecko/snapshot.rs +++ b/components/style/gecko/snapshot.rs @@ -183,13 +183,23 @@ impl ElementSnapshot for GeckoElementSnapshot { snapshot_helpers::get_id(&*self.mAttrs) } + #[inline] + fn is_part(&self, name: &Atom) -> bool { + let attr = match snapshot_helpers::find_attr(&*self.mAttrs, &atom!("part")) { + Some(attr) => attr, + None => return false, + }; + + snapshot_helpers::has_class_or_part(name, CaseSensitivity::CaseSensitive, attr) + } + #[inline] fn has_class(&self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool { if !self.has_any(Flags::MaybeClass) { return false; } - snapshot_helpers::has_class(name, case_sensitivity, &self.mClass) + snapshot_helpers::has_class_or_part(name, case_sensitivity, &self.mClass) } #[inline] diff --git a/components/style/gecko/snapshot_helpers.rs b/components/style/gecko/snapshot_helpers.rs index 0eb84830546..aaa2254dbd7 100644 --- a/components/style/gecko/snapshot_helpers.rs +++ b/components/style/gecko/snapshot_helpers.rs @@ -29,7 +29,7 @@ unsafe fn ptr(attr: &structs::nsAttrValue) -> *const T { } #[inline(always)] -unsafe fn get_class_from_attr(attr: &structs::nsAttrValue) -> Class { +unsafe fn get_class_or_part_from_attr(attr: &structs::nsAttrValue) -> Class { debug_assert!(bindings::Gecko_AssertClassAttrValueIsSane(attr)); let base_type = base_type(attr); if base_type == structs::nsAttrValue_ValueBaseType_eStringBase { @@ -82,15 +82,15 @@ pub fn get_id(attrs: &[structs::AttrArray_InternalAttr]) -> Option<&WeakAtom> { Some(unsafe { get_id_from_attr(find_attr(attrs, &atom!("id"))?) }) } -/// Given a class name, a case sensitivity, and an array of attributes, returns -/// whether the class has the attribute that name represents. +/// Given a class or part name, a case sensitivity, and an array of attributes, +/// returns whether the attribute has that name. #[inline(always)] -pub fn has_class( +pub fn has_class_or_part( name: &Atom, case_sensitivity: CaseSensitivity, attr: &structs::nsAttrValue, ) -> bool { - match unsafe { get_class_from_attr(attr) } { + match unsafe { get_class_or_part_from_attr(attr) } { Class::None => false, Class::One(atom) => unsafe { case_sensitivity.eq_atom(name, WeakAtom::new(atom)) }, Class::More(atoms) => match case_sensitivity { @@ -114,7 +114,7 @@ where F: FnMut(&Atom), { unsafe { - match get_class_from_attr(attr) { + match get_class_or_part_from_attr(attr) { Class::None => {}, Class::One(atom) => Atom::with(atom, callback), Class::More(atoms) => { diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index f88e30ab777..9d987c56308 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -573,6 +573,11 @@ impl<'le> GeckoElement<'le> { } } + #[inline(always)] + fn get_part_attr(&self) -> Option<&structs::nsAttrValue> { + snapshot_helpers::find_attr(self.attrs(), &atom!("part")) + } + #[inline(always)] fn get_class_attr(&self) -> Option<&structs::nsAttrValue> { if !self.may_have_class() { @@ -2259,8 +2264,14 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { case_sensitivity.eq_atom(element_id, id) } - fn is_part(&self, _name: &Atom) -> bool { - unimplemented!(); + #[inline] + fn is_part(&self, name: &Atom) -> bool { + let attr = match self.get_part_attr() { + Some(c) => c, + None => return false, + }; + + snapshot_helpers::has_class_or_part(name, CaseSensitivity::CaseSensitive, attr) } #[inline(always)] @@ -2270,7 +2281,7 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { None => return false, }; - snapshot_helpers::has_class(name, case_sensitivity, attr) + snapshot_helpers::has_class_or_part(name, case_sensitivity, attr) } #[inline] diff --git a/components/style/invalidation/element/element_wrapper.rs b/components/style/invalidation/element/element_wrapper.rs index 114cdc9cd8a..6e8e504c226 100644 --- a/components/style/invalidation/element/element_wrapper.rs +++ b/components/style/invalidation/element/element_wrapper.rs @@ -58,6 +58,10 @@ pub trait ElementSnapshot: Sized { /// if `has_attrs()` returns true. fn has_class(&self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool; + /// Whether this snapshot represents the part named `name`. Should only be + /// called if `has_attrs()` returns true. + fn is_part(&self, name: &Atom) -> bool; + /// A callback that should be called for each class of the snapshot. Should /// only be called if `has_attrs()` returns true. fn each_class(&self, _: F) @@ -340,8 +344,11 @@ where } } - fn is_part(&self, _name: &Atom) -> bool { - unimplemented!(); + fn is_part(&self, name: &Atom) -> bool { + match self.snapshot() { + Some(snapshot) if snapshot.has_attrs() => snapshot.is_part(name), + _ => self.element.is_part(name), + } } fn has_class(&self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool { From 89bf34f46e273029c8141dd12a0cfc2e3b4fb70f Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Wed, 1 May 2019 02:28:25 +0000 Subject: [PATCH 19/27] Move all remaining members of nsIPresShell to mozilla::PresShell Additionally, this sorts out the order of member variables for minimizing the instance size. And also this changes `enum RenderFlags` to `enum class RenderingStateFlags`. Differential Revision: https://phabricator.services.mozilla.com/D29312 --- components/style/gecko/media_queries.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index b39e748ce2b..4c04d9573e5 100644 --- a/components/style/gecko/media_queries.rs +++ b/components/style/gecko/media_queries.rs @@ -169,7 +169,6 @@ impl Device { self.document() .mPresShell .as_ref()? - ._base .mPresContext .mRawPtr .as_ref() From d89e4ad4f38afae2a0f05e12049038aa236691d4 Mon Sep 17 00:00:00 2001 From: Jeremy Ir Date: Sun, 5 May 2019 23:39:27 +0000 Subject: [PATCH 20/27] style: Convert NS_STYLE_BORDER to an enum class in nsStyleConsts.h. Converting the NS_STYLE_BORDER definitions in to enumerated classes as per bug 1277133. The original constants broke the convention used by the rest of the definitions as the CSS property being described is `border-collapse`, so corrections were made with the migration to the enumerated class. Differential Revision: https://phabricator.services.mozilla.com/D29951 --- components/style/properties/longhands/inherited_table.mako.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/style/properties/longhands/inherited_table.mako.rs b/components/style/properties/longhands/inherited_table.mako.rs index 7dc4d73fe62..246a073d2a8 100644 --- a/components/style/properties/longhands/inherited_table.mako.rs +++ b/components/style/properties/longhands/inherited_table.mako.rs @@ -9,7 +9,7 @@ ${helpers.single_keyword( "border-collapse", "separate collapse", - gecko_constant_prefix="NS_STYLE_BORDER", + gecko_enum_prefix="StyleBorderCollapse", animation_value_type="discrete", spec="https://drafts.csswg.org/css-tables/#propdef-border-collapse", servo_restyle_damage = "reflow", From deba73a528c6905c87500c414595470d319cf1c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 5 May 2019 13:27:55 +0200 Subject: [PATCH 21/27] Remove a useless MallocSizeOf implementation. Now that this is no longer used in AuthorStyles there's no point in doing this. --- components/malloc_size_of/lib.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/components/malloc_size_of/lib.rs b/components/malloc_size_of/lib.rs index a305f86f7cb..d65f8e957e4 100644 --- a/components/malloc_size_of/lib.rs +++ b/components/malloc_size_of/lib.rs @@ -777,12 +777,6 @@ impl MallocSizeOf } } -impl MallocSizeOf for selectors::context::QuirksMode { - fn size_of(&self, _ops: &mut MallocSizeOfOps) -> usize { - 0 - } -} - impl MallocSizeOf for Void { #[inline] fn size_of(&self, _ops: &mut MallocSizeOfOps) -> usize { From 57874ae90a69c7e0af6e604d570674256afbecf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 5 May 2019 13:35:24 +0200 Subject: [PATCH 22/27] style: Rustfmt recent changes. --- components/selectors/builder.rs | 3 +- components/selectors/parser.rs | 63 +++++++++++++--------- components/selectors/tree.rs | 5 +- components/style/stylist.rs | 10 ++-- components/style/values/computed/mod.rs | 2 +- components/style/values/computed/text.rs | 2 +- components/style/values/specified/box.rs | 4 +- components/style/values/specified/image.rs | 28 ++++++---- components/style/values/specified/mod.rs | 2 +- components/style/values/specified/text.rs | 10 ++-- 10 files changed, 73 insertions(+), 56 deletions(-) diff --git a/components/selectors/builder.rs b/components/selectors/builder.rs index 73c1575371c..b548ceb83ce 100644 --- a/components/selectors/builder.rs +++ b/components/selectors/builder.rs @@ -270,8 +270,7 @@ where Component::Combinator(..) => { unreachable!("Found combinator in simple selectors vector?"); }, - Component::Part(..) | - Component::PseudoElement(..) | Component::LocalName(..) => { + Component::Part(..) | Component::PseudoElement(..) | Component::LocalName(..) => { specificity.element_selectors += 1 }, Component::Slotted(ref selector) => { diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index 45cbb56236f..a924bbc17d0 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -30,10 +30,14 @@ pub trait PseudoElement: Sized + ToCss { /// Whether the pseudo-element supports a given state selector to the right /// of it. - fn accepts_state_pseudo_classes(&self) -> bool { false } + fn accepts_state_pseudo_classes(&self) -> bool { + false + } /// Whether this pseudo-element is valid after a ::slotted(..) pseudo. - fn valid_after_slotted(&self) -> bool { false } + fn valid_after_slotted(&self) -> bool { + false + } } /// A trait that represents a pseudo-class. @@ -116,7 +120,10 @@ impl SelectorParsingState { #[inline] fn allows_non_functional_pseudo_classes(self) -> bool { - !self.intersects(SelectorParsingState::AFTER_SLOTTED | SelectorParsingState::AFTER_NON_STATEFUL_PSEUDO_ELEMENT) + !self.intersects( + SelectorParsingState::AFTER_SLOTTED | + SelectorParsingState::AFTER_NON_STATEFUL_PSEUDO_ELEMENT, + ) } #[inline] @@ -1221,9 +1228,7 @@ impl ToCss for Combinator { Combinator::Descendant => dest.write_str(" "), Combinator::NextSibling => dest.write_str(" + "), Combinator::LaterSibling => dest.write_str(" ~ "), - Combinator::PseudoElement | - Combinator::Part | - Combinator::SlotAssignment => Ok(()), + Combinator::PseudoElement | Combinator::Part | Combinator::SlotAssignment => Ok(()), } } } @@ -1979,11 +1984,10 @@ where let mut state = SelectorParsingState::empty(); loop { - let parse_result = - match parse_one_simple_selector(parser, input, state)? { - None => break, - Some(result) => result, - }; + let parse_result = match parse_one_simple_selector(parser, input, state)? { + None => break, + Some(result) => result, + }; empty = false; @@ -2034,9 +2038,7 @@ where Impl: SelectorImpl, { if !state.allows_functional_pseudo_classes() { - return Err(input.new_custom_error( - SelectorParseErrorKind::InvalidState - )); + return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState)); } debug_assert!(state.allows_tree_structural_pseudo_classes()); match_ignore_ascii_case! { &name, @@ -2102,7 +2104,7 @@ where Err(..) => { input.reset(&start); return Ok(None); - } + }, }; Ok(Some(match token { @@ -2122,7 +2124,7 @@ where Token::Ident(ref class) => class, ref t => { let e = SelectorParseErrorKind::ClassNeedsIdent(t.clone()); - return Err(location.new_custom_error(e)) + return Err(location.new_custom_error(e)); }, }; let class = Component::Class(class.as_ref().into()); @@ -2149,8 +2151,7 @@ where return Err(input.new_custom_error(e)); }, }; - let is_pseudo_element = - !is_single_colon || is_css2_pseudo_element(&name); + let is_pseudo_element = !is_single_colon || is_css2_pseudo_element(&name); if is_pseudo_element { if state.intersects(SelectorParsingState::AFTER_PSEUDO_ELEMENT) { return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState)); @@ -2158,7 +2159,9 @@ where let pseudo_element = if is_functional { if P::parse_part(parser) && name.eq_ignore_ascii_case("part") { if !state.allows_part() { - return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState)); + return Err( + input.new_custom_error(SelectorParseErrorKind::InvalidState) + ); } let name = input.parse_nested_block(|input| { Ok(input.expect_ident()?.as_ref().into()) @@ -2167,7 +2170,9 @@ where } if P::parse_slotted(parser) && name.eq_ignore_ascii_case("slotted") { if !state.allows_slotted() { - return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState)); + return Err( + input.new_custom_error(SelectorParseErrorKind::InvalidState) + ); } let selector = input.parse_nested_block(|input| { parse_inner_compound_selector(parser, input) @@ -2181,7 +2186,9 @@ where P::parse_pseudo_element(parser, location, name)? }; - if state.intersects(SelectorParsingState::AFTER_SLOTTED) && !pseudo_element.valid_after_slotted() { + if state.intersects(SelectorParsingState::AFTER_SLOTTED) && + !pseudo_element.valid_after_slotted() + { return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState)); } SimpleSelectorParseResult::PseudoElement(pseudo_element) @@ -2198,7 +2205,7 @@ where }, _ => { input.reset(&start); - return Ok(None) + return Ok(None); }, })) } @@ -2234,7 +2241,9 @@ where } let pseudo_class = P::parse_non_ts_pseudo_class(parser, location, name)?; - if state.intersects(SelectorParsingState::AFTER_PSEUDO_ELEMENT) && !pseudo_class.is_user_action_state() { + if state.intersects(SelectorParsingState::AFTER_PSEUDO_ELEMENT) && + !pseudo_class.is_user_action_state() + { return Err(location.new_custom_error(SelectorParseErrorKind::InvalidState)); } Ok(Component::NonTSPseudoClass(pseudo_class)) @@ -2266,9 +2275,13 @@ pub mod tests { impl parser::PseudoElement for PseudoElement { type Impl = DummySelectorImpl; - fn accepts_state_pseudo_classes(&self) -> bool { true } + fn accepts_state_pseudo_classes(&self) -> bool { + true + } - fn valid_after_slotted(&self) -> bool { true } + fn valid_after_slotted(&self) -> bool { + true + } } impl parser::NonTSPseudoClass for PseudoClass { diff --git a/components/selectors/tree.rs b/components/selectors/tree.rs index b57876308a5..52599893d2f 100644 --- a/components/selectors/tree.rs +++ b/components/selectors/tree.rs @@ -110,10 +110,7 @@ pub trait Element: Sized + Clone + Debug { case_sensitivity: CaseSensitivity, ) -> bool; - fn is_part( - &self, - name: &::PartName, - ) -> bool; + fn is_part(&self, name: &::PartName) -> bool; /// Returns whether this element matches `:empty`. /// diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 08f3cee0464..2953e3f0671 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -49,8 +49,8 @@ use selectors::NthIndexCache; use servo_arc::{Arc, ArcBorrow}; use smallbitvec::SmallBitVec; use smallvec::SmallVec; -use std::{mem, ops}; use std::sync::Mutex; +use std::{mem, ops}; use style_traits::viewport::ViewportConstraints; /// The type of the stylesheets that the stylist contains. @@ -271,12 +271,8 @@ impl DocumentCascadeData { let origin_sheets = flusher.origin_sheets(Origin::UserAgent); let _unused_cascade_datas = { let mut ua_cache = UA_CASCADE_DATA_CACHE.lock().unwrap(); - self.user_agent = ua_cache.lookup( - origin_sheets, - device, - quirks_mode, - guards.ua_or_user, - )?; + self.user_agent = + ua_cache.lookup(origin_sheets, device, quirks_mode, guards.ua_or_user)?; debug!("User agent data cache size {:?}", ua_cache.len()); ua_cache.take_unused() }; diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index 78384ea6b94..3eb0c16836c 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -86,8 +86,8 @@ pub use self::transform::{TransformOrigin, TransformStyle, Translate}; #[cfg(feature = "gecko")] pub use self::ui::CursorImage; pub use self::ui::{Cursor, MozForceBrokenImageIcon, UserSelect}; -pub use super::specified::{BorderStyle, TextDecorationLine}; pub use super::specified::TextTransform; +pub use super::specified::{BorderStyle, TextDecorationLine}; pub use super::{Auto, Either, None_}; pub use app_units::Au; diff --git a/components/style/values/computed/text.rs b/components/style/values/computed/text.rs index b88ba850536..c29d3d45210 100644 --- a/components/style/values/computed/text.rs +++ b/components/style/values/computed/text.rs @@ -19,9 +19,9 @@ use std::fmt::{self, Write}; use style_traits::{CssWriter, ToCss}; pub use crate::values::specified::TextAlignKeyword as TextAlign; +pub use crate::values::specified::TextTransform; pub use crate::values::specified::{OverflowWrap, WordBreak}; pub use crate::values::specified::{TextDecorationLine, TextEmphasisPosition}; -pub use crate::values::specified::TextTransform; /// A computed value for the `initial-letter` property. pub type InitialLetter = GenericInitialLetter; diff --git a/components/style/values/specified/box.rs b/components/style/values/specified/box.rs index 7b2c5a34648..31a79c141e1 100644 --- a/components/style/values/specified/box.rs +++ b/components/style/values/specified/box.rs @@ -280,7 +280,9 @@ impl Parse for VerticalAlign { return Ok(GenericVerticalAlign::Length(lp)); } - Ok(GenericVerticalAlign::Keyword(VerticalAlignKeyword::parse(input)?)) + Ok(GenericVerticalAlign::Keyword(VerticalAlignKeyword::parse( + input, + )?)) } } diff --git a/components/style/values/specified/image.rs b/components/style/values/specified/image.rs index a387d8e07c2..ad2ecb18086 100644 --- a/components/style/values/specified/image.rs +++ b/components/style/values/specified/image.rs @@ -510,13 +510,16 @@ impl Gradient { items.sort_by(|a, b| { match (a, b) { ( - &generic::GradientItem::ComplexColorStop { position: ref a_position, .. }, - &generic::GradientItem::ComplexColorStop { position: ref b_position, .. }, + &generic::GradientItem::ComplexColorStop { + position: ref a_position, + .. + }, + &generic::GradientItem::ComplexColorStop { + position: ref b_position, + .. + }, ) => match (a_position, b_position) { - ( - &LengthPercentage::Percentage(a), - &LengthPercentage::Percentage(b), - ) => { + (&LengthPercentage::Percentage(a), &LengthPercentage::Percentage(b)) => { return a.0.partial_cmp(&b.0).unwrap_or(Ordering::Equal); }, _ => {}, @@ -786,7 +789,7 @@ impl LineDirection { #[cfg(feature = "gecko")] { // `-moz-` prefixed linear gradient can be both Angle and Position. - if *compat_mode == CompatMode::Moz && !simple_moz_gradient(){ + if *compat_mode == CompatMode::Moz && !simple_moz_gradient() { let position = i.try(|i| LegacyPosition::parse(context, i)).ok(); if _angle.is_none() { _angle = i.try(|i| Angle::parse(context, i)).ok(); @@ -961,10 +964,13 @@ impl GradientItem { if let Ok(multi_position) = input.try(|i| LengthPercentage::parse(context, i)) { let stop_color = stop.color.clone(); items.push(stop.into_item()); - items.push(ColorStop { - color: stop_color, - position: Some(multi_position), - }.into_item()); + items.push( + ColorStop { + color: stop_color, + position: Some(multi_position), + } + .into_item(), + ); } else { items.push(stop.into_item()); } diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index 64a45231c03..7dbb93f6319 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -80,10 +80,10 @@ pub use self::svg::{SVGLength, SVGOpacity, SVGPaint, SVGPaintKind}; pub use self::svg::{SVGPaintOrder, SVGStrokeDashArray, SVGWidth}; pub use self::svg_path::SVGPathData; pub use self::table::XSpan; +pub use self::text::TextTransform; pub use self::text::{InitialLetter, LetterSpacing, LineHeight, TextAlign}; pub use self::text::{OverflowWrap, TextEmphasisPosition, TextEmphasisStyle, WordBreak}; pub use self::text::{TextAlignKeyword, TextDecorationLine, TextOverflow, WordSpacing}; -pub use self::text::TextTransform; pub use self::time::Time; pub use self::transform::{Rotate, Scale, Transform}; pub use self::transform::{TransformOrigin, TransformStyle, Translate}; diff --git a/components/style/values/specified/text.rs b/components/style/values/specified/text.rs index 7f1b438ea24..6be20f58618 100644 --- a/components/style/values/specified/text.rs +++ b/components/style/values/specified/text.rs @@ -361,7 +361,8 @@ impl TextDecorationLine { SpecifiedValueInfo, ToComputedValue, ToResolvedValue, - ToShmem)] + ToShmem, +)] #[repr(C)] /// Specified value of the text-transform property, stored in two parts: /// the case-related transforms (mutually exclusive, only one may be in effect), and others (non-exclusive). @@ -468,7 +469,8 @@ impl ToCss for TextTransform { ToComputedValue, ToCss, ToResolvedValue, - ToShmem)] + ToShmem, +)] #[repr(C)] /// Specified keyword values for case transforms in the text-transform property. (These are exclusive.) pub enum TextTransformCase { @@ -563,7 +565,9 @@ pub enum TextAlignKeyword { } /// Specified value of text-align property. -#[derive(Clone, Copy, Debug, Eq, Hash, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, ToCss, ToShmem)] +#[derive( + Clone, Copy, Debug, Eq, Hash, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, ToCss, ToShmem, +)] pub enum TextAlign { /// Keyword value of text-align property. Keyword(TextAlignKeyword), From 561018da7d3302c3205f059440ecd987d5be1a92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 6 May 2019 10:06:26 +0200 Subject: [PATCH 23/27] style: Fix servo build. --- components/style/servo/selector_parser.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/components/style/servo/selector_parser.rs b/components/style/servo/selector_parser.rs index 9d94c467b58..1ef49c87f9f 100644 --- a/components/style/servo/selector_parser.rs +++ b/components/style/servo/selector_parser.rs @@ -66,10 +66,6 @@ pub const PSEUDO_COUNT: usize = PseudoElement::ServoInlineAbsolute as usize + 1; impl ::selectors::parser::PseudoElement for PseudoElement { type Impl = SelectorImpl; - - fn supports_pseudo_class(&self, _: &NonTSPseudoClass) -> bool { - false - } } impl ToCss for PseudoElement { @@ -293,6 +289,14 @@ impl ::selectors::parser::NonTSPseudoClass for NonTSPseudoClass { fn is_active_or_hover(&self) -> bool { matches!(*self, NonTSPseudoClass::Active | NonTSPseudoClass::Hover) } + + #[inline] + fn is_user_action_state(&self) -> bool { + matches!( + *self, + NonTSPseudoClass::Active | NonTSPseudoClass::Hover | NonTSPseudoClass::Focus + ) + } } impl ToCss for NonTSPseudoClass { @@ -393,6 +397,7 @@ impl ::selectors::SelectorImpl for SelectorImpl { type AttrValue = String; type Identifier = Atom; type ClassName = Atom; + type PartName = Atom; type LocalName = LocalName; type NamespacePrefix = Prefix; type NamespaceUrl = Namespace; @@ -679,6 +684,10 @@ impl ElementSnapshot for ServoElementSnapshot { .map(|v| v.as_atom()) } + fn is_part(&self, _name: &Atom) -> bool { + false + } + fn has_class(&self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool { self.get_attr(&ns!(), &local_name!("class")) .map_or(false, |v| { From 0000e4cec281cc5770c6400e747867877351ab6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 6 May 2019 10:17:08 +0200 Subject: [PATCH 24/27] layout: fix Servo build. --- components/layout/display_list/gradient.rs | 11 ++- components/layout/fragment.rs | 107 +++++++++++---------- components/layout/inline.rs | 31 +++--- components/layout/table_cell.rs | 8 +- components/layout/text.rs | 12 +-- components/layout_thread/dom_wrapper.rs | 11 +++ components/script/dom/element.rs | 4 + 7 files changed, 112 insertions(+), 72 deletions(-) diff --git a/components/layout/display_list/gradient.rs b/components/layout/display_list/gradient.rs index db5208eadac..9b19ee48299 100644 --- a/components/layout/display_list/gradient.rs +++ b/components/layout/display_list/gradient.rs @@ -12,7 +12,7 @@ use style::values::computed::image::{EndingShape, LineDirection}; use style::values::computed::{Angle, GradientItem, LengthPercentage, Percentage, Position}; use style::values::generics::image::EndingShape as GenericEndingShape; use style::values::generics::image::GradientItem as GenericGradientItem; -use style::values::generics::image::{Circle, Ellipse, ShapeExtent}; +use style::values::generics::image::{Circle, ColorStop, Ellipse, ShapeExtent}; use style::values::specified::position::{X, Y}; use webrender_api::{ExtendMode, Gradient, GradientBuilder, GradientStop, RadialGradient}; @@ -92,7 +92,14 @@ fn convert_gradient_stops( let mut stop_items = gradient_items .iter() .filter_map(|item| match *item { - GenericGradientItem::ColorStop(ref stop) => Some(*stop), + GenericGradientItem::SimpleColorStop(color) => Some(ColorStop { + color, + position: None, + }), + GenericGradientItem::ComplexColorStop { color, position } => Some(ColorStop { + color, + position: Some(position), + }), _ => None, }) .collect::>(); diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs index ff62cc79638..ccc3de5ec7e 100644 --- a/components/layout/fragment.rs +++ b/components/layout/fragment.rs @@ -61,8 +61,8 @@ use style::selector_parser::RestyleDamage; use style::servo::restyle_damage::ServoRestyleDamage; use style::str::char_is_whitespace; use style::values::computed::counters::ContentItem; -use style::values::computed::{LengthPercentage, LengthPercentageOrAuto, Size}; -use style::values::generics::box_::{Perspective, VerticalAlign}; +use style::values::computed::{LengthPercentage, LengthPercentageOrAuto, Size, VerticalAlign}; +use style::values::generics::box_::{Perspective, VerticalAlignKeyword}; use style::values::generics::transform; use style::Zero; use webrender_api::{self, LayoutTransform}; @@ -2397,47 +2397,49 @@ impl Fragment { // FIXME(#5624, pcwalton): This passes our current reftests but isn't the right thing // to do. match style.get_box().vertical_align { - VerticalAlign::Baseline => {}, - VerticalAlign::Middle => { - let font_metrics = - with_thread_local_font_context(layout_context, |font_context| { - text::font_metrics_for_style(font_context, self.style.clone_font()) - }); - offset += (content_inline_metrics.ascent - - content_inline_metrics.space_below_baseline - - font_metrics.x_height) - .scale_by(0.5) - }, - VerticalAlign::Sub => { - offset += minimum_line_metrics - .space_needed() - .scale_by(FONT_SUBSCRIPT_OFFSET_RATIO) - }, - VerticalAlign::Super => { - offset -= minimum_line_metrics - .space_needed() - .scale_by(FONT_SUPERSCRIPT_OFFSET_RATIO) - }, - VerticalAlign::TextTop => { - offset = self.content_inline_metrics(layout_context).ascent - - minimum_line_metrics.space_above_baseline - }, - VerticalAlign::TextBottom => { - offset = minimum_line_metrics.space_below_baseline - - self.content_inline_metrics(layout_context) - .space_below_baseline - }, - VerticalAlign::Top => { - if let Some(actual_line_metrics) = actual_line_metrics { - offset = - content_inline_metrics.ascent - actual_line_metrics.space_above_baseline - } - }, - VerticalAlign::Bottom => { - if let Some(actual_line_metrics) = actual_line_metrics { - offset = actual_line_metrics.space_below_baseline - - content_inline_metrics.space_below_baseline - } + VerticalAlign::Keyword(kw) => match kw { + VerticalAlignKeyword::Baseline => {}, + VerticalAlignKeyword::Middle => { + let font_metrics = + with_thread_local_font_context(layout_context, |font_context| { + text::font_metrics_for_style(font_context, self.style.clone_font()) + }); + offset += (content_inline_metrics.ascent - + content_inline_metrics.space_below_baseline - + font_metrics.x_height) + .scale_by(0.5) + }, + VerticalAlignKeyword::Sub => { + offset += minimum_line_metrics + .space_needed() + .scale_by(FONT_SUBSCRIPT_OFFSET_RATIO) + }, + VerticalAlignKeyword::Super => { + offset -= minimum_line_metrics + .space_needed() + .scale_by(FONT_SUPERSCRIPT_OFFSET_RATIO) + }, + VerticalAlignKeyword::TextTop => { + offset = self.content_inline_metrics(layout_context).ascent - + minimum_line_metrics.space_above_baseline + }, + VerticalAlignKeyword::TextBottom => { + offset = minimum_line_metrics.space_below_baseline - + self.content_inline_metrics(layout_context) + .space_below_baseline + }, + VerticalAlignKeyword::Top => { + if let Some(actual_line_metrics) = actual_line_metrics { + offset = content_inline_metrics.ascent - + actual_line_metrics.space_above_baseline + } + }, + VerticalAlignKeyword::Bottom => { + if let Some(actual_line_metrics) = actual_line_metrics { + offset = actual_line_metrics.space_below_baseline - + content_inline_metrics.space_below_baseline + } + }, }, VerticalAlign::Length(ref lp) => { offset -= lp.to_used_value(minimum_line_metrics.space_needed()); @@ -3087,15 +3089,22 @@ impl Fragment { /// Returns true if any of the inline styles associated with this fragment have /// `vertical-align` set to `top` or `bottom`. pub fn is_vertically_aligned_to_top_or_bottom(&self) -> bool { - match self.style.get_box().vertical_align { - VerticalAlign::Top | VerticalAlign::Bottom => return true, - _ => {}, + fn is_top_or_bottom(v: &VerticalAlign) -> bool { + match *v { + VerticalAlign::Keyword(VerticalAlignKeyword::Top) | + VerticalAlign::Keyword(VerticalAlignKeyword::Bottom) => true, + _ => false, + } } + + if is_top_or_bottom(&self.style.get_box().vertical_align) { + return true; + } + if let Some(ref inline_context) = self.inline_context { for node in &inline_context.nodes { - match node.style.get_box().vertical_align { - VerticalAlign::Top | VerticalAlign::Bottom => return true, - _ => {}, + if is_top_or_bottom(&node.style.get_box().vertical_align) { + return true; } } } diff --git a/components/layout/inline.rs b/components/layout/inline.rs index bef95a64f96..f4dacefc5ef 100644 --- a/components/layout/inline.rs +++ b/components/layout/inline.rs @@ -41,7 +41,7 @@ use style::logical_geometry::{LogicalRect, LogicalSize, WritingMode}; use style::properties::ComputedValues; use style::servo::restyle_damage::ServoRestyleDamage; use style::values::computed::box_::VerticalAlign; -use style::values::generics::box_::VerticalAlign as GenericVerticalAlign; +use style::values::generics::box_::VerticalAlignKeyword; use style::values::specified::text::TextOverflowSide; use unicode_bidi as bidi; @@ -1269,13 +1269,13 @@ impl InlineFlow { let mut largest_block_size_for_top_fragments = Au(0); let mut largest_block_size_for_bottom_fragments = Au(0); - // We use `VerticalAlign::Baseline` here because `vertical-align` must + // We use `VerticalAlign::baseline()` here because `vertical-align` must // not apply to the inside of inline blocks. update_line_metrics_for_fragment( &mut line_metrics, &inline_metrics, style.get_box().display, - GenericVerticalAlign::Baseline, + VerticalAlign::baseline(), &mut largest_block_size_for_top_fragments, &mut largest_block_size_for_bottom_fragments, ); @@ -1322,11 +1322,20 @@ impl InlineFlow { largest_block_size_for_top_fragments: &mut Au, largest_block_size_for_bottom_fragments: &mut Au, ) { + // FIXME(emilio): This should probably be handled. + let vertical_align_value = match vertical_align_value { + VerticalAlign::Keyword(kw) => kw, + VerticalAlign::Length(..) => { + *line_metrics = line_metrics.new_metrics_for_fragment(inline_metrics); + return; + }, + }; + match (display_value, vertical_align_value) { - (Display::Inline, GenericVerticalAlign::Top) | - (Display::Block, GenericVerticalAlign::Top) | - (Display::InlineFlex, GenericVerticalAlign::Top) | - (Display::InlineBlock, GenericVerticalAlign::Top) + (Display::Inline, VerticalAlignKeyword::Top) | + (Display::Block, VerticalAlignKeyword::Top) | + (Display::InlineFlex, VerticalAlignKeyword::Top) | + (Display::InlineBlock, VerticalAlignKeyword::Top) if inline_metrics.space_above_baseline >= Au(0) => { *largest_block_size_for_top_fragments = max( @@ -1334,10 +1343,10 @@ impl InlineFlow { inline_metrics.space_above_baseline + inline_metrics.space_below_baseline, ) }, - (Display::Inline, GenericVerticalAlign::Bottom) | - (Display::Block, GenericVerticalAlign::Bottom) | - (Display::InlineFlex, GenericVerticalAlign::Bottom) | - (Display::InlineBlock, GenericVerticalAlign::Bottom) + (Display::Inline, VerticalAlignKeyword::Bottom) | + (Display::Block, VerticalAlignKeyword::Bottom) | + (Display::InlineFlex, VerticalAlignKeyword::Bottom) | + (Display::InlineBlock, VerticalAlignKeyword::Bottom) if inline_metrics.space_below_baseline >= Au(0) => { *largest_block_size_for_bottom_fragments = max( diff --git a/components/layout/table_cell.rs b/components/layout/table_cell.rs index 682a1c38ec4..ec79f9f4540 100644 --- a/components/layout/table_cell.rs +++ b/components/layout/table_cell.rs @@ -23,7 +23,7 @@ use style::logical_geometry::{LogicalMargin, LogicalRect, LogicalSize, WritingMo use style::properties::ComputedValues; use style::values::computed::length::Size; use style::values::computed::Color; -use style::values::generics::box_::VerticalAlign; +use style::values::generics::box_::{VerticalAlign, VerticalAlignKeyword}; use style::values::specified::BorderStyle; #[allow(unsafe_code)] @@ -138,11 +138,11 @@ impl TableCellFlow { self.block_flow.fragment.border_padding.block_start_end(); let kids_self_gap = self_size - kids_size; - // This offset should also account for VerticalAlign::Baseline. + // This offset should also account for VerticalAlign::baseline. // Need max cell ascent from the first row of this cell. let offset = match self.block_flow.fragment.style().get_box().vertical_align { - VerticalAlign::Middle => kids_self_gap / 2, - VerticalAlign::Bottom => kids_self_gap, + VerticalAlign::Keyword(VerticalAlignKeyword::Middle) => kids_self_gap / 2, + VerticalAlign::Keyword(VerticalAlignKeyword::Bottom) => kids_self_gap, _ => Au(0), }; if offset == Au(0) { diff --git a/components/layout/text.rs b/components/layout/text.rs index b26e531e670..199bd7e07f2 100644 --- a/components/layout/text.rs +++ b/components/layout/text.rs @@ -21,13 +21,13 @@ use std::collections::LinkedList; use std::mem; use std::sync::Arc; use style::computed_values::text_rendering::T as TextRendering; -use style::computed_values::text_transform::T as TextTransform; use style::computed_values::white_space::T as WhiteSpace; use style::computed_values::word_break::T as WordBreak; use style::logical_geometry::{LogicalSize, WritingMode}; use style::properties::style_structs::Font as FontStyleStruct; use style::properties::ComputedValues; use style::values::generics::text::LineHeight; +use style::values::specified::text::{TextTransform, TextTransformCase}; use unicode_bidi as bidi; use unicode_script::{get_script, Script}; use xi_unicode::LineBreakLeafIter; @@ -738,23 +738,23 @@ fn apply_style_transform_if_necessary( last_whitespace: bool, is_first_run: bool, ) { - match text_transform { - TextTransform::None => {}, - TextTransform::Uppercase => { + match text_transform.case_ { + TextTransformCase::None => {}, + TextTransformCase::Uppercase => { let original = string[first_character_position..].to_owned(); string.truncate(first_character_position); for ch in original.chars().flat_map(|ch| ch.to_uppercase()) { string.push(ch); } }, - TextTransform::Lowercase => { + TextTransformCase::Lowercase => { let original = string[first_character_position..].to_owned(); string.truncate(first_character_position); for ch in original.chars().flat_map(|ch| ch.to_lowercase()) { string.push(ch); } }, - TextTransform::Capitalize => { + TextTransformCase::Capitalize => { let original = string[first_character_position..].to_owned(); string.truncate(first_character_position); diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index 88eedef7d17..301a7500713 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -972,6 +972,11 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { } } + #[inline] + fn is_part(&self, _name: &Atom) -> bool { + false + } + #[inline] fn has_class(&self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool { unsafe { self.element.has_class_for_layout(name, case_sensitivity) } @@ -1484,6 +1489,12 @@ impl<'le> ::selectors::Element for ServoThreadSafeLayoutElement<'le> { false } + #[inline] + fn is_part(&self, _name: &Atom) -> bool { + debug!("ServoThreadSafeLayoutElement::is_part called"); + false + } + fn has_class(&self, _name: &Atom, _case_sensitivity: CaseSensitivity) -> bool { debug!("ServoThreadSafeLayoutElement::has_class called"); false diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 2d12e93774d..1b918c0537b 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -3075,6 +3075,10 @@ impl<'a> SelectorsElement for DomRoot { .map_or(false, |atom| case_sensitivity.eq_atom(id, atom)) } + fn is_part(&self, _name: &Atom) -> bool { + false + } + fn has_class(&self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool { Element::has_class(&**self, name, case_sensitivity) } From a598648d0d48e5630570c7ee83505bf26da9405f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 7 May 2019 12:54:42 +0200 Subject: [PATCH 25/27] style: Update test expectations. --- .../parsing/text-transform-valid.html.ini | 43 ------------------- .../wide-keyword-fallback.html.ini | 2 - 2 files changed, 45 deletions(-) delete mode 100644 tests/wpt/metadata/css/css-text/parsing/text-transform-valid.html.ini delete mode 100644 tests/wpt/metadata/css/css-variables/wide-keyword-fallback.html.ini diff --git a/tests/wpt/metadata/css/css-text/parsing/text-transform-valid.html.ini b/tests/wpt/metadata/css/css-text/parsing/text-transform-valid.html.ini deleted file mode 100644 index c9e0f5c67db..00000000000 --- a/tests/wpt/metadata/css/css-text/parsing/text-transform-valid.html.ini +++ /dev/null @@ -1,43 +0,0 @@ -[text-transform-valid.html] - [e.style['text-transform'\] = "full-size-kana full-width capitalize" should set the property value] - expected: FAIL - - [e.style['text-transform'\] = "full-width" should set the property value] - expected: FAIL - - [e.style['text-transform'\] = "capitalize full-width" should set the property value] - expected: FAIL - - [e.style['text-transform'\] = "full-size-kana full-width" should set the property value] - expected: FAIL - - [e.style['text-transform'\] = "capitalize full-width full-size-kana" should set the property value] - expected: FAIL - - [e.style['text-transform'\] = "full-width full-size-kana" should set the property value] - expected: FAIL - - [e.style['text-transform'\] = "full-size-kana lowercase full-width" should set the property value] - expected: FAIL - - [e.style['text-transform'\] = "full-size-kana capitalize" should set the property value] - expected: FAIL - - [e.style['text-transform'\] = "lowercase full-size-kana full-width" should set the property value] - expected: FAIL - - [e.style['text-transform'\] = "full-width lowercase" should set the property value] - expected: FAIL - - [e.style['text-transform'\] = "full-size-kana" should set the property value] - expected: FAIL - - [e.style['text-transform'\] = "full-width full-size-kana uppercase" should set the property value] - expected: FAIL - - [e.style['text-transform'\] = "uppercase full-size-kana" should set the property value] - expected: FAIL - - [e.style['text-transform'\] = "full-width uppercase full-size-kana" should set the property value] - expected: FAIL - diff --git a/tests/wpt/metadata/css/css-variables/wide-keyword-fallback.html.ini b/tests/wpt/metadata/css/css-variables/wide-keyword-fallback.html.ini deleted file mode 100644 index 3bdd09230d8..00000000000 --- a/tests/wpt/metadata/css/css-variables/wide-keyword-fallback.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[wide-keyword-fallback.html] - expected: FAIL From 0c982dcd1660efe09431ed5f39d87f7b6c4188ea Mon Sep 17 00:00:00 2001 From: Mats Palmgren Date: Mon, 6 May 2019 02:43:50 +0200 Subject: [PATCH 26/27] style: [css-grid-2] Remove single keyword 'subgrid' as a valid value for the 'grid' and 'grid-template' shorthands. Differential Revision: https://phabricator.services.mozilla.com/D29974 --- components/style/properties/shorthands/position.mako.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/components/style/properties/shorthands/position.mako.rs b/components/style/properties/shorthands/position.mako.rs index aaf97a0674c..6fdbe1235cf 100644 --- a/components/style/properties/shorthands/position.mako.rs +++ b/components/style/properties/shorthands/position.mako.rs @@ -254,7 +254,7 @@ use crate::parser::Parse; use servo_arc::Arc; use crate::values::{Either, None_}; - use crate::values::generics::grid::{LineNameList, TrackSize, TrackList, TrackListType}; + use crate::values::generics::grid::{TrackSize, TrackList, TrackListType}; use crate::values::generics::grid::{TrackListValue, concat_serialize_idents}; use crate::values::specified::{GridTemplateComponent, GenericGridTemplateComponent}; use crate::values::specified::grid::parse_line_names; @@ -265,12 +265,11 @@ context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result<(GridTemplateComponent, GridTemplateComponent, Either), ParseError<'i>> { - // Other shorthand sub properties also parse `none` and `subgrid` keywords and this - // shorthand should know after these keywords there is nothing to parse. Otherwise it - // gets confused and rejects the sub properties that contains `none` or `subgrid`. + // Other shorthand sub properties also parse the `none` keyword and this shorthand + // should know after this keyword there is nothing to parse. Otherwise it gets + // confused and rejects the sub properties that contains `none`. <% keywords = { "none": "GenericGridTemplateComponent::None", - "subgrid": "GenericGridTemplateComponent::Subgrid(LineNameList::default())" } %> % for keyword, rust_type in keywords.items(): From 1bb15d881951425dd62f5d5b3677a18d1019167d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 6 May 2019 21:31:16 +0000 Subject: [PATCH 27/27] style: scroll-snap-coordinate shouldn't use NotInitial. The initial value for this is indeed `none` (and thus empty). The Rust code was confused. This property is disabled by default these days, and I think the get_initial_value() function, which is what could get confused, is not called for this property, so I think this shouldn't be observable. Differential Revision: https://phabricator.services.mozilla.com/D30124 --- components/style/properties/longhands/box.mako.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/style/properties/longhands/box.mako.rs b/components/style/properties/longhands/box.mako.rs index 397393ed3d8..a7a2cb9e348 100644 --- a/components/style/properties/longhands/box.mako.rs +++ b/components/style/properties/longhands/box.mako.rs @@ -337,11 +337,11 @@ ${helpers.predefined_type( "Position", "computed::Position::zero()", vector=True, + allow_empty=True, products="gecko", gecko_pref="layout.css.scroll-snap.enabled", spec="Nonstandard (https://developer.mozilla.org/en-US/docs/Web/CSS/scroll-snap-destination)", animation_value_type="discrete", - allow_empty="NotInitial", )} <% transform_extra_prefixes = "moz:layout.css.prefixes.transforms webkit" %>