From d68d6f7c568e11518f90b3bb7c5a79a6ab031c34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 4 Nov 2019 16:55:33 +0000 Subject: [PATCH 01/45] style: Always restyle / repaint when a visited query finishes. Differential Revision: https://phabricator.services.mozilla.com/D50810 --- .../style/invalidation/element/state_and_attributes.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/components/style/invalidation/element/state_and_attributes.rs b/components/style/invalidation/element/state_and_attributes.rs index 30521fb8c00..c4671b0011c 100644 --- a/components/style/invalidation/element/state_and_attributes.rs +++ b/components/style/invalidation/element/state_and_attributes.rs @@ -158,6 +158,14 @@ where // If we the visited state changed, we force a restyle here. Matching // doesn't depend on the actual visited state at all, so we can't look // at matching results to decide what to do for this case. + // + // TODO(emilio): This should be contains(), to avoid doing subtree + // restyles when adding or removing an href attribute, but invalidation + // for that case is broken right now (bug 1591987). + // + // This piece of code should be removed when + // layout.css.always-repaint-on-unvisited is true, since we cannot get + // into this situation in that case. if state_changes.intersects(ElementState::IN_VISITED_OR_UNVISITED_STATE) { trace!(" > visitedness change, force subtree restyle"); // We can't just return here because there may also be attribute From d797b0e47523fe7326abe874cf2e77b460f98c75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 5 Nov 2019 10:24:36 +0000 Subject: [PATCH 02/45] style: Fix ElementWrapper::is_link. And do a full restyle only when the state goes from visited to unvisited or vice versa. That is, use regular invalidation for addition or removals of href attributes, for example. Differential Revision: https://phabricator.services.mozilla.com/D50821 --- components/style/gecko/wrapper.rs | 3 +-- components/style/invalidation/element/element_wrapper.rs | 5 ++++- .../style/invalidation/element/state_and_attributes.rs | 8 ++------ 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index c2c4a0c0feb..da989e5a74a 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -2191,8 +2191,7 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { #[inline] fn is_link(&self) -> bool { - self.state() - .intersects(NonTSPseudoClass::AnyLink.state_flag()) + self.state().intersects(ElementState::IN_VISITED_OR_UNVISITED_STATE) } #[inline] diff --git a/components/style/invalidation/element/element_wrapper.rs b/components/style/invalidation/element/element_wrapper.rs index cb63e611348..b8e29095d24 100644 --- a/components/style/invalidation/element/element_wrapper.rs +++ b/components/style/invalidation/element/element_wrapper.rs @@ -270,7 +270,10 @@ where } fn is_link(&self) -> bool { - self.element.is_link() + match self.snapshot().and_then(|s| s.state()) { + Some(state) => state.intersects(ElementState::IN_VISITED_OR_UNVISITED_STATE), + None => self.element.is_link(), + } } fn opaque(&self) -> OpaqueElement { diff --git a/components/style/invalidation/element/state_and_attributes.rs b/components/style/invalidation/element/state_and_attributes.rs index c4671b0011c..cb3a5525e39 100644 --- a/components/style/invalidation/element/state_and_attributes.rs +++ b/components/style/invalidation/element/state_and_attributes.rs @@ -159,14 +159,10 @@ where // doesn't depend on the actual visited state at all, so we can't look // at matching results to decide what to do for this case. // - // TODO(emilio): This should be contains(), to avoid doing subtree - // restyles when adding or removing an href attribute, but invalidation - // for that case is broken right now (bug 1591987). - // - // This piece of code should be removed when + // TODO(emilio): This piece of code should be removed when // layout.css.always-repaint-on-unvisited is true, since we cannot get // into this situation in that case. - if state_changes.intersects(ElementState::IN_VISITED_OR_UNVISITED_STATE) { + if state_changes.contains(ElementState::IN_VISITED_OR_UNVISITED_STATE) { trace!(" > visitedness change, force subtree restyle"); // We can't just return here because there may also be attribute // changes as well that imply additional hints for siblings. From 9c1dd4b3ead24cbaecb6650ee509d167c74b0d32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 6 Nov 2019 05:54:17 +0000 Subject: [PATCH 03/45] style: Make zoom: 0 mean the same as zoom: 1. This matches the WebKit implementation, and is clearly a violation of the rules we generally use for ranges in CSS. But it seems to be depended-on legacy behavior, see the linked WebKit bug, this bug, and bug 1593317. Differential Revision: https://phabricator.services.mozilla.com/D51539 --- components/style/properties/shorthands/box.mako.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/components/style/properties/shorthands/box.mako.rs b/components/style/properties/shorthands/box.mako.rs index 7b4ce131922..351b8cec606 100644 --- a/components/style/properties/shorthands/box.mako.rs +++ b/components/style/properties/shorthands/box.mako.rs @@ -464,11 +464,13 @@ ${helpers.two_properties_shorthand( }; // Make sure that the initial value matches the values for the - // longhands, just for general sanity. + // longhands, just for general sanity. `zoom: 1` and `zoom: 0` are + // ignored, see [1][2]. They are just hack for the "has layout" mode on + // IE. // - // FIXME: Should we just do this for the "normal" case? Seems weird - // either way, so maybe not? - Ok(if zoom.get() == 1.0 { + // [1]: https://bugs.webkit.org/show_bug.cgi?id=18467 + // [2]: https://bugzilla.mozilla.org/show_bug.cgi?id=1593009 + Ok(if zoom.get() == 1.0 || zoom.get() == 0.0 { expanded! { transform: Transform::none(), transform_origin: TransformOrigin::initial_value(), From cceb0bcb7358464a4a1c51731c3dc9b4c38c37b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 7 Nov 2019 11:19:23 +0000 Subject: [PATCH 04/45] style: Simplify code for keeping alive shared memory until all sheets go away. The existing code wasn't sound, as CSSOM objects also needed to go away before the shared memory goes away (as they keep references to them). This is sound assuming no presence of reference cycles introduced by CSSOM. We may want to live with this and rely on chrome code not writing cycles like this with UA stylesheet DOM objects. We could explicitly drop all potentially-static objects... That seems pretty error prone though. Or we could also just leak the shared memory buffer, is there any reason why we may not want to do that? Differential Revision: https://phabricator.services.mozilla.com/D51870 --- components/servo_arc/lib.rs | 13 +++++++++---- components/style/stylesheets/stylesheet.rs | 4 ++++ 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/components/servo_arc/lib.rs b/components/servo_arc/lib.rs index ad4489c10bc..bd6b5289504 100644 --- a/components/servo_arc/lib.rs +++ b/components/servo_arc/lib.rs @@ -485,6 +485,14 @@ impl Arc { } } + /// Whether or not the `Arc` is a static reference. + #[inline] + pub fn is_static(&self) -> bool { + // Using a relaxed ordering to check for STATIC_REFCOUNT is safe, since + // `count` never changes between STATIC_REFCOUNT and other values. + self.inner().count.load(Relaxed) == STATIC_REFCOUNT + } + /// Whether or not the `Arc` is uniquely owned (is the refcount 1?) and not /// a static reference. #[inline] @@ -501,10 +509,7 @@ impl Drop for Arc { fn drop(&mut self) { // NOTE(emilio): If you change anything here, make sure that the // implementation in layout/style/ServoStyleConstsInlines.h matches! - // - // Using a relaxed ordering to check for STATIC_REFCOUNT is safe, since - // `count` never changes between STATIC_REFCOUNT and other values. - if self.inner().count.load(Relaxed) == STATIC_REFCOUNT { + if self.is_static() { return; } diff --git a/components/style/stylesheets/stylesheet.rs b/components/style/stylesheets/stylesheet.rs index 15c60402be4..f5194d7d443 100644 --- a/components/style/stylesheets/stylesheet.rs +++ b/components/style/stylesheets/stylesheet.rs @@ -124,6 +124,7 @@ impl StylesheetContents { url_data: UrlExtraData, quirks_mode: QuirksMode, ) -> Self { + debug_assert!(rules.is_static()); Self { rules, origin, @@ -144,6 +145,9 @@ impl StylesheetContents { /// Measure heap usage. #[cfg(feature = "gecko")] pub fn size_of(&self, guard: &SharedRwLockReadGuard, ops: &mut MallocSizeOfOps) -> usize { + if self.rules.is_static() { + return 0; + } // Measurement of other fields may be added later. self.rules.unconditional_shallow_size_of(ops) + self.rules.read_with(guard).size_of(guard, ops) From c04a37982c4a00c1ea521efca44eb19548f63755 Mon Sep 17 00:00:00 2001 From: Tim Nguyen Date: Tue, 12 Nov 2019 19:50:19 +0000 Subject: [PATCH 05/45] style: Remove nsStackFrame platform code. Differential Revision: https://phabricator.services.mozilla.com/D49487 --- components/style/properties/longhands/xul.mako.rs | 10 ---------- components/style/values/specified/box.rs | 6 ------ 2 files changed, 16 deletions(-) diff --git a/components/style/properties/longhands/xul.mako.rs b/components/style/properties/longhands/xul.mako.rs index 0862ce533d4..d5c4011eaf6 100644 --- a/components/style/properties/longhands/xul.mako.rs +++ b/components/style/properties/longhands/xul.mako.rs @@ -64,16 +64,6 @@ ${helpers.single_keyword( spec="Nonstandard (https://developer.mozilla.org/en-US/docs/Web/CSS/box-pack)", )} -${helpers.single_keyword( - "-moz-stack-sizing", - "stretch-to-fit ignore ignore-horizontal ignore-vertical", - engines="gecko", - gecko_ffi_name="mStackSizing", - gecko_enum_prefix="StyleStackSizing", - animation_value_type="discrete", - spec="Nonstandard (https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-stack-sizing)", -)} - ${helpers.predefined_type( "-moz-box-ordinal-group", "Integer", diff --git a/components/style/values/specified/box.rs b/components/style/values/specified/box.rs index 58d51f8c006..2b4e3a420de 100644 --- a/components/style/values/specified/box.rs +++ b/components/style/values/specified/box.rs @@ -109,8 +109,6 @@ pub enum DisplayInside { #[cfg(feature = "gecko")] MozGridLine, #[cfg(feature = "gecko")] - MozStack, - #[cfg(feature = "gecko")] MozDeck, #[cfg(feature = "gecko")] MozGroupbox, @@ -243,8 +241,6 @@ impl Display { #[cfg(feature = "gecko")] pub const MozGridLine: Self = Self::new(DisplayOutside::XUL, DisplayInside::MozGridLine); #[cfg(feature = "gecko")] - pub const MozStack: Self = Self::new(DisplayOutside::XUL, DisplayInside::MozStack); - #[cfg(feature = "gecko")] pub const MozDeck: Self = Self::new(DisplayOutside::XUL, DisplayInside::MozDeck); #[cfg(feature = "gecko")] pub const MozGroupbox: Self = Self::new(DisplayOutside::XUL, DisplayInside::MozGroupbox); @@ -669,8 +665,6 @@ impl Parse for Display { #[cfg(feature = "gecko")] "-moz-grid-line" if moz_display_values_enabled(context) => Display::MozGridLine, #[cfg(feature = "gecko")] - "-moz-stack" if moz_display_values_enabled(context) => Display::MozStack, - #[cfg(feature = "gecko")] "-moz-deck" if moz_display_values_enabled(context) => Display::MozDeck, #[cfg(feature = "gecko")] "-moz-groupbox" if moz_display_values_enabled(context) => Display::MozGroupbox, From c28ca7b405da7e47585037df13a8baa2fff8bb5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 12 Nov 2019 20:30:42 +0000 Subject: [PATCH 06/45] style: Use MaybeUninit in style struct clone impls / constructors. Differential Revision: https://phabricator.services.mozilla.com/D51788 --- components/style/properties/gecko.mako.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 7783c7eebd3..b6cd741038c 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -46,7 +46,7 @@ use crate::rule_tree::StrongRuleNode; use crate::selector_parser::PseudoElement; use servo_arc::{Arc, RawOffsetArc, UniqueArc}; use std::marker::PhantomData; -use std::mem::{forget, zeroed, ManuallyDrop}; +use std::mem::{forget, MaybeUninit}; use std::{cmp, ops, ptr}; use crate::values::{self, CustomIdent, Either, KeyframesName, None_}; use crate::values::computed::{Percentage, TransitionProperty}; @@ -218,7 +218,7 @@ impl ComputedValuesInner { unsafe { let mut arc = UniqueArc::::new_uninit(); bindings::Gecko_ComputedStyle_Init( - &mut (*arc.as_mut_ptr()).0, + arc.as_mut_ptr() as *mut _, &self, pseudo_ty, ); @@ -621,14 +621,17 @@ def set_gecko_property(ffi_name, expr): impl ${style_struct.gecko_struct_name} { #[allow(dead_code, unused_variables)] pub fn default(document: &structs::Document) -> Arc { - let mut result = Arc::new(${style_struct.gecko_struct_name} { gecko: ManuallyDrop::new(unsafe { zeroed() }) }); unsafe { + let mut result = UniqueArc::::new_uninit(); + // FIXME(bug 1595895): Zero the memory to keep valgrind happy, but + // these looks like Valgrind false-positives at a quick glance. + ptr::write_bytes::(result.as_mut_ptr(), 0, 1); Gecko_Construct_Default_${style_struct.gecko_ffi_name}( - &mut *Arc::get_mut(&mut result).unwrap().gecko, + result.as_mut_ptr() as *mut _, document, ); + UniqueArc::assume_init(result).shareable() } - result } } impl Drop for ${style_struct.gecko_struct_name} { @@ -641,9 +644,12 @@ impl Drop for ${style_struct.gecko_struct_name} { impl Clone for ${style_struct.gecko_struct_name} { fn clone(&self) -> Self { unsafe { - let mut result = ${style_struct.gecko_struct_name} { gecko: ManuallyDrop::new(zeroed()) }; - Gecko_CopyConstruct_${style_struct.gecko_ffi_name}(&mut *result.gecko, &*self.gecko); - result + let mut result = MaybeUninit::::uninit(); + // FIXME(bug 1595895): Zero the memory to keep valgrind happy, but + // these looks like Valgrind false-positives at a quick glance. + ptr::write_bytes::(result.as_mut_ptr(), 0, 1); + Gecko_CopyConstruct_${style_struct.gecko_ffi_name}(result.as_mut_ptr() as *mut _, &*self.gecko); + result.assume_init() } } } From ff1a9dbb2eece82f113f756bb0b380affd55bb73 Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Thu, 14 Nov 2019 04:59:56 +0000 Subject: [PATCH 07/45] style: Update some dependencies. Differential Revision: https://phabricator.services.mozilla.com/D49458 --- components/style/Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/style/Cargo.toml b/components/style/Cargo.toml index f15c64e2f56..b068e0f5899 100644 --- a/components/style/Cargo.toml +++ b/components/style/Cargo.toml @@ -28,7 +28,7 @@ gecko_profiler = [] [dependencies] app_units = "0.7" -arrayvec = "0.4.6" +arrayvec = "0.5" atomic_refcell = "0.1" bitflags = "1.0" byteorder = "1.0" @@ -74,7 +74,7 @@ thin-slice = "0.1.0" to_shmem = {path = "../to_shmem"} to_shmem_derive = {path = "../to_shmem_derive"} time = "0.1" -uluru = "0.3" +uluru = "0.4" unicode-bidi = "0.3" unicode-segmentation = "1.0" void = "1.0.2" From 7fec79ffcf534494f5be699f3302f3a8a0fef009 Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Thu, 14 Nov 2019 06:00:05 +0000 Subject: [PATCH 08/45] style: Factor dynamic toolbar max height into layout metrics. Now * nsPresContext::mVisibleArea is excluding the toolbar max height so that ICB is now static regardless of the dynamic toolbar transition * nsPresContext::mSizeForViewportUnits is introduced to resolve viewport units which is including the toolbar max height That means that with the dynamic toolbar max height; mVisibleArea < mSizeForViewportUnits See https://github.com/bokand/URLBarSizing for more detail backgrounds of this change. Depends on D50417 Differential Revision: https://phabricator.services.mozilla.com/D50418 --- components/style/gecko/media_queries.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index bb126e16948..2e68a889df1 100644 --- a/components/style/gecko/media_queries.rs +++ b/components/style/gecko/media_queries.rs @@ -237,7 +237,13 @@ impl Device { /// used for viewport unit resolution. pub fn au_viewport_size_for_viewport_unit_resolution(&self) -> Size2D { self.used_viewport_size.store(true, Ordering::Relaxed); - self.au_viewport_size() + + let pc = match self.pres_context() { + Some(pc) => pc, + None => return Size2D::new(Au(0), Au(0)), + }; + let size = &pc.mSizeForViewportUnits; + Size2D::new(Au(size.width), Au(size.height)) } /// Returns whether we ever looked up the viewport size of the Device. From 9bdad610ff7120d254a7472898bad0615aa22254 Mon Sep 17 00:00:00 2001 From: Sam Mauldin Date: Thu, 14 Nov 2019 00:17:23 +0000 Subject: [PATCH 09/45] style: Remove SpecialColorKeyword enum and merge into SystemColor. Move all the entires of SpecialColorKeyword into SystemColor and rearrange their computation to match. Add the new SystemColor entries into the property list of nsXPLookAndFeel. Differential Revision: https://phabricator.services.mozilla.com/D50903 --- components/style/values/specified/color.rs | 61 ++++++++-------------- 1 file changed, 23 insertions(+), 38 deletions(-) diff --git a/components/style/values/specified/color.rs b/components/style/values/specified/color.rs index 01e5890d015..ddbdb5f4cdf 100644 --- a/components/style/values/specified/color.rs +++ b/components/style/values/specified/color.rs @@ -36,9 +36,6 @@ pub enum Color { /// A system color #[cfg(feature = "gecko")] System(SystemColor), - /// A special color keyword value used in Gecko - #[cfg(feature = "gecko")] - Special(gecko::SpecialColorKeyword), /// Quirksmode-only rule for inheriting color from the body #[cfg(feature = "gecko")] InheritFromBodyQuirk, @@ -143,6 +140,8 @@ pub enum SystemColor { Windowframe, Windowtext, MozButtondefault, + MozDefaultColor, + MozDefaultBackgroundColor, MozDialog, MozDialogtext, /// Used to highlight valid regions to drop something onto. @@ -231,6 +230,10 @@ pub enum SystemColor { /// colors. MozNativehyperlinktext, + MozHyperlinktext, + MozActivehyperlinktext, + MozVisitedhyperlinktext, + /// Combobox widgets MozComboboxtext, MozCombobox, @@ -246,24 +249,24 @@ impl SystemColor { #[inline] fn compute(&self, cx: &Context) -> ComputedColor { use crate::gecko_bindings::bindings; - unsafe { - convert_nscolor_to_computedcolor(bindings::Gecko_GetLookAndFeelSystemColor( - *self as i32, - cx.device().document(), - )) - } - } -} -#[cfg(feature = "gecko")] -mod gecko { - #[derive(Clone, Copy, Debug, Eq, Hash, MallocSizeOf, Parse, PartialEq, ToCss, ToShmem)] - pub enum SpecialColorKeyword { - MozDefaultColor, - MozDefaultBackgroundColor, - MozHyperlinktext, - MozActivehyperlinktext, - MozVisitedhyperlinktext, + let prefs = cx.device().pref_sheet_prefs(); + + convert_nscolor_to_computedcolor(match *self { + SystemColor::MozDefaultColor => prefs.mDefaultColor, + SystemColor::MozDefaultBackgroundColor => prefs.mDefaultBackgroundColor, + SystemColor::MozHyperlinktext => prefs.mLinkColor, + SystemColor::MozActivehyperlinktext => prefs.mActiveLinkColor, + SystemColor::MozVisitedhyperlinktext => prefs.mVisitedLinkColor, + + _ => unsafe { + bindings::Gecko_GetLookAndFeelSystemColor( + *self as i32, + cx.device().document() + ) + } + }) + } } @@ -364,10 +367,6 @@ impl Parse for Color { if let Ok(system) = input.try(|i| SystemColor::parse(context, i)) { return Ok(Color::System(system)); } - - if let Ok(c) = input.try(gecko::SpecialColorKeyword::parse) { - return Ok(Color::Special(c)); - } } match e.kind { @@ -401,8 +400,6 @@ impl ToCss for Color { #[cfg(feature = "gecko")] Color::System(system) => system.to_css(dest), #[cfg(feature = "gecko")] - Color::Special(special) => special.to_css(dest), - #[cfg(feature = "gecko")] Color::InheritFromBodyQuirk => Ok(()), } } @@ -553,18 +550,6 @@ impl Color { #[cfg(feature = "gecko")] Color::System(system) => system.compute(_context?), #[cfg(feature = "gecko")] - Color::Special(special) => { - use self::gecko::SpecialColorKeyword as Keyword; - let prefs = _context?.device().pref_sheet_prefs(); - convert_nscolor_to_computedcolor(match special { - Keyword::MozDefaultColor => prefs.mDefaultColor, - Keyword::MozDefaultBackgroundColor => prefs.mDefaultBackgroundColor, - Keyword::MozHyperlinktext => prefs.mLinkColor, - Keyword::MozActivehyperlinktext => prefs.mActiveLinkColor, - Keyword::MozVisitedhyperlinktext => prefs.mVisitedLinkColor, - }) - }, - #[cfg(feature = "gecko")] Color::InheritFromBodyQuirk => { ComputedColor::rgba(_context?.device().body_text_color()) }, From 28110c060f1210bdb5da5155e45a5e87aa545612 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 13 Nov 2019 09:58:52 +0000 Subject: [PATCH 10/45] style: Split collect_style_attribute_and_animation_rules. Animations are their own cascade level per https://drafts.csswg.org/css-cascade-4/#cascade-origin Differential Revision: https://phabricator.services.mozilla.com/D52575 --- components/style/rule_collector.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/components/style/rule_collector.rs b/components/style/rule_collector.rs index f536b80b5c5..dd50d846715 100644 --- a/components/style/rule_collector.rs +++ b/components/style/rule_collector.rs @@ -377,7 +377,7 @@ where } } - fn collect_style_attribute_and_animation_rules(&mut self) { + fn collect_style_attribute(&mut self) { if let Some(sa) = self.style_attribute { self.rules .push(ApplicableDeclarationBlock::from_declarations( @@ -385,7 +385,9 @@ where CascadeLevel::StyleAttributeNormal, )); } + } + fn collect_animation_rules(&mut self) { if let Some(so) = self.smil_override { self.rules .push(ApplicableDeclarationBlock::from_declarations( @@ -436,6 +438,7 @@ where self.collect_normal_rules_from_containing_shadow_tree(); self.collect_document_author_rules(); self.collect_part_rules(); - self.collect_style_attribute_and_animation_rules(); + self.collect_style_attribute(); + self.collect_animation_rules(); } } From 349492b5e2754e2cff9d0a353cf0e34adb7d5772 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 14 Nov 2019 02:49:54 +0000 Subject: [PATCH 11/45] style: Fix cascade order of shadow parts. This moves the shadow cascade order into the cascade level, and refactors the code a bit for that. Differential Revision: https://phabricator.services.mozilla.com/D49988 --- components/style/applicable_declarations.rs | 83 +---- components/style/gecko/pseudo_element.rs | 6 + components/style/matching.rs | 4 +- components/style/properties/cascade.rs | 14 +- components/style/rule_collector.rs | 51 ++- components/style/rule_tree/mod.rs | 336 ++++++++++---------- components/style/selector_map.rs | 12 +- components/style/servo/selector_parser.rs | 6 + components/style/stylist.rs | 7 +- 9 files changed, 237 insertions(+), 282 deletions(-) diff --git a/components/style/applicable_declarations.rs b/components/style/applicable_declarations.rs index e4bf996a1d6..ab83e5737a9 100644 --- a/components/style/applicable_declarations.rs +++ b/components/style/applicable_declarations.rs @@ -5,11 +5,10 @@ //! Applicable declarations management. use crate::properties::PropertyDeclarationBlock; -use crate::rule_tree::{CascadeLevel, ShadowCascadeOrder, StyleSource}; +use crate::rule_tree::{CascadeLevel, StyleSource}; use crate::shared_lock::Locked; use servo_arc::Arc; use smallvec::SmallVec; -use std::fmt::{self, Debug}; /// List of applicable declarations. This is a transient structure that shuttles /// declarations between selector matching and inserting into the rule tree, and @@ -20,75 +19,27 @@ use std::fmt::{self, Debug}; /// However, it may depend a lot on workload, and stack space is cheap. pub type ApplicableDeclarationList = SmallVec<[ApplicableDeclarationBlock; 16]>; -/// Blink uses 18 bits to store source order, and does not check overflow [1]. -/// That's a limit that could be reached in realistic webpages, so we use -/// 24 bits and enforce defined behavior in the overflow case. -/// -/// [1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/ -/// RuleSet.h?l=128&rcl=90140ab80b84d0f889abc253410f44ed54ae04f3 -const SOURCE_ORDER_SHIFT: usize = 0; -const SOURCE_ORDER_BITS: usize = 24; -const SOURCE_ORDER_MAX: u32 = (1 << SOURCE_ORDER_BITS) - 1; -const SOURCE_ORDER_MASK: u32 = SOURCE_ORDER_MAX << SOURCE_ORDER_SHIFT; - -/// We store up-to-15 shadow order levels. -/// -/// You'd need an element slotted across 16 components with ::slotted rules to -/// trigger this as of this writing, which looks... Unlikely. -const SHADOW_CASCADE_ORDER_SHIFT: usize = SOURCE_ORDER_BITS; -const SHADOW_CASCADE_ORDER_BITS: usize = 4; -const SHADOW_CASCADE_ORDER_MAX: u8 = (1 << SHADOW_CASCADE_ORDER_BITS) - 1; -const SHADOW_CASCADE_ORDER_MASK: u32 = - (SHADOW_CASCADE_ORDER_MAX as u32) << SHADOW_CASCADE_ORDER_SHIFT; - -const CASCADE_LEVEL_SHIFT: usize = SOURCE_ORDER_BITS + SHADOW_CASCADE_ORDER_BITS; -const CASCADE_LEVEL_BITS: usize = 4; -const CASCADE_LEVEL_MAX: u8 = (1 << CASCADE_LEVEL_BITS) - 1; -const CASCADE_LEVEL_MASK: u32 = (CASCADE_LEVEL_MAX as u32) << CASCADE_LEVEL_SHIFT; - /// Stores the source order of a block, the cascade level it belongs to, and the /// counter needed to handle Shadow DOM cascade order properly. -#[derive(Clone, Copy, Eq, MallocSizeOf, PartialEq)] -struct ApplicableDeclarationBits(u32); +/// +/// FIXME(emilio): Optimize storage. +#[derive(Clone, Copy, Eq, MallocSizeOf, PartialEq, Debug)] +struct ApplicableDeclarationBits { + source_order: u32, + cascade_level: CascadeLevel, +} impl ApplicableDeclarationBits { - fn new( - source_order: u32, - cascade_level: CascadeLevel, - shadow_cascade_order: ShadowCascadeOrder, - ) -> Self { - debug_assert!( - cascade_level as u8 <= CASCADE_LEVEL_MAX, - "Gotta find more bits!" - ); - let mut bits = ::std::cmp::min(source_order, SOURCE_ORDER_MAX); - bits |= ((shadow_cascade_order & SHADOW_CASCADE_ORDER_MAX) as u32) << - SHADOW_CASCADE_ORDER_SHIFT; - bits |= (cascade_level as u8 as u32) << CASCADE_LEVEL_SHIFT; - ApplicableDeclarationBits(bits) + fn new(source_order: u32, cascade_level: CascadeLevel) -> Self { + Self { source_order, cascade_level } } fn source_order(&self) -> u32 { - (self.0 & SOURCE_ORDER_MASK) >> SOURCE_ORDER_SHIFT - } - - fn shadow_cascade_order(&self) -> ShadowCascadeOrder { - ((self.0 & SHADOW_CASCADE_ORDER_MASK) >> SHADOW_CASCADE_ORDER_SHIFT) as ShadowCascadeOrder + self.source_order } fn level(&self) -> CascadeLevel { - let byte = ((self.0 & CASCADE_LEVEL_MASK) >> CASCADE_LEVEL_SHIFT) as u8; - unsafe { CascadeLevel::from_byte(byte) } - } -} - -impl Debug for ApplicableDeclarationBits { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.debug_struct("ApplicableDeclarationBits") - .field("source_order", &self.source_order()) - .field("shadow_cascade_order", &self.shadow_cascade_order()) - .field("level", &self.level()) - .finish() + self.cascade_level } } @@ -119,7 +70,7 @@ impl ApplicableDeclarationBlock { ) -> Self { ApplicableDeclarationBlock { source: StyleSource::from_declarations(declarations), - bits: ApplicableDeclarationBits::new(0, level, 0), + bits: ApplicableDeclarationBits::new(0, level), specificity: 0, } } @@ -131,11 +82,10 @@ impl ApplicableDeclarationBlock { order: u32, level: CascadeLevel, specificity: u32, - shadow_cascade_order: ShadowCascadeOrder, ) -> Self { ApplicableDeclarationBlock { source, - bits: ApplicableDeclarationBits::new(order, level, shadow_cascade_order), + bits: ApplicableDeclarationBits::new(order, level), specificity, } } @@ -155,9 +105,8 @@ impl ApplicableDeclarationBlock { /// Convenience method to consume self and return the right thing for the /// rule tree to iterate over. #[inline] - pub fn for_rule_tree(self) -> (StyleSource, CascadeLevel, ShadowCascadeOrder) { + pub fn for_rule_tree(self) -> (StyleSource, CascadeLevel) { let level = self.level(); - let cascade_order = self.bits.shadow_cascade_order(); - (self.source, level, cascade_order) + (self.source, level) } } diff --git a/components/style/gecko/pseudo_element.rs b/components/style/gecko/pseudo_element.rs index aa849ceee02..8d05dd9bbb1 100644 --- a/components/style/gecko/pseudo_element.rs +++ b/components/style/gecko/pseudo_element.rs @@ -140,6 +140,12 @@ impl PseudoElement { *self == PseudoElement::FieldsetContent } + /// Whether this pseudo-element is the ::-moz-color-swatch pseudo. + #[inline] + pub fn is_color_swatch(&self) -> bool { + *self == PseudoElement::MozColorSwatch + } + /// Whether this pseudo-element is lazily-cascaded. #[inline] pub fn is_lazy(&self) -> bool { diff --git a/components/style/matching.rs b/components/style/matching.rs index 958ec00f1d9..01246ed3878 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -137,12 +137,12 @@ trait PrivateMatchMethods: TElement { if replacements.contains(RestyleHint::RESTYLE_STYLE_ATTRIBUTE) { let style_attribute = self.style_attribute(); result |= replace_rule_node( - CascadeLevel::StyleAttributeNormal, + CascadeLevel::same_tree_author_normal(), style_attribute, primary_rules, ); result |= replace_rule_node( - CascadeLevel::StyleAttributeImportant, + CascadeLevel::same_tree_author_important(), style_attribute, primary_rules, ); diff --git a/components/style/properties/cascade.rs b/components/style/properties/cascade.rs index 5266ed78356..5dc73b9abc1 100644 --- a/components/style/properties/cascade.rs +++ b/components/style/properties/cascade.rs @@ -346,16 +346,10 @@ fn should_ignore_declaration_when_ignoring_document_colors( return false; } - let is_style_attribute = matches!( - cascade_level, - CascadeLevel::StyleAttributeNormal | CascadeLevel::StyleAttributeImportant - ); - - // Don't override colors on pseudo-element's style attributes. The - // background-color on ::-moz-color-swatch is an example. Those are set - // as an author style (via the style attribute), but it's pretty - // important for it to show up for obvious reasons :) - if pseudo.is_some() && is_style_attribute { + // Don't override background-color on ::-moz-color-swatch. It is set as an + // author style (via the style attribute), but it's pretty important for it + // to show up for obvious reasons :) + if pseudo.map_or(false, |p| p.is_color_swatch()) && longhand_id == LonghandId::BackgroundColor { return false; } diff --git a/components/style/rule_collector.rs b/components/style/rule_collector.rs index dd50d846715..031a92ca728 100644 --- a/components/style/rule_collector.rs +++ b/components/style/rule_collector.rs @@ -79,7 +79,6 @@ where rules: &'a mut ApplicableDeclarationList, context: &'a mut MatchingContext<'b, E::Impl>, flags_setter: &'a mut F, - shadow_cascade_order: ShadowCascadeOrder, matches_user_and_author_rules: bool, matches_document_author_rules: bool, } @@ -132,7 +131,6 @@ where context, flags_setter, rules, - shadow_cascade_order: 0, matches_user_and_author_rules, matches_document_author_rules: matches_user_and_author_rules, } @@ -142,7 +140,7 @@ where let cascade_level = match origin { Origin::UserAgent => CascadeLevel::UANormal, Origin::User => CascadeLevel::UserNormal, - Origin::Author => CascadeLevel::SameTreeAuthorNormal, + Origin::Author => CascadeLevel::same_tree_author_normal(), }; let cascade_data = self.stylist.cascade_data().borrow_for_origin(origin); @@ -198,7 +196,6 @@ where ) { debug_assert!(shadow_host.shadow_root().is_some()); self.collect_rules_internal(Some(shadow_host), map, cascade_level); - self.shadow_cascade_order += 1; } #[inline] @@ -212,7 +209,6 @@ where let rule_hash_target = self.rule_hash_target; let rules = &mut self.rules; let flags_setter = &mut self.flags_setter; - let shadow_cascade_order = self.shadow_cascade_order; let start = rules.len(); self.context.with_shadow_host(shadow_host, |context| { map.get_all_matching_rules( @@ -222,16 +218,18 @@ where context, flags_setter, cascade_level, - shadow_cascade_order, ); }); sort_rules_from(rules, start); } - /// Collects the rules for the ::slotted pseudo-element. - fn collect_slotted_rules(&mut self) { + /// Collects the rules for the ::slotted pseudo-element and the :host + /// pseudo-class. + fn collect_host_and_slotted_rules(&mut self) { let mut slots = SmallVec::<[_; 3]>::new(); let mut current = self.rule_hash_target.assigned_slot(); + let mut shadow_cascade_order = ShadowCascadeOrder::for_outermost_shadow_tree(); + while let Some(slot) = current { debug_assert!( self.matches_user_and_author_rules, @@ -239,11 +237,16 @@ where ); slots.push(slot); current = slot.assigned_slot(); + shadow_cascade_order.dec(); } + self.collect_host_rules(shadow_cascade_order); + // Match slotted rules in reverse order, so that the outer slotted rules // come before the inner rules (and thus have less priority). for slot in slots.iter().rev() { + shadow_cascade_order.inc(); + let shadow = slot.containing_shadow().unwrap(); let data = match shadow.style_data() { Some(d) => d, @@ -253,10 +256,11 @@ where Some(r) => r, None => continue, }; + self.collect_rules_in_shadow_tree( shadow.host(), slotted_rules, - CascadeLevel::InnerShadowNormal, + CascadeLevel::AuthorNormal { shadow_cascade_order }, ); } } @@ -277,12 +281,12 @@ where let cascade_data = containing_shadow.style_data(); let host = containing_shadow.host(); if let Some(map) = cascade_data.and_then(|data| data.normal_rules(self.pseudo_element)) { - self.collect_rules_in_shadow_tree(host, map, CascadeLevel::SameTreeAuthorNormal); + self.collect_rules_in_shadow_tree(host, map, CascadeLevel::same_tree_author_normal()); } } /// Collects the rules for the :host pseudo-class. - fn collect_host_rules(&mut self) { + fn collect_host_rules(&mut self, shadow_cascade_order: ShadowCascadeOrder) { let shadow = match self.rule_hash_target.shadow_root() { Some(s) => s, None => return, @@ -307,7 +311,7 @@ where self.collect_rules_in_shadow_tree( rule_hash_target, host_rules, - CascadeLevel::InnerShadowNormal, + CascadeLevel::AuthorNormal { shadow_cascade_order }, ); } @@ -342,21 +346,18 @@ where .part_rules(self.pseudo_element), }; - // TODO(emilio): SameTreeAuthorNormal is a bit of a lie here, we may - // need an OuterTreeAuthorNormal cascade level or such, and change the - // cascade order, if we allow to forward parts to even outer trees. - // - // Though the current thing kinda works because we apply them after - // the outer tree, so as long as we don't allow forwarding we're - // good. + // TODO(emilio): Cascade order will need to increment for each tree when + // we implement forwarding. + let shadow_cascade_order = ShadowCascadeOrder::for_innermost_containing_tree(); if let Some(part_rules) = part_rules { let containing_host = containing_shadow.map(|s| s.host()); let element = self.element; let rule_hash_target = self.rule_hash_target; let rules = &mut self.rules; let flags_setter = &mut self.flags_setter; - let shadow_cascade_order = self.shadow_cascade_order; - let cascade_level = CascadeLevel::SameTreeAuthorNormal; + let cascade_level = CascadeLevel::AuthorNormal { + shadow_cascade_order, + }; let start = rules.len(); self.context.with_shadow_host(containing_host, |context| { rule_hash_target.each_part(|p| { @@ -368,7 +369,6 @@ where context, flags_setter, cascade_level, - shadow_cascade_order, ); } }); @@ -382,7 +382,7 @@ where self.rules .push(ApplicableDeclarationBlock::from_declarations( sa.clone_arc(), - CascadeLevel::StyleAttributeNormal, + CascadeLevel::same_tree_author_normal(), )); } } @@ -433,12 +433,11 @@ where if self.stylist.author_styles_enabled() == AuthorStylesEnabled::No { return; } - self.collect_host_rules(); - self.collect_slotted_rules(); + self.collect_host_and_slotted_rules(); self.collect_normal_rules_from_containing_shadow_tree(); self.collect_document_author_rules(); - self.collect_part_rules(); self.collect_style_attribute(); + self.collect_part_rules(); self.collect_animation_rules(); } } diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs index 02ebeed0452..304a539744f 100644 --- a/components/style/rule_tree/mod.rs +++ b/components/style/rule_tree/mod.rs @@ -169,15 +169,60 @@ const FREE_LIST_SENTINEL: *mut RuleNode = 0x01 as *mut RuleNode; /// another thread is currently adding an entry). We spin if we find this value. const FREE_LIST_LOCKED: *mut RuleNode = 0x02 as *mut RuleNode; -/// A counter to track how many inner shadow roots rules deep we are. -/// -/// This is used to handle: +/// A counter to track how many shadow root rules deep we are. This is used to +/// handle: /// /// https://drafts.csswg.org/css-scoping/#shadow-cascading /// -/// In particular, it'd be `0` for the innermost shadow host, `1` for the next, -/// and so on. -pub type ShadowCascadeOrder = u8; +/// See the static functions for the meaning of different values. +#[derive(Clone, Copy, Debug, Eq, Hash, MallocSizeOf, Ord, PartialEq, PartialOrd)] +pub struct ShadowCascadeOrder(i8); + +impl ShadowCascadeOrder { + /// A level for the outermost shadow tree (the shadow tree we own, and the + /// ones from the slots we're slotted in). + #[inline] + pub fn for_outermost_shadow_tree() -> Self { + Self(-1) + } + + /// A level for the element's tree. + #[inline] + fn for_same_tree() -> Self { + Self(0) + } + + /// A level for the innermost containing tree (the one closest to the + /// element). + #[inline] + pub fn for_innermost_containing_tree() -> Self { + Self(1) + } + + /// Decrement the level, moving inwards. We should only move inwards if + /// we're traversing slots. + #[inline] + pub fn dec(&mut self) { + debug_assert!(self.0 < 0); + self.0 = self.0.saturating_sub(1); + } + + /// The level, moving inwards. We should only move inwards if we're + /// traversing slots. + #[inline] + pub fn inc(&mut self) { + debug_assert_ne!(self.0, -1); + self.0 = self.0.saturating_add(1); + } +} + +impl std::ops::Neg for ShadowCascadeOrder { + type Output = Self; + #[inline] + fn neg(self) -> Self { + Self(self.0.neg()) + } +} impl RuleTree { /// Construct a new rule tree. @@ -215,26 +260,20 @@ impl RuleTree { guards: &StylesheetGuards, ) -> StrongRuleNode where - I: Iterator, + I: Iterator, { use self::CascadeLevel::*; let mut current = self.root.clone(); - let mut last_level = current.get().level; let mut found_important = false; - let mut important_style_attr = None; - let mut important_same_tree = SmallVec::<[StyleSource; 4]>::new(); - let mut important_inner_shadow = SmallVec::<[SmallVec<[StyleSource; 4]>; 4]>::new(); - important_inner_shadow.push(SmallVec::new()); + let mut important_author = SmallVec::<[(StyleSource, ShadowCascadeOrder); 4]>::new(); let mut important_user = SmallVec::<[StyleSource; 4]>::new(); let mut important_ua = SmallVec::<[StyleSource; 4]>::new(); let mut transition = None; - let mut last_cascade_order = 0; - for (source, level, shadow_cascade_order) in iter { - debug_assert!(level >= last_level, "Not really ordered"); + for (source, level) in iter { debug_assert!(!level.is_important(), "Important levels handled internally"); let any_important = { let pdb = source.read(level.guard(guards)); @@ -244,29 +283,11 @@ impl RuleTree { if any_important { found_important = true; match level { - InnerShadowNormal => { - debug_assert!( - shadow_cascade_order >= last_cascade_order, - "Not really ordered" - ); - if shadow_cascade_order > last_cascade_order && - !important_inner_shadow.last().unwrap().is_empty() - { - last_cascade_order = shadow_cascade_order; - important_inner_shadow.push(SmallVec::new()); - } - important_inner_shadow - .last_mut() - .unwrap() - .push(source.clone()) + AuthorNormal { shadow_cascade_order } => { + important_author.push((source.clone(), shadow_cascade_order)); }, - SameTreeAuthorNormal => important_same_tree.push(source.clone()), UANormal => important_ua.push(source.clone()), UserNormal => important_user.push(source.clone()), - StyleAttributeNormal => { - debug_assert!(important_style_attr.is_none()); - important_style_attr = Some(source.clone()); - }, _ => {}, }; } @@ -290,7 +311,6 @@ impl RuleTree { } else { current = current.ensure_child(self.root.downgrade(), source, level); } - last_level = level; } // Early-return in the common case of no !important declarations. @@ -298,23 +318,35 @@ impl RuleTree { return current; } - // // Insert important declarations, in order of increasing importance, // followed by any transition rule. // - - for source in important_same_tree.drain() { - current = current.ensure_child(self.root.downgrade(), source, SameTreeAuthorImportant); + // Inner shadow wins over same-tree, which wins over outer-shadow. + // + // We negate the shadow cascade order to preserve the right PartialOrd + // behavior. + if !important_author.is_empty() && + important_author.first().unwrap().1 != important_author.last().unwrap().1 + { + // We only need to sort if the important rules come from + // different trees, but we need this sort to be stable. + // + // FIXME(emilio): This could maybe be smarter, probably by chunking + // the important rules while inserting, and iterating the outer + // chunks in reverse order. + // + // That is, if we have rules with levels like: -1 -1 -1 0 0 0 1 1 1, + // we're really only sorting the chunks, while keeping elements + // inside the same chunk already sorted. Seems like we could try to + // keep a SmallVec-of-SmallVecs with the chunks and just iterate the + // outer in reverse. + important_author.sort_by_key(|&(_, order)| -order); } - if let Some(source) = important_style_attr { - current = current.ensure_child(self.root.downgrade(), source, StyleAttributeImportant); - } - - for mut list in important_inner_shadow.drain().rev() { - for source in list.drain() { - current = current.ensure_child(self.root.downgrade(), source, InnerShadowImportant); - } + for (source, shadow_cascade_order) in important_author.drain() { + current = current.ensure_child(self.root.downgrade(), source, AuthorImportant { + shadow_cascade_order: -shadow_cascade_order, + }); } for source in important_user.drain() { @@ -359,11 +391,8 @@ impl RuleTree { I: Iterator, { let mut current = from; - let mut last_level = current.get().level; for (source, level) in iter { - debug_assert!(last_level <= level, "Not really ordered"); current = current.ensure_child(self.root.downgrade(), source, level); - last_level = level; } current } @@ -439,7 +468,6 @@ impl RuleTree { guards: &StylesheetGuards, important_rules_changed: &mut bool, ) -> Option { - debug_assert!(level.is_unique_per_element()); // TODO(emilio): Being smarter with lifetimes we could avoid a bit of // the refcount churn. let mut current = path.clone(); @@ -468,7 +496,16 @@ impl RuleTree { if current.get().level == level { *important_rules_changed |= level.is_important(); - if let Some(pdb) = pdb { + let current_decls = current + .get() + .source + .as_ref() + .unwrap() + .as_declarations(); + + // If the only rule at the level we're replacing is exactly the + // same as `pdb`, we're done, and `path` is still valid. + if let (Some(ref pdb), Some(ref current_decls)) = (pdb, current_decls) { // If the only rule at the level we're replacing is exactly the // same as `pdb`, we're done, and `path` is still valid. // @@ -478,25 +515,17 @@ impl RuleTree { // also equally valid. This is less likely, and would require an // in-place mutation of the source, which is, at best, fiddly, // so let's skip it for now. - let current_decls = current - .get() - .source - .as_ref() - .unwrap() - .as_declarations() - .expect("Replacing non-declarations style?"); - let is_here_already = ArcBorrow::ptr_eq(&pdb, ¤t_decls); + let is_here_already = ArcBorrow::ptr_eq(pdb, current_decls); if is_here_already { debug!("Picking the fast path in rule replacement"); return None; } } - current = current.parent().unwrap().clone(); + + if current_decls.is_some() { + current = current.parent().unwrap().clone(); + } } - debug_assert!( - current.get().level != level, - "Multiple rules should've been replaced?" - ); // Insert the rule if it's relevant at this level in the cascade. // @@ -611,46 +640,42 @@ const RULE_TREE_GC_INTERVAL: usize = 300; /// tree may affect an element connected to the document or an "outer" shadow /// tree. /// -/// We need to differentiate between rules from the same tree and "inner" shadow -/// trees in order to be able to find the right position for the style attribute -/// easily. Otherwise we wouldn't be able to avoid selector-matching when a -/// style attribute is added or removed. -/// /// [1]: https://drafts.csswg.org/css-cascade/#cascade-origin /// [2]: https://drafts.csswg.org/css-cascade/#preshint /// [3]: https://html.spec.whatwg.org/multipage/#presentational-hints /// [4]: https://drafts.csswg.org/css-scoping/#shadow-cascading #[repr(u8)] -#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, PartialOrd)] -#[cfg_attr(feature = "servo", derive(MallocSizeOf))] +#[derive(Clone, Copy, Debug, Eq, Hash, MallocSizeOf, PartialEq, PartialOrd)] pub enum CascadeLevel { /// Normal User-Agent rules. - UANormal = 0, + UANormal, /// User normal rules. UserNormal, /// Presentational hints. PresHints, - /// Shadow DOM styles from "inner" shadow trees. - /// - /// See above for why this is needed instead of merging InnerShadowNormal, - /// SameTreeAuthorNormal and StyleAttributeNormal inside something like - /// AuthorNormal. - InnerShadowNormal, - /// Author normal rules from the same tree the element is in. - SameTreeAuthorNormal, - /// Style attribute normal rules. - StyleAttributeNormal, + /// Shadow DOM styles from author styles. + AuthorNormal { + /// The order in the shadow tree hierarchy. This number is relative to + /// the tree of the element, and thus the only invariants that need to + /// be preserved is: + /// + /// * Zero is the same tree as the element that matched the rule. This + /// is important so that we can optimize style attribute insertions. + /// + /// * The levels are ordered in accordance with + /// https://drafts.csswg.org/css-scoping/#shadow-cascading + shadow_cascade_order: ShadowCascadeOrder, + }, /// SVG SMIL animations. SMILOverride, /// CSS animations and script-generated animations. Animations, - /// Author-supplied important rules from the same tree the element came - /// from. - SameTreeAuthorImportant, - /// Style attribute important rules. - StyleAttributeImportant, - /// Shadow DOM important rules. - InnerShadowImportant, + /// Author-supplied important rules. + AuthorImportant { + /// The order in the shadow tree hierarchy, inverted, so that PartialOrd + /// does the right thing. + shadow_cascade_order: ShadowCascadeOrder, + }, /// User important rules. UserImportant, /// User-agent important rules. @@ -662,12 +687,6 @@ pub enum CascadeLevel { } impl CascadeLevel { - /// Converts a raw byte to a CascadeLevel. - pub unsafe fn from_byte(byte: u8) -> Self { - debug_assert!(byte <= CascadeLevel::Transitions as u8); - mem::transmute(byte) - } - /// Select a lock guard for this level pub fn guard<'a>(&self, guards: &'a StylesheetGuards<'a>) -> &'a SharedRwLockReadGuard<'a> { match *self { @@ -679,16 +698,21 @@ impl CascadeLevel { } } - /// Returns whether this cascade level is unique per element, in which case - /// we can replace the path in the cascade without fear. - pub fn is_unique_per_element(&self) -> bool { - match *self { - CascadeLevel::Transitions | - CascadeLevel::Animations | - CascadeLevel::SMILOverride | - CascadeLevel::StyleAttributeNormal | - CascadeLevel::StyleAttributeImportant => true, - _ => false, + /// Returns the cascade level for author important declarations from the + /// same tree as the element. + #[inline] + pub fn same_tree_author_important() -> Self { + CascadeLevel::AuthorImportant { + shadow_cascade_order: ShadowCascadeOrder::for_same_tree(), + } + } + + /// Returns the cascade level for author normal declarations from the same + /// tree as the element. + #[inline] + pub fn same_tree_author_normal() -> Self { + CascadeLevel::AuthorNormal { + shadow_cascade_order: ShadowCascadeOrder::for_same_tree(), } } @@ -697,9 +721,7 @@ impl CascadeLevel { #[inline] pub fn is_important(&self) -> bool { match *self { - CascadeLevel::SameTreeAuthorImportant | - CascadeLevel::InnerShadowImportant | - CascadeLevel::StyleAttributeImportant | + CascadeLevel::AuthorImportant { .. } | CascadeLevel::UserImportant | CascadeLevel::UAImportant => true, _ => false, @@ -724,14 +746,10 @@ impl CascadeLevel { CascadeLevel::UAImportant | CascadeLevel::UANormal => Origin::UserAgent, CascadeLevel::UserImportant | CascadeLevel::UserNormal => Origin::User, CascadeLevel::PresHints | - CascadeLevel::InnerShadowNormal | - CascadeLevel::SameTreeAuthorNormal | - CascadeLevel::StyleAttributeNormal | + CascadeLevel::AuthorNormal { .. } | + CascadeLevel::AuthorImportant { .. } | CascadeLevel::SMILOverride | CascadeLevel::Animations | - CascadeLevel::SameTreeAuthorImportant | - CascadeLevel::StyleAttributeImportant | - CascadeLevel::InnerShadowImportant | CascadeLevel::Transitions => Origin::Author, } } @@ -1146,6 +1164,15 @@ impl StrongRuleNode { ) -> StrongRuleNode { use parking_lot::RwLockUpgradableReadGuard; + debug_assert!( + self.get().level <= level, + "Should be ordered (instead {:?} > {:?}), from {:?} and {:?}", + self.get().level, + level, + self.get().source, + source, + ); + let key = ChildKey(level, source.key()); let read_guard = self.get().children.upgradable_read(); @@ -1448,55 +1475,40 @@ impl StrongRuleNode { } }); - match node.cascade_level() { - // Non-author rules: - CascadeLevel::UANormal | - CascadeLevel::UAImportant | - CascadeLevel::UserNormal | - CascadeLevel::UserImportant => { - for (id, declaration) in longhands { - if properties.contains(id) { - // This property was set by a non-author rule. - // Stop looking for it in this element's rule - // nodes. - properties.remove(id); + let is_author = node.cascade_level().origin() == Origin::Author; + for (id, declaration) in longhands { + if !properties.contains(id) { + continue; + } - // However, if it is inherited, then it might be - // inherited from an author rule from an - // ancestor element's rule nodes. - if declaration.get_css_wide_keyword() == - Some(CSSWideKeyword::Inherit) - { - have_explicit_ua_inherit = true; - inherited_properties.insert(id); - } + if is_author { + if !author_colors_allowed { + // FIXME(emilio): this looks wrong, this should + // do: if color is not transparent, then return + // true, or something. + if let PropertyDeclaration::BackgroundColor(ref color) = + *declaration + { + return *color == Color::transparent(); } } - }, - // Author rules: - CascadeLevel::PresHints | - CascadeLevel::SameTreeAuthorNormal | - CascadeLevel::InnerShadowNormal | - CascadeLevel::StyleAttributeNormal | - CascadeLevel::SMILOverride | - CascadeLevel::Animations | - CascadeLevel::SameTreeAuthorImportant | - CascadeLevel::InnerShadowImportant | - CascadeLevel::StyleAttributeImportant | - CascadeLevel::Transitions => { - for (id, declaration) in longhands { - if properties.contains(id) { - if !author_colors_allowed { - if let PropertyDeclaration::BackgroundColor(ref color) = - *declaration - { - return *color == Color::transparent(); - } - } - return true; - } - } - }, + return true; + } + + // This property was set by a non-author rule. + // Stop looking for it in this element's rule + // nodes. + properties.remove(id); + + // However, if it is inherited, then it might be + // inherited from an author rule from an + // ancestor element's rule nodes. + if declaration.get_css_wide_keyword() == + Some(CSSWideKeyword::Inherit) + { + have_explicit_ua_inherit = true; + inherited_properties.insert(id); + } } } diff --git a/components/style/selector_map.rs b/components/style/selector_map.rs index 52b939e0219..81144c2a4bf 100644 --- a/components/style/selector_map.rs +++ b/components/style/selector_map.rs @@ -10,7 +10,7 @@ use crate::context::QuirksMode; use crate::dom::TElement; use crate::hash::map as hash_map; use crate::hash::{HashMap, HashSet}; -use crate::rule_tree::{CascadeLevel, ShadowCascadeOrder}; +use crate::rule_tree::CascadeLevel; use crate::selector_parser::SelectorImpl; use crate::stylist::Rule; use crate::{Atom, LocalName, Namespace, WeakAtom}; @@ -171,7 +171,6 @@ impl SelectorMap { context: &mut MatchingContext, flags_setter: &mut F, cascade_level: CascadeLevel, - shadow_cascade_order: ShadowCascadeOrder, ) where E: TElement, F: FnMut(&E, ElementSelectorFlags), @@ -190,7 +189,6 @@ impl SelectorMap { context, flags_setter, cascade_level, - shadow_cascade_order, ); } @@ -203,7 +201,6 @@ impl SelectorMap { context, flags_setter, cascade_level, - shadow_cascade_order, ) } } @@ -217,7 +214,6 @@ impl SelectorMap { context, flags_setter, cascade_level, - shadow_cascade_order, ) } }); @@ -230,7 +226,6 @@ impl SelectorMap { context, flags_setter, cascade_level, - shadow_cascade_order, ) } @@ -242,7 +237,6 @@ impl SelectorMap { context, flags_setter, cascade_level, - shadow_cascade_order, ) } @@ -253,7 +247,6 @@ impl SelectorMap { context, flags_setter, cascade_level, - shadow_cascade_order, ); } @@ -265,7 +258,6 @@ impl SelectorMap { context: &mut MatchingContext, flags_setter: &mut F, cascade_level: CascadeLevel, - shadow_cascade_order: ShadowCascadeOrder, ) where E: TElement, F: FnMut(&E, ElementSelectorFlags), @@ -280,7 +272,7 @@ impl SelectorMap { flags_setter, ) { matching_rules.push( - rule.to_applicable_declaration_block(cascade_level, shadow_cascade_order), + rule.to_applicable_declaration_block(cascade_level), ); } } diff --git a/components/style/servo/selector_parser.rs b/components/style/servo/selector_parser.rs index 4fdd4d4ab37..72c47e65dda 100644 --- a/components/style/servo/selector_parser.rs +++ b/components/style/servo/selector_parser.rs @@ -173,6 +173,12 @@ impl PseudoElement { false } + /// Whether this pseudo-element is the ::-moz-color-swatch pseudo. + #[inline] + pub fn is_color_swatch(&self) -> bool { + false + } + /// Whether this pseudo-element is eagerly-cascaded. #[inline] pub fn is_eager(&self) -> bool { diff --git a/components/style/stylist.rs b/components/style/stylist.rs index ab6fe309f3f..414210b7d20 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -18,7 +18,7 @@ use crate::properties::{self, CascadeMode, ComputedValues}; use crate::properties::{AnimationRules, PropertyDeclarationBlock}; use crate::rule_cache::{RuleCache, RuleCacheConditions}; use crate::rule_collector::{containing_shadow_ignoring_svg_use, RuleCollector}; -use crate::rule_tree::{CascadeLevel, RuleTree, ShadowCascadeOrder, StrongRuleNode, StyleSource}; +use crate::rule_tree::{CascadeLevel, RuleTree, StrongRuleNode, StyleSource}; use crate::selector_map::{PrecomputedHashMap, PrecomputedHashSet, SelectorMap, SelectorMapEntry}; use crate::selector_parser::{PerPseudoElementMap, PseudoElement, SelectorImpl, SnapshotMap}; use crate::shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards}; @@ -1324,7 +1324,7 @@ impl Stylist { .declaration_importance_iter() .map(|(declaration, importance)| { debug_assert!(!importance.important()); - (declaration, CascadeLevel::StyleAttributeNormal) + (declaration, CascadeLevel::same_tree_author_normal()) }) }; @@ -1987,7 +1987,6 @@ impl CascadeData { self.rules_source_order, CascadeLevel::UANormal, selector.specificity(), - 0, )); continue; } @@ -2323,7 +2322,6 @@ impl Rule { pub fn to_applicable_declaration_block( &self, level: CascadeLevel, - shadow_cascade_order: ShadowCascadeOrder, ) -> ApplicableDeclarationBlock { let source = StyleSource::from_rule(self.style_rule.clone()); ApplicableDeclarationBlock::new( @@ -2331,7 +2329,6 @@ impl Rule { self.source_order, level, self.specificity(), - shadow_cascade_order, ) } From 1f2c1f555cd5dcc3e0877325ef607ef695de3c59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 13 Nov 2019 09:59:10 +0000 Subject: [PATCH 12/45] style: Optimize storage of ApplicableDeclaration again. So that we don't regress perf. Differential Revision: https://phabricator.services.mozilla.com/D52576 --- components/style/applicable_declarations.rs | 28 ++++++--- components/style/rule_tree/mod.rs | 68 ++++++++++++++++++++- 2 files changed, 85 insertions(+), 11 deletions(-) diff --git a/components/style/applicable_declarations.rs b/components/style/applicable_declarations.rs index ab83e5737a9..cc7d9c90db8 100644 --- a/components/style/applicable_declarations.rs +++ b/components/style/applicable_declarations.rs @@ -19,27 +19,37 @@ use smallvec::SmallVec; /// However, it may depend a lot on workload, and stack space is cheap. pub type ApplicableDeclarationList = SmallVec<[ApplicableDeclarationBlock; 16]>; +/// Blink uses 18 bits to store source order, and does not check overflow [1]. +/// That's a limit that could be reached in realistic webpages, so we use +/// 24 bits and enforce defined behavior in the overflow case. +/// +/// [1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/ +/// RuleSet.h?l=128&rcl=90140ab80b84d0f889abc253410f44ed54ae04f3 +const SOURCE_ORDER_SHIFT: usize = 0; +const SOURCE_ORDER_BITS: usize = 24; +const SOURCE_ORDER_MAX: u32 = (1 << SOURCE_ORDER_BITS) - 1; +const SOURCE_ORDER_MASK: u32 = SOURCE_ORDER_MAX << SOURCE_ORDER_SHIFT; + +/// We pack the cascade level in a single byte, see CascadeLevel::to_byte_lossy +/// for the different trade-offs there. +const CASCADE_LEVEL_SHIFT: usize = SOURCE_ORDER_BITS; + /// Stores the source order of a block, the cascade level it belongs to, and the /// counter needed to handle Shadow DOM cascade order properly. -/// -/// FIXME(emilio): Optimize storage. #[derive(Clone, Copy, Eq, MallocSizeOf, PartialEq, Debug)] -struct ApplicableDeclarationBits { - source_order: u32, - cascade_level: CascadeLevel, -} +struct ApplicableDeclarationBits(u32); impl ApplicableDeclarationBits { fn new(source_order: u32, cascade_level: CascadeLevel) -> Self { - Self { source_order, cascade_level } + Self((source_order & SOURCE_ORDER_MASK) | ((cascade_level.to_byte_lossy() as u32) << CASCADE_LEVEL_SHIFT)) } fn source_order(&self) -> u32 { - self.source_order + self.0 & SOURCE_ORDER_MASK } fn level(&self) -> CascadeLevel { - self.cascade_level + CascadeLevel::from_byte((self.0 >> CASCADE_LEVEL_SHIFT) as u8) } } diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs index 304a539744f..7236c32e30d 100644 --- a/components/style/rule_tree/mod.rs +++ b/components/style/rule_tree/mod.rs @@ -681,12 +681,76 @@ pub enum CascadeLevel { /// User-agent important rules. UAImportant, /// Transitions - /// - /// NB: If this changes from being last, change from_byte below. Transitions, } impl CascadeLevel { + /// Pack this cascade level in a single byte. + /// + /// We have 10 levels, which we can represent with 4 bits, and then a + /// cascade order optionally, which we can clamp to three bits max, and + /// represent with a fourth bit for the sign. + /// + /// So this creates: SOOODDDD + /// + /// Where `S` is the sign of the order (one if negative, 0 otherwise), `O` + /// is the absolute value of the order, and `D`s are the discriminant. + #[inline] + pub fn to_byte_lossy(&self) -> u8 { + let (discriminant, order) = match *self { + Self::UANormal => (0, 0), + Self::UserNormal => (1, 0), + Self::PresHints => (2, 0), + Self::AuthorNormal { shadow_cascade_order } => (3, shadow_cascade_order.0), + Self::SMILOverride => (4, 0), + Self::Animations => (5, 0), + Self::AuthorImportant { shadow_cascade_order } => (6, shadow_cascade_order.0), + Self::UserImportant => (7, 0), + Self::UAImportant => (8, 0), + Self::Transitions => (9, 0), + }; + + debug_assert_eq!(discriminant & 0xf, discriminant); + if order == 0 { + return discriminant; + } + + let negative = order < 0; + let value = std::cmp::min(order.abs() as u8, 0b111); + (negative as u8) << 7 | value << 4 | discriminant + } + + /// Convert back from the single-byte representation of the cascade level + /// explained above. + #[inline] + pub fn from_byte(b: u8) -> Self { + let order = { + let abs = ((b & 0b01110000) >> 4) as i8; + let negative = b & 0b10000000 != 0; + if negative { -abs } else { abs } + }; + let discriminant = b & 0xf; + let level = match discriminant { + 0 => Self::UANormal, + 1 => Self::UserNormal, + 2 => Self::PresHints, + 3 => return Self::AuthorNormal { + shadow_cascade_order: ShadowCascadeOrder(order), + }, + 4 => Self::SMILOverride, + 5 => Self::Animations, + 6 => return Self::AuthorImportant { + shadow_cascade_order: ShadowCascadeOrder(order), + }, + 7 => Self::UserImportant, + 8 => Self::UAImportant, + 9 => Self::Transitions, + _ => unreachable!("Didn't expect {} as a discriminant", discriminant), + }; + debug_assert_eq!(order, 0, "Didn't expect an order value for {:?}", level); + level + } + /// Select a lock guard for this level pub fn guard<'a>(&self, guards: &'a StylesheetGuards<'a>) -> &'a SharedRwLockReadGuard<'a> { match *self { From 0cfc91d6cefb8502e7d1cafea0fd42d828ca6436 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 14 Nov 2019 12:54:00 +0000 Subject: [PATCH 13/45] style: Remove NODE_MAY_BE_IN_BINDING_MNGR. Never set anymore. Differential Revision: https://phabricator.services.mozilla.com/D52994 --- components/style/gecko/wrapper.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index da989e5a74a..f76c0c5fda6 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -300,7 +300,10 @@ impl<'ln> GeckoNode<'ln> { fn flattened_tree_parent_is_parent(&self) -> bool { use crate::gecko_bindings::structs::*; let flags = self.flags(); - if flags & (NODE_MAY_BE_IN_BINDING_MNGR as u32 | NODE_IS_IN_SHADOW_TREE as u32) != 0 { + + // FIXME(emilio): The shadow tree condition seems it shouldn't be needed + // anymore, if we check for slots. + if self.is_in_shadow_tree() { return false; } From bb06ed72064812c841a39c900aa1c950e348c10a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 15 Nov 2019 13:39:08 +0000 Subject: [PATCH 14/45] style: Centralize logic to ignore document colors. This was a follow-up from the backplate stuff which I requested but didn't happen. Differential Revision: https://phabricator.services.mozilla.com/D53170 --- components/style/gecko/media_queries.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index 2e68a889df1..3463c1f95e9 100644 --- a/components/style/gecko/media_queries.rs +++ b/components/style/gecko/media_queries.rs @@ -274,13 +274,7 @@ impl Device { if doc.mIsBeingUsedAsImage() { return true; } - let document_color_use = static_prefs::pref!("browser.display.document_color_use"); - let prefs = self.pref_sheet_prefs(); - match document_color_use { - 1 => true, - 2 => prefs.mIsChrome, - _ => !prefs.mUseAccessibilityTheme, - } + self.pref_sheet_prefs().mUseDocumentColors } /// Returns the default background color. From b420293a57305f36f3e1d6d2f9fbc8ee72bbd81a Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Fri, 15 Nov 2019 19:38:24 +0000 Subject: [PATCH 15/45] style: Treat 3d translate/scale as 2d if the value can be expressed as 2d. For the individual transform properties if they spec a value that can be expressed as 2d we treat as 2d and serialize accordingly. We drop Translate::Translate and Scale::Scale, and then rename Translate::Translate3D as Translate::Translate, Scale::Scale3D as Scale::Scale. So now we use Translate::Translate to represent 2d and 3d translation, and Scale::Scale to represent 2d and 3d scale. There is no difference between 2d and 3d translate/scale in Gecko because we always convert them into 3d format to layers (on the compositor thread), so this change makes things simpler. Differential Revision: https://phabricator.services.mozilla.com/D52931 --- components/style/values/animated/transform.rs | 201 ++++++++---------- components/style/values/generics/transform.rs | 87 ++++---- .../style/values/specified/transform.rs | 15 +- 3 files changed, 149 insertions(+), 154 deletions(-) diff --git a/components/style/values/animated/transform.rs b/components/style/values/animated/transform.rs index 9ac64c3f2a7..89bb3c0849d 100644 --- a/components/style/values/animated/transform.rs +++ b/components/style/values/animated/transform.rs @@ -139,10 +139,10 @@ impl ComputeSquaredDistance for MatrixDecomposed2D { const RAD_PER_DEG: f64 = std::f64::consts::PI / 180.0; let angle1 = self.angle as f64 * RAD_PER_DEG; let angle2 = other.angle as f64 * RAD_PER_DEG; - Ok(self.translate.compute_squared_distance(&other.translate)? + - self.scale.compute_squared_distance(&other.scale)? + - angle1.compute_squared_distance(&angle2)? + - self.matrix.compute_squared_distance(&other.matrix)?) + Ok(self.translate.compute_squared_distance(&other.translate)? + + self.scale.compute_squared_distance(&other.scale)? + + angle1.compute_squared_distance(&angle2)? + + self.matrix.compute_squared_distance(&other.matrix)?) } } @@ -316,9 +316,9 @@ impl ComputeSquaredDistance for Skew { // ComputeSquaredDistance manually. #[inline] fn compute_squared_distance(&self, other: &Self) -> Result { - Ok(self.0.atan().compute_squared_distance(&other.0.atan())? + - self.1.atan().compute_squared_distance(&other.1.atan())? + - self.2.atan().compute_squared_distance(&other.2.atan())?) + Ok(self.0.atan().compute_squared_distance(&other.0.atan())? + + self.1.atan().compute_squared_distance(&other.1.atan())? + + self.2.atan().compute_squared_distance(&other.2.atan())?) } } @@ -394,9 +394,9 @@ impl Animate for Quaternion { debug_assert!( // Doule EPSILON since both this_weight and other_weght have calculation errors // which are approximately equal to EPSILON. - (this_weight + other_weight - 1.0f64).abs() <= f64::EPSILON * 2.0 || - other_weight == 1.0f64 || - other_weight == 0.0f64, + (this_weight + other_weight - 1.0f64).abs() <= f64::EPSILON * 2.0 + || other_weight == 1.0f64 + || other_weight == 0.0f64, "animate should only be used for interpolating or accumulating transforms" ); @@ -830,17 +830,17 @@ fn is_matched_operation( second: &ComputedTransformOperation, ) -> bool { match (first, second) { - (&TransformOperation::Matrix(..), &TransformOperation::Matrix(..)) | - (&TransformOperation::Matrix3D(..), &TransformOperation::Matrix3D(..)) | - (&TransformOperation::Skew(..), &TransformOperation::Skew(..)) | - (&TransformOperation::SkewX(..), &TransformOperation::SkewX(..)) | - (&TransformOperation::SkewY(..), &TransformOperation::SkewY(..)) | - (&TransformOperation::Rotate(..), &TransformOperation::Rotate(..)) | - (&TransformOperation::Rotate3D(..), &TransformOperation::Rotate3D(..)) | - (&TransformOperation::RotateX(..), &TransformOperation::RotateX(..)) | - (&TransformOperation::RotateY(..), &TransformOperation::RotateY(..)) | - (&TransformOperation::RotateZ(..), &TransformOperation::RotateZ(..)) | - (&TransformOperation::Perspective(..), &TransformOperation::Perspective(..)) => true, + (&TransformOperation::Matrix(..), &TransformOperation::Matrix(..)) + | (&TransformOperation::Matrix3D(..), &TransformOperation::Matrix3D(..)) + | (&TransformOperation::Skew(..), &TransformOperation::Skew(..)) + | (&TransformOperation::SkewX(..), &TransformOperation::SkewX(..)) + | (&TransformOperation::SkewY(..), &TransformOperation::SkewY(..)) + | (&TransformOperation::Rotate(..), &TransformOperation::Rotate(..)) + | (&TransformOperation::Rotate3D(..), &TransformOperation::Rotate3D(..)) + | (&TransformOperation::RotateX(..), &TransformOperation::RotateX(..)) + | (&TransformOperation::RotateY(..), &TransformOperation::RotateY(..)) + | (&TransformOperation::RotateZ(..), &TransformOperation::RotateZ(..)) + | (&TransformOperation::Perspective(..), &TransformOperation::Perspective(..)) => true, // Match functions that have the same primitive transform function (a, b) if a.is_translate() && b.is_translate() => true, (a, b) if a.is_scale() && b.is_scale() => true, @@ -895,21 +895,21 @@ impl Animate for ComputedTransform { Procedure::Add => { debug_assert!(false, "Should have already dealt with add by the point"); return Err(()); - }, + } Procedure::Interpolate { progress } => { result.push(TransformOperation::InterpolateMatrix { from_list: Transform(this_remainder.to_vec().into()), to_list: Transform(other_remainder.to_vec().into()), progress: Percentage(progress as f32), }); - }, + } Procedure::Accumulate { count } => { result.push(TransformOperation::AccumulateMatrix { from_list: Transform(this_remainder.to_vec().into()), to_list: Transform(other_remainder.to_vec().into()), count: cmp::min(count, i32::max_value() as u64) as i32, }); - }, + } }, // If there is a remainder from just one list, then one list must be shorter but // completely match the type of the corresponding functions in the longer list. @@ -925,8 +925,8 @@ impl Animate for ComputedTransform { match transform { // We can't interpolate/accumulate ___Matrix types directly with a // matrix. Instead we need to wrap it in another ___Matrix type. - TransformOperation::AccumulateMatrix { .. } | - TransformOperation::InterpolateMatrix { .. } => { + TransformOperation::AccumulateMatrix { .. } + | TransformOperation::InterpolateMatrix { .. } => { let transform_list = Transform(vec![transform.clone()].into()); let identity_list = Transform(vec![identity].into()); let (from_list, to_list) = if fill_right { @@ -943,7 +943,7 @@ impl Animate for ComputedTransform { to_list, progress: Percentage(progress as f32), }) - }, + } Procedure::Accumulate { count } => { Ok(TransformOperation::AccumulateMatrix { from_list, @@ -951,9 +951,9 @@ impl Animate for ComputedTransform { count: cmp::min(count, i32::max_value() as u64) as i32, }) - }, + } } - }, + } _ => { let (lhs, rhs) = if fill_right { (transform, &identity) @@ -961,13 +961,13 @@ impl Animate for ComputedTransform { (&identity, transform) }; lhs.animate(rhs, procedure) - }, + } } }) .collect::, _>>()?, ); - }, - (None, None) => {}, + } + (None, None) => {} } Ok(Transform(result.into())) @@ -999,10 +999,10 @@ impl Animate for ComputedTransformOperation { Ok(TransformOperation::Matrix3D( this.animate(other, procedure)?, )) - }, + } (&TransformOperation::Matrix(ref this), &TransformOperation::Matrix(ref other)) => { Ok(TransformOperation::Matrix(this.animate(other, procedure)?)) - }, + } ( &TransformOperation::Skew(ref fx, ref fy), &TransformOperation::Skew(ref tx, ref ty), @@ -1012,10 +1012,10 @@ impl Animate for ComputedTransformOperation { )), (&TransformOperation::SkewX(ref f), &TransformOperation::SkewX(ref t)) => { Ok(TransformOperation::SkewX(f.animate(t, procedure)?)) - }, + } (&TransformOperation::SkewY(ref f), &TransformOperation::SkewY(ref t)) => { Ok(TransformOperation::SkewY(f.animate(t, procedure)?)) - }, + } ( &TransformOperation::Translate3D(ref fx, ref fy, ref fz), &TransformOperation::Translate3D(ref tx, ref ty, ref tz), @@ -1033,13 +1033,13 @@ impl Animate for ComputedTransformOperation { )), (&TransformOperation::TranslateX(ref f), &TransformOperation::TranslateX(ref t)) => { Ok(TransformOperation::TranslateX(f.animate(t, procedure)?)) - }, + } (&TransformOperation::TranslateY(ref f), &TransformOperation::TranslateY(ref t)) => { Ok(TransformOperation::TranslateY(f.animate(t, procedure)?)) - }, + } (&TransformOperation::TranslateZ(ref f), &TransformOperation::TranslateZ(ref t)) => { Ok(TransformOperation::TranslateZ(f.animate(t, procedure)?)) - }, + } ( &TransformOperation::Scale3D(ref fx, ref fy, ref fz), &TransformOperation::Scale3D(ref tx, ref ty, ref tz), @@ -1072,25 +1072,25 @@ impl Animate for ComputedTransformOperation { .animate(&Rotate::Rotate3D(tx, ty, tz, ta), procedure)?; let (fx, fy, fz, fa) = ComputedRotate::resolve(&animated); Ok(TransformOperation::Rotate3D(fx, fy, fz, fa)) - }, + } (&TransformOperation::RotateX(fa), &TransformOperation::RotateX(ta)) => { Ok(TransformOperation::RotateX(fa.animate(&ta, procedure)?)) - }, + } (&TransformOperation::RotateY(fa), &TransformOperation::RotateY(ta)) => { Ok(TransformOperation::RotateY(fa.animate(&ta, procedure)?)) - }, + } (&TransformOperation::RotateZ(fa), &TransformOperation::RotateZ(ta)) => { Ok(TransformOperation::RotateZ(fa.animate(&ta, procedure)?)) - }, + } (&TransformOperation::Rotate(fa), &TransformOperation::Rotate(ta)) => { Ok(TransformOperation::Rotate(fa.animate(&ta, procedure)?)) - }, + } (&TransformOperation::Rotate(fa), &TransformOperation::RotateZ(ta)) => { Ok(TransformOperation::Rotate(fa.animate(&ta, procedure)?)) - }, + } (&TransformOperation::RotateZ(fa), &TransformOperation::Rotate(ta)) => { Ok(TransformOperation::Rotate(fa.animate(&ta, procedure)?)) - }, + } ( &TransformOperation::Perspective(ref fd), &TransformOperation::Perspective(ref td), @@ -1120,13 +1120,13 @@ impl Animate for ComputedTransformOperation { Ok(TransformOperation::Perspective(CSSPixelLength::new( used_value, ))) - }, + } _ if self.is_translate() && other.is_translate() => self .to_translate_3d() .animate(&other.to_translate_3d(), procedure), _ if self.is_scale() && other.is_scale() => { self.to_scale_3d().animate(&other.to_scale_3d(), procedure) - }, + } _ if self.is_rotate() && other.is_rotate() => self .to_rotate_3d() .animate(&other.to_rotate_3d(), procedure), @@ -1144,20 +1144,20 @@ impl ComputeSquaredDistance for ComputedTransformOperation { match (self, other) { (&TransformOperation::Matrix3D(ref this), &TransformOperation::Matrix3D(ref other)) => { this.compute_squared_distance(other) - }, + } (&TransformOperation::Matrix(ref this), &TransformOperation::Matrix(ref other)) => { let this: Matrix3D = (*this).into(); let other: Matrix3D = (*other).into(); this.compute_squared_distance(&other) - }, + } ( &TransformOperation::Skew(ref fx, ref fy), &TransformOperation::Skew(ref tx, ref ty), ) => Ok(fx.compute_squared_distance(&tx)? + fy.compute_squared_distance(&ty)?), - (&TransformOperation::SkewX(ref f), &TransformOperation::SkewX(ref t)) | - (&TransformOperation::SkewY(ref f), &TransformOperation::SkewY(ref t)) => { + (&TransformOperation::SkewX(ref f), &TransformOperation::SkewX(ref t)) + | (&TransformOperation::SkewY(ref f), &TransformOperation::SkewY(ref t)) => { f.compute_squared_distance(&t) - }, + } ( &TransformOperation::Translate3D(ref fx, ref fy, ref fz), &TransformOperation::Translate3D(ref tx, ref ty, ref tz), @@ -1173,33 +1173,33 @@ impl ComputeSquaredDistance for ComputedTransformOperation { let tx = tx.length_component().px(); let ty = ty.length_component().px(); - Ok(fx.compute_squared_distance(&tx)? + - fy.compute_squared_distance(&ty)? + - fz.compute_squared_distance(&tz)?) - }, + Ok(fx.compute_squared_distance(&tx)? + + fy.compute_squared_distance(&ty)? + + fz.compute_squared_distance(&tz)?) + } ( &TransformOperation::Scale3D(ref fx, ref fy, ref fz), &TransformOperation::Scale3D(ref tx, ref ty, ref tz), - ) => Ok(fx.compute_squared_distance(&tx)? + - fy.compute_squared_distance(&ty)? + - fz.compute_squared_distance(&tz)?), + ) => Ok(fx.compute_squared_distance(&tx)? + + fy.compute_squared_distance(&ty)? + + fz.compute_squared_distance(&tz)?), ( &TransformOperation::Rotate3D(fx, fy, fz, fa), &TransformOperation::Rotate3D(tx, ty, tz, ta), ) => Rotate::Rotate3D(fx, fy, fz, fa) .compute_squared_distance(&Rotate::Rotate3D(tx, ty, tz, ta)), - (&TransformOperation::RotateX(fa), &TransformOperation::RotateX(ta)) | - (&TransformOperation::RotateY(fa), &TransformOperation::RotateY(ta)) | - (&TransformOperation::RotateZ(fa), &TransformOperation::RotateZ(ta)) | - (&TransformOperation::Rotate(fa), &TransformOperation::Rotate(ta)) => { + (&TransformOperation::RotateX(fa), &TransformOperation::RotateX(ta)) + | (&TransformOperation::RotateY(fa), &TransformOperation::RotateY(ta)) + | (&TransformOperation::RotateZ(fa), &TransformOperation::RotateZ(ta)) + | (&TransformOperation::Rotate(fa), &TransformOperation::Rotate(ta)) => { fa.compute_squared_distance(&ta) - }, + } ( &TransformOperation::Perspective(ref fd), &TransformOperation::Perspective(ref td), ) => fd.compute_squared_distance(td), - (&TransformOperation::Perspective(ref p), &TransformOperation::Matrix3D(ref m)) | - (&TransformOperation::Matrix3D(ref m), &TransformOperation::Perspective(ref p)) => { + (&TransformOperation::Perspective(ref p), &TransformOperation::Matrix3D(ref m)) + | (&TransformOperation::Matrix3D(ref m), &TransformOperation::Perspective(ref p)) => { // FIXME(emilio): Is this right? Why interpolating this with // Perspective but not with anything else? let mut p_matrix = Matrix3D::identity(); @@ -1207,7 +1207,7 @@ impl ComputeSquaredDistance for ComputedTransformOperation { p_matrix.m34 = -1. / p.px(); } p_matrix.compute_squared_distance(&m) - }, + } // Gecko cross-interpolates amongst all translate and all scale // functions (See ToPrimitive in layout/style/StyleAnimationValue.cpp) // without falling back to InterpolateMatrix @@ -1256,7 +1256,7 @@ impl Animate for ComputedRotate { fz, fa.animate(&Angle::zero(), procedure)?, )) - }, + } (&Rotate::None, &Rotate::Rotate3D(tx, ty, tz, ta)) => { // No need to normalize `none`, so animate angle directly. Ok(Rotate::Rotate3D( @@ -1265,7 +1265,7 @@ impl Animate for ComputedRotate { tz, Angle::zero().animate(&ta, procedure)?, )) - }, + } (&Rotate::Rotate3D(_, ..), _) | (_, &Rotate::Rotate3D(_, ..)) => { let (from, to) = (self.resolve(), other.resolve()); let (mut fx, mut fy, mut fz, fa) = @@ -1301,12 +1301,12 @@ impl Animate for ComputedRotate { ); Ok(Rotate::Rotate3D(x, y, z, Angle::from_radians(angle))) - }, + } (&Rotate::Rotate(_), _) | (_, &Rotate::Rotate(_)) => { // If this is a 2D rotation, we just animate the let (from, to) = (self.resolve().3, other.resolve().3); Ok(Rotate::Rotate(from.animate(&to, procedure)?)) - }, + } } } } @@ -1316,10 +1316,10 @@ impl ComputeSquaredDistance for ComputedRotate { fn compute_squared_distance(&self, other: &Self) -> Result { match (self, other) { (&Rotate::None, &Rotate::None) => Ok(SquaredDistance::from_sqrt(0.)), - (&Rotate::Rotate3D(_, _, _, a), &Rotate::None) | - (&Rotate::None, &Rotate::Rotate3D(_, _, _, a)) => { + (&Rotate::Rotate3D(_, _, _, a), &Rotate::None) + | (&Rotate::None, &Rotate::Rotate3D(_, _, _, a)) => { a.compute_squared_distance(&Angle::zero()) - }, + } (&Rotate::Rotate3D(_, ..), _) | (_, &Rotate::Rotate3D(_, ..)) => { let (from, to) = (self.resolve(), other.resolve()); let (mut fx, mut fy, mut fz, angle1) = @@ -1346,7 +1346,7 @@ impl ComputeSquaredDistance for ComputedRotate { let q2 = Quaternion::from_direction_and_angle(&v2, angle2.radians64()); q1.compute_squared_distance(&q2) } - }, + } (&Rotate::Rotate(_), _) | (_, &Rotate::Rotate(_)) => self .resolve() .3 @@ -1368,8 +1368,7 @@ impl ComputedTranslate { LengthPercentage::zero(), Length::zero(), ), - Translate::Translate3D(tx, ty, tz) => (tx, ty, tz), - Translate::Translate(tx, ty) => (tx, ty, Length::zero()), + Translate::Translate(tx, ty, tz) => (tx, ty, tz), } } } @@ -1379,21 +1378,14 @@ impl Animate for ComputedTranslate { fn animate(&self, other: &Self, procedure: Procedure) -> Result { match (self, other) { (&Translate::None, &Translate::None) => Ok(Translate::None), - (&Translate::Translate3D(_, ..), _) | (_, &Translate::Translate3D(_, ..)) => { - let (from, to) = (self.resolve(), other.resolve()); - Ok(Translate::Translate3D( - from.0.animate(&to.0, procedure)?, - from.1.animate(&to.1, procedure)?, - from.2.animate(&to.2, procedure)?, - )) - }, (&Translate::Translate(_, ..), _) | (_, &Translate::Translate(_, ..)) => { let (from, to) = (self.resolve(), other.resolve()); Ok(Translate::Translate( from.0.animate(&to.0, procedure)?, from.1.animate(&to.1, procedure)?, + from.2.animate(&to.2, procedure)?, )) - }, + } } } } @@ -1402,9 +1394,9 @@ impl ComputeSquaredDistance for ComputedTranslate { #[inline] fn compute_squared_distance(&self, other: &Self) -> Result { let (from, to) = (self.resolve(), other.resolve()); - Ok(from.0.compute_squared_distance(&to.0)? + - from.1.compute_squared_distance(&to.1)? + - from.2.compute_squared_distance(&to.2)?) + Ok(from.0.compute_squared_distance(&to.0)? + + from.1.compute_squared_distance(&to.1)? + + from.2.compute_squared_distance(&to.2)?) } } @@ -1417,8 +1409,7 @@ impl ComputedScale { // Unspecified scales default to 1 match *self { Scale::None => (1.0, 1.0, 1.0), - Scale::Scale3D(sx, sy, sz) => (sx, sy, sz), - Scale::Scale(sx, sy) => (sx, sy, 1.), + Scale::Scale(sx, sy, sz) => (sx, sy, sz), } } } @@ -1428,7 +1419,7 @@ impl Animate for ComputedScale { fn animate(&self, other: &Self, procedure: Procedure) -> Result { match (self, other) { (&Scale::None, &Scale::None) => Ok(Scale::None), - (&Scale::Scale3D(_, ..), _) | (_, &Scale::Scale3D(_, ..)) => { + (&Scale::Scale(_, ..), _) | (_, &Scale::Scale(_, ..)) => { let (from, to) = (self.resolve(), other.resolve()); // For transform lists, we add by appending to the list of // transform functions. However, ComputedScale cannot be @@ -1436,26 +1427,14 @@ impl Animate for ComputedScale { // result here. if procedure == Procedure::Add { // scale(x1,y1,z1)*scale(x2,y2,z2) = scale(x1*x2, y1*y2, z1*z2) - return Ok(Scale::Scale3D(from.0 * to.0, from.1 * to.1, from.2 * to.2)); - } - Ok(Scale::Scale3D( - animate_multiplicative_factor(from.0, to.0, procedure)?, - animate_multiplicative_factor(from.1, to.1, procedure)?, - animate_multiplicative_factor(from.2, to.2, procedure)?, - )) - }, - (&Scale::Scale(_, ..), _) | (_, &Scale::Scale(_, ..)) => { - let (from, to) = (self.resolve(), other.resolve()); - // As with Scale3D, addition needs special handling. - if procedure == Procedure::Add { - // scale(x1,y1)*scale(x2,y2) = scale(x1*x2, y1*y2) - return Ok(Scale::Scale(from.0 * to.0, from.1 * to.1)); + return Ok(Scale::Scale(from.0 * to.0, from.1 * to.1, from.2 * to.2)); } Ok(Scale::Scale( animate_multiplicative_factor(from.0, to.0, procedure)?, animate_multiplicative_factor(from.1, to.1, procedure)?, + animate_multiplicative_factor(from.2, to.2, procedure)?, )) - }, + } } } } @@ -1464,8 +1443,8 @@ impl ComputeSquaredDistance for ComputedScale { #[inline] fn compute_squared_distance(&self, other: &Self) -> Result { let (from, to) = (self.resolve(), other.resolve()); - Ok(from.0.compute_squared_distance(&to.0)? + - from.1.compute_squared_distance(&to.1)? + - from.2.compute_squared_distance(&to.2)?) + Ok(from.0.compute_squared_distance(&to.0)? + + from.1.compute_squared_distance(&to.1)? + + from.2.compute_squared_distance(&to.2)?) } } diff --git a/components/style/values/generics/transform.rs b/components/style/values/generics/transform.rs index 4a8dc7f63fa..cc401966a97 100644 --- a/components/style/values/generics/transform.rs +++ b/components/style/values/generics/transform.rs @@ -303,7 +303,7 @@ where match *self { Translate(..) | Translate3D(..) | TranslateX(..) | TranslateY(..) | TranslateZ(..) => { true - }, + } _ => false, } } @@ -415,8 +415,8 @@ where fn is_3d(&self) -> bool { use self::TransformOperation::*; match *self { - Translate3D(..) | TranslateZ(..) | Rotate3D(..) | RotateX(..) | RotateY(..) | - RotateZ(..) | Scale3D(..) | ScaleZ(..) | Perspective(..) | Matrix3D(..) => true, + Translate3D(..) | TranslateZ(..) | Rotate3D(..) | RotateX(..) | RotateY(..) + | RotateZ(..) | Scale3D(..) | ScaleZ(..) | Perspective(..) | Matrix3D(..) => true, _ => false, } } @@ -444,23 +444,23 @@ where az as f64, euclid::Angle::radians(theta), ) - }, + } RotateX(theta) => { let theta = euclid::Angle::radians(TWO_PI - theta.radians64()); Transform3D::create_rotation(1., 0., 0., theta) - }, + } RotateY(theta) => { let theta = euclid::Angle::radians(TWO_PI - theta.radians64()); Transform3D::create_rotation(0., 1., 0., theta) - }, + } RotateZ(theta) | Rotate(theta) => { let theta = euclid::Angle::radians(TWO_PI - theta.radians64()); Transform3D::create_rotation(0., 0., 1., theta) - }, + } Perspective(ref d) => { let m = create_perspective_matrix(d.to_pixel_length(None)?); m.cast() - }, + } Scale3D(sx, sy, sz) => Transform3D::create_scale(sx.into(), sy.into(), sz.into()), Scale(sx, sy) => Transform3D::create_scale(sx.into(), sy.into(), 1.), ScaleX(s) => Transform3D::create_scale(s.into(), 1., 1.), @@ -470,23 +470,23 @@ where let tx = tx.to_pixel_length(reference_width)? as f64; let ty = ty.to_pixel_length(reference_height)? as f64; Transform3D::create_translation(tx, ty, tz.to_pixel_length(None)? as f64) - }, + } Translate(ref tx, ref ty) => { let tx = tx.to_pixel_length(reference_width)? as f64; let ty = ty.to_pixel_length(reference_height)? as f64; Transform3D::create_translation(tx, ty, 0.) - }, + } TranslateX(ref t) => { let t = t.to_pixel_length(reference_width)? as f64; Transform3D::create_translation(t, 0., 0.) - }, + } TranslateY(ref t) => { let t = t.to_pixel_length(reference_height)? as f64; Transform3D::create_translation(0., t, 0.) - }, + } TranslateZ(ref z) => { Transform3D::create_translation(0., 0., z.to_pixel_length(None)? as f64) - }, + } Skew(theta_x, theta_y) => Transform3D::create_skew( euclid::Angle::radians(theta_x.radians64()), euclid::Angle::radians(theta_y.radians64()), @@ -509,7 +509,7 @@ where // return an identity matrix. // Note: DOMMatrix doesn't go into this arm. Transform3D::identity() - }, + } }; Ok(matrix) } @@ -676,7 +676,7 @@ where } dest.write_char(' ')?; angle.to_css(dest) - }, + } } } } @@ -700,40 +700,52 @@ where pub enum GenericScale { /// 'none' None, - /// '{1,2}' - Scale(Number, Number), - /// '{3}' - Scale3D(Number, Number, Number), + /// '{1,3}' + Scale(Number, Number, Number), } pub use self::GenericScale as Scale; -impl ToCss for Scale { +impl ToCss for Scale +where + Number: ToCss + PartialEq + Copy, + f32: From, +{ fn to_css(&self, dest: &mut CssWriter) -> fmt::Result where W: fmt::Write, + f32: From, { match *self { Scale::None => dest.write_str("none"), - Scale::Scale(ref x, ref y) => { + Scale::Scale(ref x, ref y, ref z) => { x.to_css(dest)?; - if x != y { + + let is_3d = f32::from(*z) != 1.0; + if is_3d || x != y { dest.write_char(' ')?; y.to_css(dest)?; } + + if is_3d { + dest.write_char(' ')?; + z.to_css(dest)?; + } Ok(()) - }, - Scale::Scale3D(ref x, ref y, ref z) => { - x.to_css(dest)?; - dest.write_char(' ')?; - y.to_css(dest)?; - dest.write_char(' ')?; - z.to_css(dest) - }, + } } } } +#[inline] +fn y_axis_and_z_axis_are_zero( + _: &LengthPercentage, + y: &LengthPercentage, + z: &Length, +) -> bool { + y.is_zero() && z.is_zero() +} + #[derive( Clone, Debug, @@ -755,25 +767,24 @@ impl ToCss for Scale { /// or two values (per usual, if the second value is 0px, the default, it must /// be omitted when serializing). /// -/// If a 3d translation is specified, all three values must be serialized. -/// -/// We don't omit the 3rd component even if it is 0px for now, and the -/// related spec issue is https://github.com/w3c/csswg-drafts/issues/3305 +/// If a 3d translation is specified and the value can be expressed as 2d, we treat as 2d and +/// serialize accoringly. Otherwise, we serialize all three values. +/// https://github.com/w3c/csswg-drafts/issues/3305 /// /// pub enum GenericTranslate where LengthPercentage: Zero, + Length: Zero, { /// 'none' None, - /// '' or ' ' + /// [ ? ]? Translate( LengthPercentage, - #[css(skip_if = "Zero::is_zero")] LengthPercentage, + #[css(contextual_skip_if = "y_axis_and_z_axis_are_zero")] LengthPercentage, + #[css(skip_if = "Zero::is_zero")] Length, ), - /// ' ' - Translate3D(LengthPercentage, LengthPercentage, Length), } pub use self::GenericTranslate as Translate; diff --git a/components/style/values/specified/transform.rs b/components/style/values/specified/transform.rs index 157c6504856..6a33b665463 100644 --- a/components/style/values/specified/transform.rs +++ b/components/style/values/specified/transform.rs @@ -421,17 +421,22 @@ impl Parse for Translate { if let Ok(ty) = input.try(|i| specified::LengthPercentage::parse(context, i)) { if let Ok(tz) = input.try(|i| specified::Length::parse(context, i)) { // 'translate: ' - return Ok(generic::Translate::Translate3D(tx, ty, tz)); + return Ok(generic::Translate::Translate(tx, ty, tz)); } // translate: ' - return Ok(generic::Translate::Translate(tx, ty)); + return Ok(generic::Translate::Translate( + tx, + ty, + specified::Length::zero(), + )); } // 'translate: ' Ok(generic::Translate::Translate( tx, specified::LengthPercentage::zero(), + specified::Length::zero(), )) } } @@ -452,14 +457,14 @@ impl Parse for Scale { if let Ok(sy) = input.try(|i| Number::parse(context, i)) { if let Ok(sz) = input.try(|i| Number::parse(context, i)) { // 'scale: ' - return Ok(generic::Scale::Scale3D(sx, sy, sz)); + return Ok(generic::Scale::Scale(sx, sy, sz)); } // 'scale: ' - return Ok(generic::Scale::Scale(sx, sy)); + return Ok(generic::Scale::Scale(sx, sy, Number::new(1.0))); } // 'scale: ' - Ok(generic::Scale::Scale(sx, sx)) + Ok(generic::Scale::Scale(sx, sx, Number::new(1.0))) } } From 45c64a7224c3926a5c4de2af93f0ea6c731ca358 Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Sat, 16 Nov 2019 01:52:32 +0000 Subject: [PATCH 16/45] style: Set WillChangeBits::TRANSFORM for offset-path and add tests for it. Differential Revision: https://phabricator.services.mozilla.com/D53109 --- components/style/values/specified/box.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/components/style/values/specified/box.rs b/components/style/values/specified/box.rs index 2b4e3a420de..2173c8f5f2a 100644 --- a/components/style/values/specified/box.rs +++ b/components/style/values/specified/box.rs @@ -1151,7 +1151,9 @@ fn change_bits_for_longhand(longhand: LonghandId) -> WillChangeBits { LonghandId::Opacity => WillChangeBits::OPACITY, LonghandId::Transform => WillChangeBits::TRANSFORM, #[cfg(feature = "gecko")] - LonghandId::Translate | LonghandId::Rotate | LonghandId::Scale => WillChangeBits::TRANSFORM, + LonghandId::Translate | LonghandId::Rotate | LonghandId::Scale | LonghandId::OffsetPath => { + WillChangeBits::TRANSFORM + } _ => WillChangeBits::empty(), }; From 425025c2304c994ce0ab5018b1d23f687a087976 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 17 Nov 2019 23:28:39 +0000 Subject: [PATCH 17/45] style: Use only Origin during the cascade, rather than CascadeLevel. The micro-benchmark `style-attr-1.html` regressed slightly with my patch, after the CascadeLevel size increase. This benchmark is meant to test for the "changing the style attribute doesn't cause selector-matching" optimization (which, mind you, keeps working). But in the process it creates 10k rules which form a perfect path in the rule tree and that we put into a SmallVec during the cascade, and the benchmark spends most of the time pushing to that SmallVec and iterating the declarations (as there's only one property to apply). So we could argue that the regression is minor and is not what the benchark is supposed to be testing, but given I did the digging... :) My patch made CascadeLevel bigger, which means that we create a somewhat bigger vector in this case. Thankfully it also removed the dependency in the CascadeLevel, so we can stop using that and use just Origin which is one byte to revert the perf regression. Differential Revision: https://phabricator.services.mozilla.com/D53181 --- components/style/animation.rs | 4 +-- components/style/properties/cascade.rs | 36 ++++++++++++++------------ components/style/stylist.rs | 9 ++++--- 3 files changed, 26 insertions(+), 23 deletions(-) diff --git a/components/style/animation.rs b/components/style/animation.rs index c9b4fc5a6f1..7f96e55b18f 100644 --- a/components/style/animation.rs +++ b/components/style/animation.rs @@ -16,8 +16,8 @@ use crate::properties::animated_properties::AnimatedProperty; use crate::properties::longhands::animation_direction::computed_value::single_value::T as AnimationDirection; use crate::properties::longhands::animation_play_state::computed_value::single_value::T as AnimationPlayState; use crate::properties::{self, CascadeMode, ComputedValues, LonghandId}; -use crate::rule_tree::CascadeLevel; use crate::stylesheets::keyframes_rule::{KeyframesAnimation, KeyframesStep, KeyframesStepValue}; +use crate::stylesheets::Origin; use crate::timer::Timer; use crate::values::computed::box_::TransitionProperty; use crate::values::computed::Time; @@ -491,7 +491,7 @@ where guard .normal_declaration_iter() .filter(|declaration| declaration.is_animatable()) - .map(|decl| (decl, CascadeLevel::Animations)) + .map(|decl| (decl, Origin::Author)) }; // This currently ignores visited styles, which seems acceptable, diff --git a/components/style/properties/cascade.rs b/components/style/properties/cascade.rs index 5dc73b9abc1..a667cba09bc 100644 --- a/components/style/properties/cascade.rs +++ b/components/style/properties/cascade.rs @@ -15,7 +15,7 @@ use crate::properties::{LonghandId, LonghandIdSet, CSSWideKeyword}; use crate::properties::{PropertyDeclaration, PropertyDeclarationId, DeclarationImportanceIterator}; use crate::properties::CASCADE_PROPERTY; use crate::rule_cache::{RuleCache, RuleCacheConditions}; -use crate::rule_tree::{CascadeLevel, StrongRuleNode}; +use crate::rule_tree::StrongRuleNode; use crate::selector_parser::PseudoElement; use crate::stylesheets::{Origin, PerOrigin}; use servo_arc::Arc; @@ -134,11 +134,15 @@ where let restriction = pseudo.and_then(|p| p.property_restriction()); let iter_declarations = || { rule_node.self_and_ancestors().flat_map(|node| { - let cascade_level = node.cascade_level(); + let origin = node.cascade_level().origin(); let node_importance = node.importance(); + let guard = match origin { + Origin::Author => guards.author, + Origin::User | Origin::UserAgent => guards.ua_or_user, + }; let declarations = match node.style_source() { Some(source) => source - .read(cascade_level.guard(guards)) + .read(guard) .declaration_importance_iter(), None => DeclarationImportanceIterator::new(&[], &empty), }; @@ -153,14 +157,14 @@ where // longhands are only allowed if they have our // restriction flag set. if let PropertyDeclarationId::Longhand(id) = declaration.id() { - if !id.flags().contains(restriction) && cascade_level.origin() != Origin::UserAgent { + if !id.flags().contains(restriction) && origin != Origin::UserAgent { return None; } } } if declaration_importance == node_importance { - Some((declaration, cascade_level)) + Some((declaration, origin)) } else { None } @@ -223,7 +227,7 @@ pub fn apply_declarations<'a, E, F, I>( where E: TElement, F: Fn() -> I, - I: Iterator, + I: Iterator, { debug_assert!(layout_parent_style.is_none() || parent_style.is_some()); debug_assert_eq!( @@ -242,17 +246,17 @@ where let inherited_style = parent_style.unwrap_or(device.default_computed_values()); - let mut declarations = SmallVec::<[(&_, CascadeLevel); 32]>::new(); + let mut declarations = SmallVec::<[(&_, Origin); 32]>::new(); let custom_properties = { let mut builder = CustomPropertiesBuilder::new( inherited_style.custom_properties(), device.environment(), ); - for (declaration, cascade_level) in iter_declarations() { - declarations.push((declaration, cascade_level)); + for (declaration, origin) in iter_declarations() { + declarations.push((declaration, origin)); if let PropertyDeclaration::Custom(ref declaration) = *declaration { - builder.cascade(declaration, cascade_level.origin()); + builder.cascade(declaration, origin); } } @@ -332,7 +336,7 @@ where fn should_ignore_declaration_when_ignoring_document_colors( device: &Device, longhand_id: LonghandId, - cascade_level: CascadeLevel, + origin: Origin, pseudo: Option<&PseudoElement>, declaration: &mut Cow, ) -> bool { @@ -340,8 +344,7 @@ fn should_ignore_declaration_when_ignoring_document_colors( return false; } - let is_ua_or_user_rule = - matches!(cascade_level.origin(), Origin::User | Origin::UserAgent); + let is_ua_or_user_rule = matches!(origin, Origin::User | Origin::UserAgent); if is_ua_or_user_rule { return false; } @@ -446,7 +449,7 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { declarations: I, ) where Phase: CascadePhase, - I: Iterator, + I: Iterator, { let apply_reset = apply_reset == ApplyResetProperties::Yes; @@ -459,9 +462,8 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { let ignore_colors = !self.context.builder.device.use_document_colors(); - for (declaration, cascade_level) in declarations { + for (declaration, origin) in declarations { let declaration_id = declaration.id(); - let origin = cascade_level.origin(); let longhand_id = match declaration_id { PropertyDeclarationId::Longhand(id) => id, PropertyDeclarationId::Custom(..) => continue, @@ -509,7 +511,7 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { let should_ignore = should_ignore_declaration_when_ignoring_document_colors( self.context.builder.device, longhand_id, - cascade_level, + origin, self.context.builder.pseudo, &mut declaration, ); diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 414210b7d20..bd499027219 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -1322,16 +1322,17 @@ impl Stylist { let iter_declarations = || { block .declaration_importance_iter() - .map(|(declaration, importance)| { - debug_assert!(!importance.important()); - (declaration, CascadeLevel::same_tree_author_normal()) - }) + .map(|(declaration, _)| (declaration, Origin::Author)) }; let metrics = get_metrics_provider_for_product(); // We don't bother inserting these declarations in the rule tree, since // it'd be quite useless and slow. + // + // TODO(emilio): Now that we fixed bug 1493420, we should consider + // reversing this as it shouldn't be slow anymore, and should avoid + // generating two instantiations of apply_declarations. properties::apply_declarations::( &self.device, /* pseudo = */ None, From 246433acfa8caae242145326752b3d0d71c1f3d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 18 Nov 2019 08:49:27 +0000 Subject: [PATCH 18/45] style: Remove NODE_IS_ANONYMOUS_ROOT. We no longer have multiple kinds of anonymous subtrees, so we can get back one node bit. Differential Revision: https://phabricator.services.mozilla.com/D53344 --- components/style/gecko/wrapper.rs | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index f76c0c5fda6..ffa6855736e 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -761,13 +761,6 @@ impl<'le> GeckoElement<'le> { data.damage |= damage; } - /// This logic is duplicated in Gecko's nsIContent::IsRootOfAnonymousSubtree. - #[inline] - fn is_root_of_anonymous_subtree(&self) -> bool { - use crate::gecko_bindings::structs::NODE_IS_ANONYMOUS_ROOT; - self.flags() & (NODE_IS_ANONYMOUS_ROOT as u32) != 0 - } - /// This logic is duplicated in Gecko's nsIContent::IsRootOfNativeAnonymousSubtree. #[inline] fn is_root_of_native_anonymous_subtree(&self) -> bool { @@ -775,11 +768,6 @@ impl<'le> GeckoElement<'le> { return self.flags() & (NODE_IS_NATIVE_ANONYMOUS_ROOT as u32) != 0; } - #[inline] - fn is_in_anonymous_subtree(&self) -> bool { - unsafe { bindings::Gecko_IsInAnonymousSubtree(self.0) } - } - /// Returns true if this node is the shadow root of an use-element shadow tree. #[inline] fn is_root_of_use_element_shadow_tree(&self) -> bool { @@ -1028,7 +1016,7 @@ impl<'le> TElement for GeckoElement<'le> { // StyleChildrenIterator::IsNeeded does, except that it might return // true if we used to (but no longer) have anonymous content from // ::before/::after, or nsIAnonymousContentCreators. - if self.is_in_anonymous_subtree() || + if self.is_in_native_anonymous_subtree() || self.is_html_slot_element() || self.shadow_root().is_some() || self.may_have_anonymous_children() @@ -2243,7 +2231,7 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { #[inline] fn ignores_nth_child_selectors(&self) -> bool { - self.is_root_of_anonymous_subtree() + self.is_root_of_native_anonymous_subtree() } } From 9a43ad996f87f10a8c61fead3cbbf6460ca2f382 Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Mon, 18 Nov 2019 20:47:01 +0000 Subject: [PATCH 19/45] style: Update the expected direction vector of rotate property in wpt. For interpolations with the primitive rotate3d(), the direction vectors of the transform functions get normalized first. This should also be applied to rotate property. https://drafts.csswg.org/css-transforms-2/#interpolation-of-transform-functions Differential Revision: https://phabricator.services.mozilla.com/D52944 --- components/style/values/animated/transform.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/components/style/values/animated/transform.rs b/components/style/values/animated/transform.rs index 89bb3c0849d..5a6f3afb9c1 100644 --- a/components/style/values/animated/transform.rs +++ b/components/style/values/animated/transform.rs @@ -1249,7 +1249,11 @@ impl Animate for ComputedRotate { match (self, other) { (&Rotate::None, &Rotate::None) => Ok(Rotate::None), (&Rotate::Rotate3D(fx, fy, fz, fa), &Rotate::None) => { - // No need to normalize `none`, so animate angle directly. + // We always normalize direction vector for rotate3d() first, so we should also + // apply the same rule for rotate property. In other words, we promote none into + // a 3d rotate, and normalize both direction vector first, and then do + // interpolation. + let (fx, fy, fz, fa) = transform::get_normalized_vector_and_angle(fx, fy, fz, fa); Ok(Rotate::Rotate3D( fx, fy, @@ -1258,7 +1262,8 @@ impl Animate for ComputedRotate { )) } (&Rotate::None, &Rotate::Rotate3D(tx, ty, tz, ta)) => { - // No need to normalize `none`, so animate angle directly. + // Normalize direction vector first. + let (tx, ty, tz, ta) = transform::get_normalized_vector_and_angle(tx, ty, tz, ta); Ok(Rotate::Rotate3D( tx, ty, From a7c50b57a182bc58266e20a85dbdbed1aecbe2e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 19 Nov 2019 00:43:34 +0000 Subject: [PATCH 20/45] style: Handle logical shorthand animations with variable references correctly. When we physicalize the declarations for @keyframes, we end up having a physical declaration with an unparsed value with `from_shorthand` being the logical shorthand. Account for this case properly when substituting custom properties, to avoid panicking. Differential Revision: https://phabricator.services.mozilla.com/D53663 --- components/style/properties/data.py | 12 ++++---- .../style/properties/properties.mako.rs | 30 ++++++++++++------- 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/components/style/properties/data.py b/components/style/properties/data.py index d1876a5ea02..87ec87a7fa5 100644 --- a/components/style/properties/data.py +++ b/components/style/properties/data.py @@ -257,10 +257,12 @@ class Longhand(object): def type(): return "longhand" - # For a given logical property return all the physical - # property names corresponding to it. - def all_physical_mapped_properties(self): - assert self.logical + # For a given logical property return all the physical property names + # corresponding to it. + def all_physical_mapped_properties(self, data): + if not self.logical: + return [] + candidates = [s for s in LOGICAL_SIDES + LOGICAL_SIZES + LOGICAL_CORNERS if s in self.name] + [s for s in LOGICAL_AXES if self.name.endswith(s)] assert(len(candidates) == 1) @@ -270,7 +272,7 @@ class Longhand(object): else PHYSICAL_SIZES if logical_side in LOGICAL_SIZES \ else PHYSICAL_AXES if logical_side in LOGICAL_AXES \ else LOGICAL_CORNERS - return [to_phys(self.name, logical_side, physical_side) + return [data.longhands_by_name[to_phys(self.name, logical_side, physical_side)] for physical_side in physical] def experimental(self, engine): diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 37b2319dfd6..484b52e8c87 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -1651,13 +1651,25 @@ impl UnparsedValue { shorthands::${shorthand.ident}::parse_value(&context, input) .map(|longhands| { match longhand_id { + <% seen = set() %> % for property in shorthand.sub_properties: - LonghandId::${property.camel_case} => { - PropertyDeclaration::${property.camel_case}( - longhands.${property.ident} - ) - } + // When animating logical properties, we end up + // physicalizing the value during the animation, but + // the value still comes from the logical shorthand. + // + // So we need to handle the physical properties too. + % for prop in [property] + property.all_physical_mapped_properties(data): + % if prop.camel_case not in seen: + LonghandId::${prop.camel_case} => { + PropertyDeclaration::${prop.camel_case}( + longhands.${property.ident} + ) + } + <% seen.add(prop.camel_case) %> + % endif % endfor + % endfor + <% del seen %> _ => unreachable!() } }) @@ -2176,14 +2188,12 @@ impl PropertyDeclaration { let mut ret = self.clone(); % for prop in data.longhands: - % if prop.logical: - % for physical_property in prop.all_physical_mapped_properties(): - % if data.longhands_by_name[physical_property].specified_type() != prop.specified_type(): + % for physical_property in prop.all_physical_mapped_properties(data): + % if physical_property.specified_type() != prop.specified_type(): <% raise "Logical property %s should share specified value with physical property %s" % \ - (prop.name, physical_property) %> + (prop.name, physical_property.name) %> % endif % endfor - % endif % endfor unsafe { From d7167cec58ee2dc336d1037bc6ae0f8490bfa580 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 19 Nov 2019 06:47:12 +0000 Subject: [PATCH 21/45] style: Minor debugging improvements. These were useful when implementing forwarding, and forgot to send them earlier. Differential Revision: https://phabricator.services.mozilla.com/D53767 --- components/style/dom.rs | 2 +- components/style/gecko/wrapper.rs | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/components/style/dom.rs b/components/style/dom.rs index 2ea64bbb3dc..ef571b8df0a 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -331,7 +331,7 @@ where } /// The ShadowRoot trait. -pub trait TShadowRoot: Sized + Copy + Clone + PartialEq { +pub trait TShadowRoot: Sized + Copy + Clone + Debug + PartialEq { /// The concrete node type. type ConcreteNode: TNode; diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index ffa6855736e..bd110801c05 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -144,6 +144,13 @@ impl<'ld> TDocument for GeckoDocument<'ld> { #[derive(Clone, Copy)] pub struct GeckoShadowRoot<'lr>(pub &'lr structs::ShadowRoot); +impl<'ln> fmt::Debug for GeckoShadowRoot<'ln> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + // TODO(emilio): Maybe print the host or something? + write!(f, " ({:#x})", self.as_node().opaque().0) + } +} + impl<'lr> PartialEq for GeckoShadowRoot<'lr> { #[inline] fn eq(&self, other: &Self) -> bool { @@ -232,8 +239,8 @@ impl<'ln> fmt::Debug for GeckoNode<'ln> { return write!(f, " ({:#x})", self.opaque().0); } - if self.is_shadow_root() { - return write!(f, " ({:#x})", self.opaque().0); + if let Some(sr) = self.as_shadow_root() { + return sr.fmt(f); } write!(f, " ({:#x})", self.opaque().0) From fe93be82d29c2b6bcb90a29df437a710e3f0479b Mon Sep 17 00:00:00 2001 From: jeffin143 Date: Wed, 20 Nov 2019 00:49:19 +0000 Subject: [PATCH 22/45] style: use Enum class for NS_STYLE_SHAPE_RENDERING instead of #define. Differential Revision: https://phabricator.services.mozilla.com/D53840 --- components/style/properties/longhands/inherited_svg.mako.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/components/style/properties/longhands/inherited_svg.mako.rs b/components/style/properties/longhands/inherited_svg.mako.rs index bbb7eba1c5b..487456cbe12 100644 --- a/components/style/properties/longhands/inherited_svg.mako.rs +++ b/components/style/properties/longhands/inherited_svg.mako.rs @@ -80,6 +80,7 @@ ${helpers.single_keyword( engines="gecko", animation_value_type="discrete", spec="https://www.w3.org/TR/SVG11/painting.html#ShapeRenderingProperty", + gecko_enum_prefix = "StyleShapeRendering", )} ${helpers.predefined_type( From e48c4b88f119cfc8287581688a3995e45bbdb15b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 20 Nov 2019 05:46:56 +0000 Subject: [PATCH 23/45] style: Make rust Atom use NonZeroUsize. At first I thought this was going to enable simplifications in the selector parser (to simplify the attribute selector setup), but I couldn't end up shrinking the layout enough. However this should help with bug 1559076, which returns Option, and it was easy to write. Differential Revision: https://phabricator.services.mozilla.com/D53766 --- components/style/gecko_string_cache/mod.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/components/style/gecko_string_cache/mod.rs b/components/style/gecko_string_cache/mod.rs index b209f316d09..5d849a96f6e 100644 --- a/components/style/gecko_string_cache/mod.rs +++ b/components/style/gecko_string_cache/mod.rs @@ -26,6 +26,7 @@ use std::fmt::{self, Write}; use std::hash::{Hash, Hasher}; use std::iter::Cloned; use std::mem::{self, ManuallyDrop}; +use std::num::NonZeroUsize; use std::ops::Deref; use std::{slice, str}; use style_traits::SpecifiedValueInfo; @@ -54,7 +55,7 @@ macro_rules! local_name { /// or an offset from gGkAtoms to the nsStaticAtom object. #[derive(Eq, PartialEq)] #[repr(C)] -pub struct Atom(usize); +pub struct Atom(NonZeroUsize); /// An atom *without* a strong reference. /// @@ -101,9 +102,9 @@ impl Deref for Atom { fn deref(&self) -> &WeakAtom { unsafe { let addr = if self.is_static() { - (&gGkAtoms as *const _ as usize) + (self.0 >> 1) + (&gGkAtoms as *const _ as usize) + (self.0.get() >> 1) } else { - self.0 + self.0.get() }; debug_assert!(!self.is_static() || valid_static_atom_addr(addr)); WeakAtom::new(addr as *const nsAtom) @@ -341,29 +342,29 @@ impl fmt::Display for WeakAtom { } #[inline] -unsafe fn make_handle(ptr: *const nsAtom) -> usize { +unsafe fn make_handle(ptr: *const nsAtom) -> NonZeroUsize { debug_assert!(!ptr.is_null()); if !WeakAtom::new(ptr).is_static() { - ptr as usize + NonZeroUsize::new_unchecked(ptr as usize) } else { make_static_handle(ptr as *mut nsStaticAtom) } } #[inline] -unsafe fn make_static_handle(ptr: *const nsStaticAtom) -> usize { +unsafe fn make_static_handle(ptr: *const nsStaticAtom) -> NonZeroUsize { // FIXME(heycam): Use offset_from once it's stabilized. // https://github.com/rust-lang/rust/issues/41079 debug_assert!(valid_static_atom_addr(ptr as usize)); let base = &gGkAtoms as *const _; let offset = ptr as usize - base as usize; - (offset << 1) | 1 + NonZeroUsize::new_unchecked((offset << 1) | 1) } impl Atom { #[inline] fn is_static(&self) -> bool { - self.0 & 1 == 1 + self.0.get() & 1 == 1 } /// Execute a callback with the atom represented by `ptr`. From 0c44382348ee78c823a3c01eaa722f7556e3a25a Mon Sep 17 00:00:00 2001 From: jeffin143 Date: Thu, 21 Nov 2019 06:07:30 +0000 Subject: [PATCH 24/45] style: convert NS_STYLE_STROKE_LINECAP_* to an enum class in nsStyleConsts.h Differential Revision: https://phabricator.services.mozilla.com/D53908 --- components/style/properties/longhands/inherited_svg.mako.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/components/style/properties/longhands/inherited_svg.mako.rs b/components/style/properties/longhands/inherited_svg.mako.rs index 487456cbe12..4ad94b6fb63 100644 --- a/components/style/properties/longhands/inherited_svg.mako.rs +++ b/components/style/properties/longhands/inherited_svg.mako.rs @@ -108,6 +108,7 @@ ${helpers.single_keyword( engines="gecko", animation_value_type="discrete", spec="https://www.w3.org/TR/SVG11/painting.html#StrokeLinecapProperty", + gecko_enum_prefix = "StyleStrokeLinecap", )} ${helpers.single_keyword( From 576883d5389c87686bd2bf5b7ff84f7575bc4d2a Mon Sep 17 00:00:00 2001 From: jeffin143 Date: Thu, 21 Nov 2019 08:48:19 +0000 Subject: [PATCH 25/45] style: convert NS_STYLE_TEXT_ANCHOR_* to an enum class in nsStyleConsts.h Differential Revision: https://phabricator.services.mozilla.com/D53956 --- components/style/properties/longhands/inherited_svg.mako.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/components/style/properties/longhands/inherited_svg.mako.rs b/components/style/properties/longhands/inherited_svg.mako.rs index 4ad94b6fb63..1d75df08bd8 100644 --- a/components/style/properties/longhands/inherited_svg.mako.rs +++ b/components/style/properties/longhands/inherited_svg.mako.rs @@ -25,6 +25,7 @@ ${helpers.single_keyword( engines="gecko", animation_value_type="discrete", spec="https://www.w3.org/TR/SVG/text.html#TextAnchorProperty", + gecko_enum_prefix="StyleTextAnchor", )} // Section 11 - Painting: Filling, Stroking and Marker Symbols From e3009a4de9186c648c0a3beef4fdde6ce66b1a3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 21 Nov 2019 10:32:32 +0000 Subject: [PATCH 26/45] style: Implement shadow part forwarding (minus invalidation). Some of the stuff, in particular inside GeckoBindings stuff should be refactored to be less ugly and duplicate a bit less code, but the rest of the code should be landable as is. Some invalidation changes are already needed because we weren't matching with the right shadow host during invalidation (which made existing ::part() tests fail). Pending invalidation work: * Making exportparts work right on the snapshots. * Invalidating parts from descendant hosts. They're not very hard but I need to think how to best implement it: * Maybe get rid of ShadowRoot::mParts and just walk DOM descendants in the Shadow DOM. * Maybe implement a ElementHasExportPartsAttr much like HasPartAttr and use that to keep the list of elements. * Maybe invalidate :host and ::part() together in here[1] * Maybe something else. Opinions? [1]: https://searchfox.org/mozilla-central/rev/131338e5017bc0283d86fb73844407b9a2155c98/servo/components/style/invalidation/element/invalidator.rs#561 Differential Revision: https://phabricator.services.mozilla.com/D53730 --- components/selectors/matching.rs | 37 ++++++- components/selectors/tree.rs | 14 +++ components/style/dom_apis.rs | 6 +- components/style/gecko/wrapper.rs | 22 +++++ .../invalidation/element/document_state.rs | 8 +- .../invalidation/element/element_wrapper.rs | 10 ++ .../style/invalidation/element/invalidator.rs | 31 ++++-- .../element/state_and_attributes.rs | 1 + components/style/rule_collector.rs | 98 ++++++++++++------- 9 files changed, 180 insertions(+), 47 deletions(-) diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs index 49b7f3dbd32..8993a5ea003 100644 --- a/components/selectors/matching.rs +++ b/components/selectors/matching.rs @@ -8,6 +8,7 @@ use crate::nth_index_cache::NthIndexCacheInner; use crate::parser::{AncestorHashes, Combinator, Component, LocalName}; use crate::parser::{NonTSPseudoClass, Selector, SelectorImpl, SelectorIter, SelectorList}; use crate::tree::Element; +use smallvec::SmallVec; use std::borrow::Borrow; use std::iter; @@ -667,7 +668,41 @@ where match *selector { Component::Combinator(_) => unreachable!(), - Component::Part(ref parts) => parts.iter().all(|part| element.is_part(part)), + Component::Part(ref parts) => { + let mut hosts = SmallVec::<[E; 4]>::new(); + + let mut host = match element.containing_shadow_host() { + Some(h) => h, + None => return false, + }; + + loop { + let outer_host = host.containing_shadow_host(); + if outer_host.as_ref().map(|h| h.opaque()) == context.shared.current_host { + break; + } + let outer_host = match outer_host { + Some(h) => h, + None => return false, + }; + // TODO(emilio): if worth it, we could early return if + // host doesn't have the exportparts attribute. + hosts.push(host); + host = outer_host; + } + + // Translate the part into the right scope. + parts.iter().all(|part| { + let mut part = part.clone(); + for host in hosts.iter().rev() { + part = match host.imported_part(&part) { + Some(p) => p, + None => return false, + }; + } + element.is_part(&part) + }) + }, Component::Slotted(ref selector) => { // are never flattened tree slottables. !element.is_html_slot_element() && diff --git a/components/selectors/tree.rs b/components/selectors/tree.rs index f892abd4e94..d6198c5a5f5 100644 --- a/components/selectors/tree.rs +++ b/components/selectors/tree.rs @@ -117,6 +117,20 @@ pub trait Element: Sized + Clone + Debug { case_sensitivity: CaseSensitivity, ) -> bool; + /// Returns the mapping from the `exportparts` attribute in the regular + /// direction, that is, inner-tree -> outer-tree. + fn exported_part( + &self, + name: &::PartName, + ) -> Option<::PartName>; + + /// Returns the mapping from the `exportparts` attribute in the reverse + /// direction, that is, in an outer-tree -> inner-tree direction. + fn imported_part( + &self, + name: &::PartName, + ) -> Option<::PartName>; + fn is_part(&self, name: &::PartName) -> bool; /// Returns whether this element matches `:empty`. diff --git a/components/style/dom_apis.rs b/components/style/dom_apis.rs index e43c861271e..8f764dc8958 100644 --- a/components/style/dom_apis.rs +++ b/components/style/dom_apis.rs @@ -172,7 +172,11 @@ where }; for selector in self.selector_list.0.iter() { - target_vector.push(Invalidation::new(selector, 0)) + target_vector.push(Invalidation::new( + selector, + self.matching_context.current_host.clone(), + 0, + )) } false diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index bd110801c05..2d1236ce698 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -2216,6 +2216,28 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { snapshot_helpers::has_class_or_part(name, CaseSensitivity::CaseSensitive, attr) } + #[inline] + fn imported_part(&self, name: &Atom) -> Option { + let imported = unsafe { + bindings::Gecko_Element_ImportedPart(self.0, name.as_ptr()) + }; + if imported.is_null() { + return None; + } + Some(unsafe { Atom::from_raw(imported) }) + } + + #[inline] + fn exported_part(&self, name: &Atom) -> Option { + let exported = unsafe { + bindings::Gecko_Element_ExportedPart(self.0, name.as_ptr()) + }; + if exported.is_null() { + return None; + } + Some(unsafe { Atom::from_raw(exported) }) + } + #[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/document_state.rs b/components/style/invalidation/element/document_state.rs index 4e29e91eb0e..7fc51338b1e 100644 --- a/components/style/invalidation/element/document_state.rs +++ b/components/style/invalidation/element/document_state.rs @@ -79,7 +79,13 @@ where continue; } - self_invalidations.push(Invalidation::new(&dependency.selector, 0)); + // We pass `None` as a scope, as document state selectors aren't + // affected by the current scope. + self_invalidations.push(Invalidation::new( + &dependency.selector, + /* scope = */ None, + 0, + )); } } diff --git a/components/style/invalidation/element/element_wrapper.rs b/components/style/invalidation/element/element_wrapper.rs index b8e29095d24..e17e42a31c0 100644 --- a/components/style/invalidation/element/element_wrapper.rs +++ b/components/style/invalidation/element/element_wrapper.rs @@ -365,6 +365,16 @@ where } } + fn exported_part(&self, name: &Atom) -> Option { + // FIXME(emilio): Implement for proper invalidation. + self.element.exported_part(name) + } + + fn imported_part(&self, name: &Atom) -> Option { + // FIXME(emilio): Implement for proper invalidation. + self.element.imported_part(name) + } + 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/invalidator.rs b/components/style/invalidation/element/invalidator.rs index 3487d7b9353..9e8309396cd 100644 --- a/components/style/invalidation/element/invalidator.rs +++ b/components/style/invalidation/element/invalidator.rs @@ -8,6 +8,7 @@ use crate::context::StackLimitChecker; use crate::dom::{TElement, TNode, TShadowRoot}; use crate::selector_parser::SelectorImpl; +use selectors::OpaqueElement; use selectors::matching::matches_compound_selector_from; use selectors::matching::{CompoundSelectorMatchingResult, MatchingContext}; use selectors::parser::{Combinator, Component, Selector}; @@ -127,6 +128,8 @@ enum InvalidationKind { #[derive(Clone)] pub struct Invalidation<'a> { selector: &'a Selector, + /// The right shadow host from where the rule came from, if any. + scope: Option, /// The offset of the selector pointing to a compound selector. /// /// This order is a "parse order" offset, that is, zero is the leftmost part @@ -143,9 +146,14 @@ pub struct Invalidation<'a> { impl<'a> Invalidation<'a> { /// Create a new invalidation for a given selector and offset. - pub fn new(selector: &'a Selector, offset: usize) -> Self { + pub fn new( + selector: &'a Selector, + scope: Option, + offset: usize, + ) -> Self { Self { selector, + scope, offset, matched_by_any_previous: false, } @@ -483,6 +491,9 @@ where let mut any = false; let mut sibling_invalidations = InvalidationVector::new(); + + // FIXME(emilio): We also need to invalidate parts in descendant shadow + // hosts that have exportparts attributes. for element in shadow.parts() { any |= self.invalidate_child( *element, @@ -721,12 +732,17 @@ where self.element, invalidation, invalidation_kind ); - let matching_result = matches_compound_selector_from( - &invalidation.selector, - invalidation.offset, - self.processor.matching_context(), - &self.element, - ); + let matching_result = { + let mut context = self.processor.matching_context(); + context.current_host = invalidation.scope; + + matches_compound_selector_from( + &invalidation.selector, + invalidation.offset, + context, + &self.element, + ) + }; let mut invalidated_self = false; let mut matched = false; @@ -809,6 +825,7 @@ where let next_invalidation = Invalidation { selector: invalidation.selector, + scope: invalidation.scope, offset: next_combinator_offset + 1, matched_by_any_previous: false, }; diff --git a/components/style/invalidation/element/state_and_attributes.rs b/components/style/invalidation/element/state_and_attributes.rs index cb3a5525e39..e678413218c 100644 --- a/components/style/invalidation/element/state_and_attributes.rs +++ b/components/style/invalidation/element/state_and_attributes.rs @@ -457,6 +457,7 @@ where let invalidation = Invalidation::new( &dependency.selector, + self.matching_context.current_host.clone(), dependency.selector.len() - dependency.selector_offset + 1, ); diff --git a/components/style/rule_collector.rs b/components/style/rule_collector.rs index 031a92ca728..c0197572e0a 100644 --- a/components/style/rule_collector.rs +++ b/components/style/rule_collector.rs @@ -4,6 +4,7 @@ //! Collects a series of applicable rules for a given element. +use crate::Atom; use crate::applicable_declarations::{ApplicableDeclarationBlock, ApplicableDeclarationList}; use crate::dom::{TElement, TNode, TShadowRoot}; use crate::properties::{AnimationRules, PropertyDeclarationBlock}; @@ -328,52 +329,75 @@ where return; } - let shadow = match self.rule_hash_target.containing_shadow() { + let mut inner_shadow = match self.rule_hash_target.containing_shadow() { Some(s) => s, None => return, }; - let host = shadow.host(); - let containing_shadow = host.containing_shadow(); - let part_rules = match containing_shadow { - Some(shadow) => shadow - .style_data() - .and_then(|data| data.part_rules(self.pseudo_element)), - None => self - .stylist - .cascade_data() - .borrow_for_origin(Origin::Author) - .part_rules(self.pseudo_element), - }; + let mut shadow_cascade_order = ShadowCascadeOrder::for_innermost_containing_tree(); - // TODO(emilio): Cascade order will need to increment for each tree when - // we implement forwarding. - let shadow_cascade_order = ShadowCascadeOrder::for_innermost_containing_tree(); - if let Some(part_rules) = part_rules { - let containing_host = containing_shadow.map(|s| s.host()); - let element = self.element; - let rule_hash_target = self.rule_hash_target; - let rules = &mut self.rules; - let flags_setter = &mut self.flags_setter; - let cascade_level = CascadeLevel::AuthorNormal { - shadow_cascade_order, + let mut parts = SmallVec::<[Atom; 3]>::new(); + self.rule_hash_target.each_part(|p| parts.push(p.clone())); + + loop { + if parts.is_empty() { + return; + } + + let outer_shadow = inner_shadow.host().containing_shadow(); + let part_rules = match outer_shadow { + Some(shadow) => shadow + .style_data() + .and_then(|data| data.part_rules(self.pseudo_element)), + None => self + .stylist + .cascade_data() + .borrow_for_origin(Origin::Author) + .part_rules(self.pseudo_element), }; - let start = rules.len(); - self.context.with_shadow_host(containing_host, |context| { - rule_hash_target.each_part(|p| { - if let Some(part_rules) = part_rules.get(p) { - SelectorMap::get_matching_rules( - element, - &part_rules, - rules, - context, - flags_setter, - cascade_level, - ); + + if let Some(part_rules) = part_rules { + let containing_host = outer_shadow.map(|s| s.host()); + let element = self.element; + let rules = &mut self.rules; + let flags_setter = &mut self.flags_setter; + let cascade_level = CascadeLevel::AuthorNormal { + shadow_cascade_order, + }; + let start = rules.len(); + self.context.with_shadow_host(containing_host, |context| { + for p in &parts { + if let Some(part_rules) = part_rules.get(p) { + SelectorMap::get_matching_rules( + element, + &part_rules, + rules, + context, + flags_setter, + cascade_level, + ); + } } }); + sort_rules_from(rules, start); + shadow_cascade_order.inc(); + } + + let inner_shadow_host = inner_shadow.host(); + + inner_shadow = match outer_shadow { + Some(s) => s, + None => break, // Nowhere to export to. + }; + + parts.retain(|part| { + let exported_part = match inner_shadow_host.exported_part(part) { + Some(part) => part, + None => return false, + }; + std::mem::replace(part, exported_part); + true }); - sort_rules_from(rules, start); } } From f8ceb5cb8409f434ec4acec8ab9c186bea45bcbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 21 Nov 2019 10:32:10 +0000 Subject: [PATCH 27/45] style: Invalidate parts in nested shadow trees correctly. Differential Revision: https://phabricator.services.mozilla.com/D54010 --- components/style/dom.rs | 3 + components/style/gecko/snapshot.rs | 10 +++ components/style/gecko/snapshot_helpers.rs | 20 ++++++ components/style/gecko/wrapper.rs | 28 +++----- .../invalidation/element/element_wrapper.rs | 18 +++-- .../style/invalidation/element/invalidator.rs | 68 +++++++++++++------ 6 files changed, 104 insertions(+), 43 deletions(-) diff --git a/components/style/dom.rs b/components/style/dom.rs index ef571b8df0a..fbe35e2e49d 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -523,6 +523,9 @@ pub trait TElement: /// Returns whether this element has a `part` attribute. fn has_part_attr(&self) -> bool; + /// Returns whether this element exports any part from its shadow tree. + fn exports_any_part(&self) -> bool; + /// The ID for this element. fn id(&self) -> Option<&WeakAtom>; diff --git a/components/style/gecko/snapshot.rs b/components/style/gecko/snapshot.rs index cffb78d3e9e..02707682b4d 100644 --- a/components/style/gecko/snapshot.rs +++ b/components/style/gecko/snapshot.rs @@ -193,6 +193,16 @@ impl ElementSnapshot for GeckoElementSnapshot { snapshot_helpers::has_class_or_part(name, CaseSensitivity::CaseSensitive, attr) } + #[inline] + fn exported_part(&self, name: &Atom) -> Option { + snapshot_helpers::exported_part(&*self.mAttrs, name) + } + + #[inline] + fn imported_part(&self, name: &Atom) -> Option { + snapshot_helpers::imported_part(&*self.mAttrs, name) + } + #[inline] fn has_class(&self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool { if !self.has_any(Flags::MaybeClass) { diff --git a/components/style/gecko/snapshot_helpers.rs b/components/style/gecko/snapshot_helpers.rs index b8b31bc87dd..24b3ebb1991 100644 --- a/components/style/gecko/snapshot_helpers.rs +++ b/components/style/gecko/snapshot_helpers.rs @@ -82,6 +82,26 @@ pub fn get_id(attrs: &[structs::AttrArray_InternalAttr]) -> Option<&WeakAtom> { Some(unsafe { get_id_from_attr(find_attr(attrs, &atom!("id"))?) }) } +#[inline(always)] +pub(super) fn exported_part(attrs: &[structs::AttrArray_InternalAttr], name: &Atom) -> Option { + let attr = find_attr(attrs, &atom!("exportparts"))?; + let atom = unsafe { bindings::Gecko_Element_ExportedPart(attr, name.as_ptr()) }; + if atom.is_null() { + return None; + } + Some(unsafe { Atom::from_raw(atom) }) +} + +#[inline(always)] +pub(super) fn imported_part(attrs: &[structs::AttrArray_InternalAttr], name: &Atom) -> Option { + let attr = find_attr(attrs, &atom!("exportparts"))?; + let atom = unsafe { bindings::Gecko_Element_ImportedPart(attr, name.as_ptr()) }; + if atom.is_null() { + return None; + } + Some(unsafe { Atom::from_raw(atom) }) +} + /// Given a class or part name, a case sensitivity, and an array of attributes, /// returns whether the attribute has that name. #[inline(always)] diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 2d1236ce698..ac47927744b 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -1244,8 +1244,12 @@ impl<'le> TElement for GeckoElement<'le> { #[inline] fn has_part_attr(&self) -> bool { - self.as_node() - .get_bool_flag(nsINode_BooleanFlag::ElementHasPart) + self.as_node().get_bool_flag(nsINode_BooleanFlag::ElementHasPart) + } + + #[inline] + fn exports_any_part(&self) -> bool { + snapshot_helpers::find_attr(self.attrs(), &atom!("exportparts")).is_some() } // FIXME(emilio): we should probably just return a reference to the Atom. @@ -2217,25 +2221,13 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { } #[inline] - fn imported_part(&self, name: &Atom) -> Option { - let imported = unsafe { - bindings::Gecko_Element_ImportedPart(self.0, name.as_ptr()) - }; - if imported.is_null() { - return None; - } - Some(unsafe { Atom::from_raw(imported) }) + fn exported_part(&self, name: &Atom) -> Option { + snapshot_helpers::exported_part(self.attrs(), name) } #[inline] - fn exported_part(&self, name: &Atom) -> Option { - let exported = unsafe { - bindings::Gecko_Element_ExportedPart(self.0, name.as_ptr()) - }; - if exported.is_null() { - return None; - } - Some(unsafe { Atom::from_raw(exported) }) + fn imported_part(&self, name: &Atom) -> Option { + snapshot_helpers::imported_part(self.attrs(), name) } #[inline(always)] diff --git a/components/style/invalidation/element/element_wrapper.rs b/components/style/invalidation/element/element_wrapper.rs index e17e42a31c0..bc74527bf16 100644 --- a/components/style/invalidation/element/element_wrapper.rs +++ b/components/style/invalidation/element/element_wrapper.rs @@ -62,6 +62,12 @@ pub trait ElementSnapshot: Sized { /// called if `has_attrs()` returns true. fn is_part(&self, name: &Atom) -> bool; + /// See Element::exported_part. + fn exported_part(&self, name: &Atom) -> Option; + + /// See Element::imported_part. + fn imported_part(&self, name: &Atom) -> Option; + /// 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) @@ -366,13 +372,17 @@ where } fn exported_part(&self, name: &Atom) -> Option { - // FIXME(emilio): Implement for proper invalidation. - self.element.exported_part(name) + match self.snapshot() { + Some(snapshot) if snapshot.has_attrs() => snapshot.exported_part(name), + _ => self.element.exported_part(name), + } } fn imported_part(&self, name: &Atom) -> Option { - // FIXME(emilio): Implement for proper invalidation. - self.element.imported_part(name) + match self.snapshot() { + Some(snapshot) if snapshot.has_attrs() => snapshot.imported_part(name), + _ => self.element.imported_part(name), + } } fn has_class(&self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool { diff --git a/components/style/invalidation/element/invalidator.rs b/components/style/invalidation/element/invalidator.rs index 9e8309396cd..8bc898f9587 100644 --- a/components/style/invalidation/element/invalidator.rs +++ b/components/style/invalidation/element/invalidator.rs @@ -129,6 +129,10 @@ enum InvalidationKind { pub struct Invalidation<'a> { selector: &'a Selector, /// The right shadow host from where the rule came from, if any. + /// + /// This is needed to ensure that we match the selector with the right + /// state, as whether some selectors like :host and ::part() match depends + /// on it. scope: Option, /// The offset of the selector pointing to a compound selector. /// @@ -479,6 +483,47 @@ where any_descendant } + fn invalidate_parts_in_shadow_tree( + &mut self, + shadow: ::ConcreteShadowRoot, + invalidations: &[Invalidation<'b>], + ) -> bool { + debug_assert!(!invalidations.is_empty()); + + let mut any = false; + let mut sibling_invalidations = InvalidationVector::new(); + + for node in shadow.as_node().dom_descendants() { + let element = match node.as_element() { + Some(e) => e, + None => continue, + }; + + if element.has_part_attr() { + any |= self.invalidate_child( + element, + invalidations, + &mut sibling_invalidations, + DescendantInvalidationKind::Part, + ); + debug_assert!( + sibling_invalidations.is_empty(), + "::part() shouldn't have sibling combinators to the right, \ + this makes no sense! {:?}", + sibling_invalidations + ); + } + + if let Some(shadow) = element.shadow_root() { + if element.exports_any_part() { + any |= self.invalidate_parts_in_shadow_tree(shadow, invalidations) + } + } + } + + any + } + fn invalidate_parts(&mut self, invalidations: &[Invalidation<'b>]) -> bool { if invalidations.is_empty() { return false; @@ -489,26 +534,7 @@ where None => return false, }; - let mut any = false; - let mut sibling_invalidations = InvalidationVector::new(); - - // FIXME(emilio): We also need to invalidate parts in descendant shadow - // hosts that have exportparts attributes. - for element in shadow.parts() { - any |= self.invalidate_child( - *element, - invalidations, - &mut sibling_invalidations, - DescendantInvalidationKind::Part, - ); - debug_assert!( - sibling_invalidations.is_empty(), - "::part() shouldn't have sibling combinators to the right, \ - this makes no sense! {:?}", - sibling_invalidations - ); - } - any + self.invalidate_parts_in_shadow_tree(shadow, invalidations) } fn invalidate_slotted_elements(&mut self, invalidations: &[Invalidation<'b>]) -> bool { @@ -733,7 +759,7 @@ where ); let matching_result = { - let mut context = self.processor.matching_context(); + let context = self.processor.matching_context(); context.current_host = invalidation.scope; matches_compound_selector_from( From 5582de5d7eb89d948d8175d9cda679ecb9f85e3e Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Wed, 20 Nov 2019 22:38:26 +0000 Subject: [PATCH 28/45] style: Add a preference for offset-path:ray(). Differential Revision: https://phabricator.services.mozilla.com/D53110 --- components/style/values/specified/motion.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/components/style/values/specified/motion.rs b/components/style/values/specified/motion.rs index 90652b5ed64..2351acecc6a 100644 --- a/components/style/values/specified/motion.rs +++ b/components/style/values/specified/motion.rs @@ -16,11 +16,24 @@ use style_traits::{ParseError, StyleParseErrorKind}; /// The specified value of `offset-path`. pub type OffsetPath = GenericOffsetPath; +#[cfg(feature = "gecko")] +fn is_ray_enabled() -> bool { + static_prefs::pref!("layout.css.motion-path-ray.enabled") +} +#[cfg(feature = "servo")] +fn is_ray_enabled() -> bool { + false +} + impl Parse for RayFunction { fn parse<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { + if !is_ray_enabled() { + return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)); + } + let mut angle = None; let mut size = None; let mut contain = false; From 014c41f54aceeaf59a9f00f91f9f32217fc40fb5 Mon Sep 17 00:00:00 2001 From: Mats Palmgren Date: Sat, 23 Nov 2019 19:56:34 +0000 Subject: [PATCH 29/45] style: Don't accept two trailing in the grid/grid-template shorthands. Differential Revision: https://phabricator.services.mozilla.com/D53909 --- .../properties/shorthands/position.mako.rs | 38 ++++++++++++------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/components/style/properties/shorthands/position.mako.rs b/components/style/properties/shorthands/position.mako.rs index 81a54dbfa57..3e0ef26133a 100644 --- a/components/style/properties/shorthands/position.mako.rs +++ b/components/style/properties/shorthands/position.mako.rs @@ -301,27 +301,37 @@ % endfor let first_line_names = input.try(parse_line_names).unwrap_or_default(); - if let Ok(mut string) = input.try(|i| i.expect_string().map(|s| s.as_ref().to_owned().into())) { + if let Ok(string) = input.try(|i| i.expect_string().map(|s| s.as_ref().to_owned().into())) { let mut strings = vec![]; let mut values = vec![]; let mut line_names = vec![]; - let mut names = first_line_names; + line_names.push(first_line_names); + strings.push(string); loop { - line_names.push(names); - strings.push(string); let size = input.try(|i| TrackSize::parse(context, i)).unwrap_or_default(); values.push(TrackListValue::TrackSize(size)); - names = input.try(parse_line_names).unwrap_or_default(); - if let Ok(v) = input.try(parse_line_names) { - let mut names_vec = names.into_vec(); - names_vec.extend(v.into_iter()); - names = names_vec.into(); - } + let mut names = input.try(parse_line_names).unwrap_or_default(); + let more_names = input.try(parse_line_names); - string = match input.try(|i| i.expect_string().map(|s| s.as_ref().to_owned().into())) { - Ok(s) => s, - _ => { // only the named area determines whether we should bail out - line_names.push(names.into()); + match input.try(|i| i.expect_string().map(|s| s.as_ref().to_owned().into())) { + Ok(string) => { + strings.push(string); + if let Ok(v) = more_names { + // We got `[names] [more_names] "string"` - merge the two name lists. + let mut names_vec = names.into_vec(); + names_vec.extend(v.into_iter()); + names = names_vec.into(); + } + line_names.push(names); + }, + Err(e) => { + if more_names.is_ok() { + // We've parsed `"string" [names] [more_names]` but then failed to parse another `"string"`. + // The grammar doesn't allow two trailing `` so this is an invalid value. + return Err(e.into()); + } + // only the named area determines whether we should bail out + line_names.push(names); break }, }; From 70ec6ffe36f1a34f66cbe1b4a2c1a36cf21b37c2 Mon Sep 17 00:00:00 2001 From: Mats Palmgren Date: Sat, 23 Nov 2019 19:58:35 +0000 Subject: [PATCH 30/45] style: Make some grid/grid-template values serialize to a shorter form. Differential Revision: https://phabricator.services.mozilla.com/D53913 --- .../properties/shorthands/position.mako.rs | 25 +++++---- components/style/values/generics/grid.rs | 53 +++++++++++++++---- components/style/values/specified/grid.rs | 4 +- 3 files changed, 62 insertions(+), 20 deletions(-) diff --git a/components/style/properties/shorthands/position.mako.rs b/components/style/properties/shorthands/position.mako.rs index 3e0ef26133a..5c884243c98 100644 --- a/components/style/properties/shorthands/position.mako.rs +++ b/components/style/properties/shorthands/position.mako.rs @@ -360,7 +360,7 @@ value } else { - GenericGridTemplateComponent::None + GridTemplateComponent::default() }; Ok(( @@ -409,6 +409,9 @@ W: Write { match *template_areas { GridTemplateAreas::None => { + if template_rows.is_initial() && template_columns.is_initial() { + return GridTemplateComponent::default().to_css(dest); + } template_rows.to_css(dest)?; dest.write_str(" / ")?; template_columns.to_css(dest) @@ -465,8 +468,12 @@ } string.to_css(dest)?; - dest.write_str(" ")?; - value.to_css(dest)?; + + // If the track size is the initial value then it's redundant here. + if !value.is_initial() { + dest.write_str(" ")?; + value.to_css(dest)?; + } } if let Some(names) = names_iter.next() { @@ -513,8 +520,8 @@ context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { - let mut temp_rows = GridTemplateComponent::None; - let mut temp_cols = GridTemplateComponent::None; + let mut temp_rows = GridTemplateComponent::default(); + let mut temp_cols = GridTemplateComponent::default(); let mut temp_areas = GridTemplateAreas::None; let mut auto_rows = ImplicitGridTracks::default(); let mut auto_cols = ImplicitGridTracks::default(); @@ -587,8 +594,8 @@ impl<'a> ToCss for LonghandsToSerialize<'a> { fn to_css(&self, dest: &mut CssWriter) -> fmt::Result where W: fmt::Write { if *self.grid_template_areas != GridTemplateAreas::None || - (*self.grid_template_rows != GridTemplateComponent::None && - *self.grid_template_columns != GridTemplateComponent::None) || + (!self.grid_template_rows.is_initial() && + !self.grid_template_columns.is_initial()) || self.is_grid_template() { return super::grid_template::serialize_grid_template(self.grid_template_rows, self.grid_template_columns, @@ -598,7 +605,7 @@ if self.grid_auto_flow.autoflow == AutoFlow::Column { // It should fail to serialize if other branch of the if condition's values are set. if !self.grid_auto_rows.is_initial() || - *self.grid_template_columns != GridTemplateComponent::None { + !self.grid_template_columns.is_initial() { return Ok(()); } @@ -622,7 +629,7 @@ } else { // It should fail to serialize if other branch of the if condition's values are set. if !self.grid_auto_columns.is_initial() || - *self.grid_template_rows != GridTemplateComponent::None { + !self.grid_template_rows.is_initial() { return Ok(()); } diff --git a/components/style/values/generics/grid.rs b/components/style/values/generics/grid.rs index 807136d98c4..6a2ca05b4b3 100644 --- a/components/style/values/generics/grid.rs +++ b/components/style/values/generics/grid.rs @@ -261,6 +261,19 @@ pub enum GenericTrackSize { pub use self::GenericTrackSize as TrackSize; impl TrackSize { + /// The initial value. + const INITIAL_VALUE: Self = TrackSize::Breadth(TrackBreadth::Auto); + + /// Returns the initial value. + pub const fn initial_value() -> Self { + Self::INITIAL_VALUE + } + + /// Returns true if `self` is the initial value. + pub fn is_initial(&self) -> bool { + matches!(*self, TrackSize::Breadth(TrackBreadth::Auto)) // FIXME: can't use Self::INITIAL_VALUE here yet: https://github.com/rust-lang/rust/issues/66585 + } + /// Check whether this is a `` /// /// @@ -286,17 +299,9 @@ impl TrackSize { } } -impl TrackSize { - /// Return true if it is `auto`. - #[inline] - pub fn is_auto(&self) -> bool { - *self == TrackSize::Breadth(TrackBreadth::Auto) - } -} - impl Default for TrackSize { fn default() -> Self { - TrackSize::Breadth(TrackBreadth::Auto) + Self::initial_value() } } @@ -513,9 +518,24 @@ pub enum GenericTrackListValue { pub use self::GenericTrackListValue as TrackListValue; impl TrackListValue { + // FIXME: can't use TrackSize::initial_value() here b/c rustc error "is not yet stable as a const fn" + const INITIAL_VALUE: Self = TrackListValue::TrackSize(TrackSize::Breadth(TrackBreadth::Auto)); + fn is_repeat(&self) -> bool { matches!(*self, TrackListValue::TrackRepeat(..)) } + + /// Returns true if `self` is the initial value. + pub fn is_initial(&self) -> bool { + matches!(*self, TrackListValue::TrackSize(TrackSize::Breadth(TrackBreadth::Auto))) // FIXME: can't use Self::INITIAL_VALUE here yet: https://github.com/rust-lang/rust/issues/66585 + } +} + +impl Default for TrackListValue { + #[inline] + fn default() -> Self { + Self::INITIAL_VALUE + } } /// A grid `` type. @@ -755,6 +775,9 @@ pub enum GenericGridTemplateComponent { pub use self::GenericGridTemplateComponent as GridTemplateComponent; impl GridTemplateComponent { + /// The initial value. + const INITIAL_VALUE: Self = Self::None; + /// Returns length of the s pub fn track_list_len(&self) -> usize { match *self { @@ -762,4 +785,16 @@ impl GridTemplateComponent { _ => 0, } } + + /// Returns true if `self` is the initial value. + pub fn is_initial(&self) -> bool { + matches!(*self, Self::None) // FIXME: can't use Self::INITIAL_VALUE here yet: https://github.com/rust-lang/rust/issues/66585 + } +} + +impl Default for GridTemplateComponent { + #[inline] + fn default() -> Self { + Self::INITIAL_VALUE + } } diff --git a/components/style/values/specified/grid.rs b/components/style/values/specified/grid.rs index b75e97aca99..4830aea24d6 100644 --- a/components/style/values/specified/grid.rs +++ b/components/style/values/specified/grid.rs @@ -102,8 +102,8 @@ impl Parse for ImplicitGridTracks> { ) -> Result> { use style_traits::{Separator, Space}; let track_sizes = Space::parse(input, |i| TrackSize::parse(context, i))?; - if track_sizes.len() == 1 && track_sizes[0].is_auto() { - //`auto`, which is the initial value, is always represented by an empty slice. + if track_sizes.len() == 1 && track_sizes[0].is_initial() { + // A single track with the initial value is always represented by an empty slice. return Ok(Default::default()); } return Ok(ImplicitGridTracks(track_sizes.into())); From 4359b3bd472ff5785f4aeaabff2b06695c79a772 Mon Sep 17 00:00:00 2001 From: Mats Palmgren Date: Sat, 23 Nov 2019 21:24:33 +0000 Subject: [PATCH 31/45] style: Make 'grid-area'/'grid-column'/'grid-row' shorthands serialize to shortest possible form. Differential Revision: https://phabricator.services.mozilla.com/D54386 --- .../properties/shorthands/position.mako.rs | 39 ++++++++++++++++++- components/style/values/generics/grid.rs | 20 +++++++++- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/components/style/properties/shorthands/position.mako.rs b/components/style/properties/shorthands/position.mako.rs index 5c884243c98..d9cb9da3342 100644 --- a/components/style/properties/shorthands/position.mako.rs +++ b/components/style/properties/shorthands/position.mako.rs @@ -181,8 +181,18 @@ } impl<'a> ToCss for LonghandsToSerialize<'a> { + // Return the shortest possible serialization of the `grid-${kind}-[start/end]` values. + // This function exploits the opportunities to omit the end value per this spec text: + // + // https://drafts.csswg.org/css-grid/#propdef-grid-column + // "When the second value is omitted, if the first value is a , + // the grid-row-end/grid-column-end longhand is also set to that ; + // otherwise, it is set to auto." fn to_css(&self, dest: &mut CssWriter) -> fmt::Result where W: fmt::Write { self.grid_${kind}_start.to_css(dest)?; + if self.grid_${kind}_start.can_omit(self.grid_${kind}_end) { + return Ok(()); // the end value is redundant + } dest.write_str(" / ")?; self.grid_${kind}_end.to_css(dest) } @@ -247,14 +257,39 @@ } impl<'a> ToCss for LonghandsToSerialize<'a> { + // Return the shortest possible serialization of the `grid-[column/row]-[start/end]` values. + // This function exploits the opportunities to omit trailing values per this spec text: + // + // https://drafts.csswg.org/css-grid/#propdef-grid-area + // "If four values are specified, grid-row-start is set to the first value, + // grid-column-start is set to the second value, grid-row-end is set to the third value, + // and grid-column-end is set to the fourth value. + // + // When grid-column-end is omitted, if grid-column-start is a , + // grid-column-end is set to that ; otherwise, it is set to auto. + // + // When grid-row-end is omitted, if grid-row-start is a , grid-row-end is + // set to that ; otherwise, it is set to auto. + // + // When grid-column-start is omitted, if grid-row-start is a , all four + // longhands are set to that value. Otherwise, it is set to auto." fn to_css(&self, dest: &mut CssWriter) -> fmt::Result where W: fmt::Write { self.grid_row_start.to_css(dest)?; + let mut trailing_values = 3; + if self.grid_column_start.can_omit(self.grid_column_end) { + trailing_values -= 1; + if self.grid_row_start.can_omit(self.grid_row_end) { + trailing_values -= 1; + if self.grid_row_start.can_omit(self.grid_column_start) { + trailing_values -= 1; + } + } + } let values = [&self.grid_column_start, &self.grid_row_end, &self.grid_column_end]; - for value in &values { + for value in values.iter().take(trailing_values) { dest.write_str(" / ")?; value.to_css(dest)?; } - Ok(()) } } diff --git a/components/style/values/generics/grid.rs b/components/style/values/generics/grid.rs index 6a2ca05b4b3..70b0a491b8e 100644 --- a/components/style/values/generics/grid.rs +++ b/components/style/values/generics/grid.rs @@ -58,7 +58,7 @@ pub use self::GenericGridLine as GridLine; impl GridLine where - Integer: Zero, + Integer: PartialEq + Zero, { /// The `auto` value. pub fn auto() -> Self { @@ -73,11 +73,27 @@ where pub fn is_auto(&self) -> bool { self.ident == atom!("") && self.line_num.is_zero() && !self.is_span } + + /// Check whether this `` represents a `` value. + pub fn is_ident_only(&self) -> bool { + self.ident != atom!("") && self.line_num.is_zero() && !self.is_span + } + + /// Check if `self` makes `other` omittable according to the rules at: + /// https://drafts.csswg.org/css-grid/#propdef-grid-column + /// https://drafts.csswg.org/css-grid/#propdef-grid-area + pub fn can_omit(&self, other: &Self) -> bool { + if self.is_ident_only() { + self == other + } else { + other.is_auto() + } + } } impl ToCss for GridLine where - Integer: ToCss + Zero, + Integer: ToCss + PartialEq + Zero, { fn to_css(&self, dest: &mut CssWriter) -> fmt::Result where From aa1ad6036d7b0ead1c1aa2588e83a8ac62f8fc91 Mon Sep 17 00:00:00 2001 From: jeffin143 Date: Mon, 25 Nov 2019 15:41:18 +0000 Subject: [PATCH 32/45] style: convert NS_STYLE_IME_MODE_* to an enum class in nsStyleConsts.h Differential Revision: https://phabricator.services.mozilla.com/D54255 --- components/style/properties/longhands/ui.mako.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/components/style/properties/longhands/ui.mako.rs b/components/style/properties/longhands/ui.mako.rs index 64702855967..9bb38a150c1 100644 --- a/components/style/properties/longhands/ui.mako.rs +++ b/components/style/properties/longhands/ui.mako.rs @@ -15,6 +15,7 @@ ${helpers.single_keyword( "ime-mode", "auto normal active disabled inactive", engines="gecko", + gecko_enum_prefix="StyleImeMode", gecko_ffi_name="mIMEMode", animation_value_type="discrete", spec="https://drafts.csswg.org/css-ui/#input-method-editor", From f777c17f3fd4f04afd7b2b344ccf3918da05c7db Mon Sep 17 00:00:00 2001 From: jeffin143 Date: Mon, 25 Nov 2019 15:40:50 +0000 Subject: [PATCH 33/45] style: convert NS_STYLE_OBJECT_FIT_* to an enum class in nsStyleConsts.h Differential Revision: https://phabricator.services.mozilla.com/D54153 --- components/style/properties/longhands/position.mako.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/components/style/properties/longhands/position.mako.rs b/components/style/properties/longhands/position.mako.rs index 1c2f5cc2e4a..95595f36316 100644 --- a/components/style/properties/longhands/position.mako.rs +++ b/components/style/properties/longhands/position.mako.rs @@ -331,6 +331,7 @@ ${helpers.single_keyword( engines="gecko", animation_value_type="discrete", spec="https://drafts.csswg.org/css-images/#propdef-object-fit", + gecko_enum_prefix = "StyleObjectFit", )} ${helpers.predefined_type( From 1ef7c2f96b3eb169290c569903e1b7b64405fb56 Mon Sep 17 00:00:00 2001 From: jeffin143 Date: Mon, 25 Nov 2019 16:00:23 +0000 Subject: [PATCH 34/45] style: convert NS_STYLE_WINDOW_SHADOW_* to an enum class in nsStyleConsts.h Differential Revision: https://phabricator.services.mozilla.com/D53920 --- components/style/properties/longhands/ui.mako.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/style/properties/longhands/ui.mako.rs b/components/style/properties/longhands/ui.mako.rs index 9bb38a150c1..05d22abd9e9 100644 --- a/components/style/properties/longhands/ui.mako.rs +++ b/components/style/properties/longhands/ui.mako.rs @@ -59,7 +59,7 @@ ${helpers.single_keyword( "none default menu tooltip sheet", engines="gecko", gecko_ffi_name="mWindowShadow", - gecko_constant_prefix="NS_STYLE_WINDOW_SHADOW", + gecko_enum_prefix="StyleWindowShadow", animation_value_type="discrete", enabled_in="chrome", spec="None (Nonstandard internal property)", From d9aa0571e3c3df9c003bde06a3f45c2b361bf128 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 25 Nov 2019 19:14:37 +0000 Subject: [PATCH 35/45] style: Simplify some code now that lifetimes are non-lexical. Differential Revision: https://phabricator.services.mozilla.com/D54529 --- components/style/custom_properties.rs | 35 +++-- components/style/stylesheets/supports_rule.rs | 8 +- components/style/values/specified/angle.rs | 24 +++- components/style/values/specified/calc.rs | 36 ++--- components/style/values/specified/counters.rs | 56 ++++---- components/style/values/specified/font.rs | 104 +++++++------- components/style/values/specified/image.rs | 39 +++--- components/style/values/specified/length.rs | 131 +++++++++--------- components/style/values/specified/mod.rs | 37 +++-- .../style/values/specified/percentage.rs | 25 ++-- components/style/values/specified/time.rs | 25 ++-- 11 files changed, 262 insertions(+), 258 deletions(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index 639d32a97a9..3717e1f0614 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -306,8 +306,7 @@ fn parse_declaration_value_block<'i, 't>( ) -> Result<(TokenSerializationType, TokenSerializationType), ParseError<'i>> { let mut token_start = input.position(); let mut token = match input.next_including_whitespace_and_comments() { - // FIXME: remove clone() when borrows are non-lexical - Ok(token) => token.clone(), + Ok(token) => token, Err(_) => { return Ok(( TokenSerializationType::nothing(), @@ -335,8 +334,9 @@ fn parse_declaration_value_block<'i, 't>( } }; } - let last_token_type = match token { + let last_token_type = match *token { Token::Comment(_) => { + let serialization_type = token.serialization_type(); let token_slice = input.slice_from(token_start); if !token_slice.ends_with("*/") { missing_closing_characters.push_str(if token_slice.ends_with('*') { @@ -345,14 +345,14 @@ fn parse_declaration_value_block<'i, 't>( "*/" }) } - token.serialization_type() + serialization_type }, - Token::BadUrl(u) => { - let e = StyleParseErrorKind::BadUrlInDeclarationValueBlock(u); + Token::BadUrl(ref u) => { + let e = StyleParseErrorKind::BadUrlInDeclarationValueBlock(u.clone()); return Err(input.new_custom_error(e)); }, - Token::BadString(s) => { - let e = StyleParseErrorKind::BadStringInDeclarationValueBlock(s); + Token::BadString(ref s) => { + let e = StyleParseErrorKind::BadStringInDeclarationValueBlock(s.clone()); return Err(input.new_custom_error(e)); }, Token::CloseParenthesis => { @@ -401,13 +401,14 @@ fn parse_declaration_value_block<'i, 't>( Token::CloseSquareBracket.serialization_type() }, Token::QuotedString(_) => { + let serialization_type = token.serialization_type(); let token_slice = input.slice_from(token_start); let quote = &token_slice[..1]; debug_assert!(matches!(quote, "\"" | "'")); if !(token_slice.ends_with(quote) && token_slice.len() > 1) { missing_closing_characters.push_str(quote) } - token.serialization_type() + serialization_type }, Token::Ident(ref value) | Token::AtKeyword(ref value) | @@ -417,6 +418,8 @@ fn parse_declaration_value_block<'i, 't>( Token::Dimension { unit: ref value, .. } => { + let serialization_type = token.serialization_type(); + let is_unquoted_url = matches!(token, Token::UnquotedUrl(_)); if value.ends_with("�") && input.slice_from(token_start).ends_with("\\") { // Unescaped backslash at EOF in these contexts is interpreted as U+FFFD // Check the value in case the final backslash was itself escaped. @@ -424,18 +427,17 @@ fn parse_declaration_value_block<'i, 't>( // (Unescaped U+FFFD would also work, but removing the backslash is annoying.) missing_closing_characters.push_str("�") } - if matches!(token, Token::UnquotedUrl(_)) { + if is_unquoted_url { check_closed!(")"); } - token.serialization_type() + serialization_type }, _ => token.serialization_type(), }; token_start = input.position(); token = match input.next_including_whitespace_and_comments() { - // FIXME: remove clone() when borrows are non-lexical - Ok(token) => token.clone(), + Ok(token) => token, Err(..) => return Ok((first_token_type, last_token_type)), }; } @@ -888,15 +890,12 @@ fn substitute_block<'i>( let mut set_position_at_next_iteration = false; loop { let before_this_token = input.position(); - // FIXME: remove clone() when borrows are non-lexical - let next = input - .next_including_whitespace_and_comments() - .map(|t| t.clone()); + let next = input.next_including_whitespace_and_comments(); if set_position_at_next_iteration { *position = ( before_this_token, match next { - Ok(ref token) => token.serialization_type(), + Ok(token) => token.serialization_type(), Err(_) => TokenSerializationType::nothing(), }, ); diff --git a/components/style/stylesheets/supports_rule.rs b/components/style/stylesheets/supports_rule.rs index 721634dd9a3..6a522bcdb74 100644 --- a/components/style/stylesheets/supports_rule.rs +++ b/components/style/stylesheets/supports_rule.rs @@ -178,8 +178,7 @@ impl SupportsCondition { while input.try(Parser::expect_whitespace).is_ok() {} let pos = input.position(); let location = input.current_source_location(); - // FIXME: remove clone() when lifetimes are non-lexical - match input.next()?.clone() { + match *input.next()? { Token::ParenthesisBlock => { let nested = input.try(|input| input.parse_nested_block(parse_condition_or_declaration)); @@ -187,7 +186,8 @@ impl SupportsCondition { return nested; } }, - Token::Function(ident) => { + Token::Function(ref ident) => { + let ident = ident.clone(); let nested = input.try(|input| { input.parse_nested_block(|input| { SupportsCondition::parse_functional(&ident, input) @@ -197,7 +197,7 @@ impl SupportsCondition { return nested; } }, - t => return Err(location.new_unexpected_token_error(t)), + ref t => return Err(location.new_unexpected_token_error(t.clone())), } input.parse_nested_block(consume_any_value)?; Ok(SupportsCondition::FutureSyntax( diff --git a/components/style/values/specified/angle.rs b/components/style/values/specified/angle.rs index 47c1a60772b..b616ed9c600 100644 --- a/components/style/values/specified/angle.rs +++ b/components/style/values/specified/angle.rs @@ -208,24 +208,34 @@ impl Angle { input: &mut Parser<'i, 't>, allow_unitless_zero: AllowUnitlessZeroAngle, ) -> Result> { - // FIXME: remove clone() when lifetimes are non-lexical - let token = input.next()?.clone(); - match token { + let t = input.next()?; + match *t { Token::Dimension { value, ref unit, .. } => { - Angle::parse_dimension(value, unit, /* from_calc = */ false) + match Angle::parse_dimension(value, unit, /* from_calc = */ false) { + Ok(angle) => Ok(angle), + Err(()) => { + let t = t.clone(); + Err(input.new_unexpected_token_error(t)) + } + } }, Token::Number { value, .. } if value == 0. => match allow_unitless_zero { AllowUnitlessZeroAngle::Yes => Ok(Angle::zero()), - AllowUnitlessZeroAngle::No => Err(()), + AllowUnitlessZeroAngle::No => { + let t = t.clone(); + Err(input.new_unexpected_token_error(t)) + }, }, Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => { return input.parse_nested_block(|i| CalcNode::parse_angle(context, i)); }, - _ => Err(()), + ref t => { + let t = t.clone(); + Err(input.new_unexpected_token_error(t)) + } } - .map_err(|()| input.new_unexpected_token_error(token.clone())) } } diff --git a/components/style/values/specified/calc.rs b/components/style/values/specified/calc.rs index b101cd69de7..87f88039506 100644 --- a/components/style/values/specified/calc.rs +++ b/components/style/values/specified/calc.rs @@ -163,9 +163,8 @@ impl CalcNode { expected_unit: CalcUnit, ) -> Result> { let location = input.current_source_location(); - // FIXME: remove early returns when lifetimes are non-lexical match (input.next()?, expected_unit) { - (&Token::Number { value, .. }, _) => return Ok(CalcNode::Number(value)), + (&Token::Number { value, .. }, _) => Ok(CalcNode::Number(value)), ( &Token::Dimension { value, ref unit, .. @@ -178,11 +177,11 @@ impl CalcNode { }, CalcUnit::LengthPercentage, ) => { - return NoCalcLength::parse_dimension(context, value, unit) + NoCalcLength::parse_dimension(context, value, unit) .map(CalcNode::Length) .map_err(|()| { location.new_custom_error(StyleParseErrorKind::UnspecifiedError) - }); + }) }, ( &Token::Dimension { @@ -190,11 +189,11 @@ impl CalcNode { }, CalcUnit::Angle, ) => { - return Angle::parse_dimension(value, unit, /* from_calc = */ true) + Angle::parse_dimension(value, unit, /* from_calc = */ true) .map(CalcNode::Angle) .map_err(|()| { location.new_custom_error(StyleParseErrorKind::UnspecifiedError) - }); + }) }, ( &Token::Dimension { @@ -202,21 +201,24 @@ impl CalcNode { }, CalcUnit::Time, ) => { - return Time::parse_dimension(value, unit, /* from_calc = */ true) + Time::parse_dimension(value, unit, /* from_calc = */ true) .map(CalcNode::Time) .map_err(|()| { location.new_custom_error(StyleParseErrorKind::UnspecifiedError) - }); + }) }, (&Token::Percentage { unit_value, .. }, CalcUnit::LengthPercentage) | (&Token::Percentage { unit_value, .. }, CalcUnit::Percentage) => { - return Ok(CalcNode::Percentage(unit_value)); + Ok(CalcNode::Percentage(unit_value)) }, - (&Token::ParenthesisBlock, _) => {}, - (&Token::Function(ref name), _) if name.eq_ignore_ascii_case("calc") => {}, - (t, _) => return Err(location.new_unexpected_token_error(t.clone())), + (&Token::ParenthesisBlock, _) => { + input.parse_nested_block(|i| CalcNode::parse(context, i, expected_unit)) + }, + (&Token::Function(ref name), _) if name.eq_ignore_ascii_case("calc") => { + input.parse_nested_block(|i| CalcNode::parse(context, i, expected_unit)) + }, + (t, _) => Err(location.new_unexpected_token_error(t.clone())), } - input.parse_nested_block(|i| CalcNode::parse(context, i, expected_unit)) } /// Parse a top-level `calc` expression, with all nested sub-expressions. @@ -236,8 +238,7 @@ impl CalcNode { if input.is_exhausted() { break; // allow trailing whitespace } - // FIXME: remove clone() when lifetimes are non-lexical - match input.next()?.clone() { + match *input.next()? { Token::Delim('+') => { let rhs = Self::parse_product(context, input, expected_unit)?; let new_root = CalcNode::Sum(Box::new(root), Box::new(rhs)); @@ -248,7 +249,10 @@ impl CalcNode { let new_root = CalcNode::Sub(Box::new(root), Box::new(rhs)); root = new_root; }, - t => return Err(input.new_unexpected_token_error(t)), + ref t => { + let t = t.clone(); + return Err(input.new_unexpected_token_error(t)); + } } }, _ => { diff --git a/components/style/values/specified/counters.rs b/components/style/values/specified/counters.rs index 54690b53105..69a05da792b 100644 --- a/components/style/values/specified/counters.rs +++ b/components/style/values/specified/counters.rs @@ -63,7 +63,10 @@ fn parse_counters<'i, 't>( let location = input.current_source_location(); let name = match input.next() { Ok(&Token::Ident(ref ident)) => CustomIdent::from_ident(location, ident, &["none"])?, - Ok(t) => return Err(location.new_unexpected_token_error(t.clone())), + Ok(t) => { + let t = t.clone(); + return Err(location.new_unexpected_token_error(t)); + }, Err(_) => break, }; @@ -147,57 +150,60 @@ impl Parse for Content { continue; } } - // FIXME: remove clone() when lifetimes are non-lexical - match input.next().map(|t| t.clone()) { - Ok(Token::QuotedString(ref value)) => { + match input.next() { + Ok(&Token::QuotedString(ref value)) => { content.push(generics::ContentItem::String( value.as_ref().to_owned().into_boxed_str(), )); }, - Ok(Token::Function(ref name)) => { + Ok(&Token::Function(ref name)) => { let result = match_ignore_ascii_case! { &name, - "counter" => Some(input.parse_nested_block(|input| { + "counter" => input.parse_nested_block(|input| { let location = input.current_source_location(); let name = CustomIdent::from_ident(location, input.expect_ident()?, &[])?; let style = Content::parse_counter_style(context, input); Ok(generics::ContentItem::Counter(name, style)) - })), - "counters" => Some(input.parse_nested_block(|input| { + }), + "counters" => input.parse_nested_block(|input| { let location = input.current_source_location(); let name = CustomIdent::from_ident(location, input.expect_ident()?, &[])?; input.expect_comma()?; let separator = input.expect_string()?.as_ref().to_owned().into_boxed_str(); let style = Content::parse_counter_style(context, input); Ok(generics::ContentItem::Counters(name, separator, style)) - })), + }), #[cfg(feature = "gecko")] - "attr" => Some(input.parse_nested_block(|input| { + "attr" => input.parse_nested_block(|input| { Ok(generics::ContentItem::Attr(Attr::parse_function(context, input)?)) - })), - _ => None - }; - match result { - Some(result) => content.push(result?), - None => { + }), + _ => { + let name = name.clone(); return Err(input.new_custom_error( - StyleParseErrorKind::UnexpectedFunction(name.clone()), - )); - }, - } + StyleParseErrorKind::UnexpectedFunction(name), + )) + } + }?; + content.push(result); }, - Ok(Token::Ident(ref ident)) => { + Ok(&Token::Ident(ref ident)) => { content.push(match_ignore_ascii_case! { &ident, "open-quote" => generics::ContentItem::OpenQuote, "close-quote" => generics::ContentItem::CloseQuote, "no-open-quote" => generics::ContentItem::NoOpenQuote, "no-close-quote" => generics::ContentItem::NoCloseQuote, - _ => return Err(input.new_custom_error( - SelectorParseErrorKind::UnexpectedIdent(ident.clone()) - )) + _ =>{ + let ident = ident.clone(); + return Err(input.new_custom_error( + SelectorParseErrorKind::UnexpectedIdent(ident) + )); + } }); }, Err(_) => break, - Ok(t) => return Err(input.new_unexpected_token_error(t)), + Ok(t) => { + let t = t.clone(); + return Err(input.new_unexpected_token_error(t)); + } } } if content.is_empty() { diff --git a/components/style/values/specified/font.rs b/components/style/values/specified/font.rs index 88f5969f52c..a6a67bc37de 100644 --- a/components/style/values/specified/font.rs +++ b/components/style/values/specified/font.rs @@ -1213,64 +1213,66 @@ impl Parse for FontVariantAlternates { ) ); while let Ok(_) = input.try(|input| { - // FIXME: remove clone() when lifetimes are non-lexical - match input.next()?.clone() { + match *input.next()? { Token::Ident(ref value) if value.eq_ignore_ascii_case("historical-forms") => { check_if_parsed!(input, VariantAlternatesParsingFlags::HISTORICAL_FORMS); alternates.push(VariantAlternates::HistoricalForms); Ok(()) }, - Token::Function(ref name) => input.parse_nested_block(|i| { - match_ignore_ascii_case! { &name, - "swash" => { - check_if_parsed!(i, VariantAlternatesParsingFlags::SWASH); - let location = i.current_source_location(); - let ident = CustomIdent::from_ident(location, i.expect_ident()?, &[])?; - alternates.push(VariantAlternates::Swash(ident)); - Ok(()) - }, - "stylistic" => { - check_if_parsed!(i, VariantAlternatesParsingFlags::STYLISTIC); - let location = i.current_source_location(); - let ident = CustomIdent::from_ident(location, i.expect_ident()?, &[])?; - alternates.push(VariantAlternates::Stylistic(ident)); - Ok(()) - }, - "ornaments" => { - check_if_parsed!(i, VariantAlternatesParsingFlags::ORNAMENTS); - let location = i.current_source_location(); - let ident = CustomIdent::from_ident(location, i.expect_ident()?, &[])?; - alternates.push(VariantAlternates::Ornaments(ident)); - Ok(()) - }, - "annotation" => { - check_if_parsed!(i, VariantAlternatesParsingFlags::ANNOTATION); - let location = i.current_source_location(); - let ident = CustomIdent::from_ident(location, i.expect_ident()?, &[])?; - alternates.push(VariantAlternates::Annotation(ident)); - Ok(()) - }, - "styleset" => { - check_if_parsed!(i, VariantAlternatesParsingFlags::STYLESET); - let idents = i.parse_comma_separated(|i| { + Token::Function(ref name) => { + let name = name.clone(); + input.parse_nested_block(|i| { + match_ignore_ascii_case! { &name, + "swash" => { + check_if_parsed!(i, VariantAlternatesParsingFlags::SWASH); let location = i.current_source_location(); - CustomIdent::from_ident(location, i.expect_ident()?, &[]) - })?; - alternates.push(VariantAlternates::Styleset(idents.into())); - Ok(()) - }, - "character-variant" => { - check_if_parsed!(i, VariantAlternatesParsingFlags::CHARACTER_VARIANT); - let idents = i.parse_comma_separated(|i| { + let ident = CustomIdent::from_ident(location, i.expect_ident()?, &[])?; + alternates.push(VariantAlternates::Swash(ident)); + Ok(()) + }, + "stylistic" => { + check_if_parsed!(i, VariantAlternatesParsingFlags::STYLISTIC); let location = i.current_source_location(); - CustomIdent::from_ident(location, i.expect_ident()?, &[]) - })?; - alternates.push(VariantAlternates::CharacterVariant(idents.into())); - Ok(()) - }, - _ => return Err(i.new_custom_error(StyleParseErrorKind::UnspecifiedError)), - } - }), + let ident = CustomIdent::from_ident(location, i.expect_ident()?, &[])?; + alternates.push(VariantAlternates::Stylistic(ident)); + Ok(()) + }, + "ornaments" => { + check_if_parsed!(i, VariantAlternatesParsingFlags::ORNAMENTS); + let location = i.current_source_location(); + let ident = CustomIdent::from_ident(location, i.expect_ident()?, &[])?; + alternates.push(VariantAlternates::Ornaments(ident)); + Ok(()) + }, + "annotation" => { + check_if_parsed!(i, VariantAlternatesParsingFlags::ANNOTATION); + let location = i.current_source_location(); + let ident = CustomIdent::from_ident(location, i.expect_ident()?, &[])?; + alternates.push(VariantAlternates::Annotation(ident)); + Ok(()) + }, + "styleset" => { + check_if_parsed!(i, VariantAlternatesParsingFlags::STYLESET); + let idents = i.parse_comma_separated(|i| { + let location = i.current_source_location(); + CustomIdent::from_ident(location, i.expect_ident()?, &[]) + })?; + alternates.push(VariantAlternates::Styleset(idents.into())); + Ok(()) + }, + "character-variant" => { + check_if_parsed!(i, VariantAlternatesParsingFlags::CHARACTER_VARIANT); + let idents = i.parse_comma_separated(|i| { + let location = i.current_source_location(); + CustomIdent::from_ident(location, i.expect_ident()?, &[]) + })?; + alternates.push(VariantAlternates::CharacterVariant(idents.into())); + Ok(()) + }, + _ => return Err(i.new_custom_error(StyleParseErrorKind::UnspecifiedError)), + } + }) + }, _ => Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)), } }) {} diff --git a/components/style/values/specified/image.rs b/components/style/values/specified/image.rs index 7488e631e54..126a4cc69d1 100644 --- a/components/style/values/specified/image.rs +++ b/components/style/values/specified/image.rs @@ -192,62 +192,57 @@ impl Parse for Gradient { Radial, } - // FIXME: remove clone() when lifetimes are non-lexical - let func = input.expect_function()?.clone(); - let result = match_ignore_ascii_case! { &func, + let func = input.expect_function()?; + let (shape, repeating, mut compat_mode) = match_ignore_ascii_case! { &func, "linear-gradient" => { - Some((Shape::Linear, false, GradientCompatMode::Modern)) + (Shape::Linear, false, GradientCompatMode::Modern) }, "-webkit-linear-gradient" => { - Some((Shape::Linear, false, GradientCompatMode::WebKit)) + (Shape::Linear, false, GradientCompatMode::WebKit) }, #[cfg(feature = "gecko")] "-moz-linear-gradient" => { - Some((Shape::Linear, false, GradientCompatMode::Moz)) + (Shape::Linear, false, GradientCompatMode::Moz) }, "repeating-linear-gradient" => { - Some((Shape::Linear, true, GradientCompatMode::Modern)) + (Shape::Linear, true, GradientCompatMode::Modern) }, "-webkit-repeating-linear-gradient" => { - Some((Shape::Linear, true, GradientCompatMode::WebKit)) + (Shape::Linear, true, GradientCompatMode::WebKit) }, #[cfg(feature = "gecko")] "-moz-repeating-linear-gradient" => { - Some((Shape::Linear, true, GradientCompatMode::Moz)) + (Shape::Linear, true, GradientCompatMode::Moz) }, "radial-gradient" => { - Some((Shape::Radial, false, GradientCompatMode::Modern)) + (Shape::Radial, false, GradientCompatMode::Modern) }, "-webkit-radial-gradient" => { - Some((Shape::Radial, false, GradientCompatMode::WebKit)) + (Shape::Radial, false, GradientCompatMode::WebKit) }, #[cfg(feature = "gecko")] "-moz-radial-gradient" => { - Some((Shape::Radial, false, GradientCompatMode::Moz)) + (Shape::Radial, false, GradientCompatMode::Moz) }, "repeating-radial-gradient" => { - Some((Shape::Radial, true, GradientCompatMode::Modern)) + (Shape::Radial, true, GradientCompatMode::Modern) }, "-webkit-repeating-radial-gradient" => { - Some((Shape::Radial, true, GradientCompatMode::WebKit)) + (Shape::Radial, true, GradientCompatMode::WebKit) }, #[cfg(feature = "gecko")] "-moz-repeating-radial-gradient" => { - Some((Shape::Radial, true, GradientCompatMode::Moz)) + (Shape::Radial, true, GradientCompatMode::Moz) }, "-webkit-gradient" => { return input.parse_nested_block(|i| { Self::parse_webkit_gradient_argument(context, i) }); }, - _ => None, - }; - - let (shape, repeating, mut compat_mode) = match result { - Some(result) => result, - None => { + _ => { + let func = func.clone(); return Err(input.new_custom_error(StyleParseErrorKind::UnexpectedFunction(func))); - }, + } }; let (kind, items) = input.parse_nested_block(|i| { diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index c98038bdc68..b553c7bd33e 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -587,39 +587,37 @@ impl Length { num_context: AllowedNumericType, allow_quirks: AllowQuirks, ) -> Result> { - // FIXME: remove early returns when lifetimes are non-lexical - { - let location = input.current_source_location(); - let token = input.next()?; - match *token { - Token::Dimension { - value, ref unit, .. - } if num_context.is_ok(context.parsing_mode, value) => { - return NoCalcLength::parse_dimension(context, value, unit) - .map(Length::NoCalc) - .map_err(|()| location.new_unexpected_token_error(token.clone())); - }, - Token::Number { value, .. } if num_context.is_ok(context.parsing_mode, value) => { - if value != 0. && - !context.parsing_mode.allows_unitless_lengths() && - !allow_quirks.allowed(context.quirks_mode) - { - return Err( - location.new_custom_error(StyleParseErrorKind::UnspecifiedError) - ); - } - return Ok(Length::NoCalc(NoCalcLength::Absolute(AbsoluteLength::Px( - value, - )))); - }, - Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {}, - ref token => return Err(location.new_unexpected_token_error(token.clone())), - } + let location = input.current_source_location(); + let token = input.next()?; + match *token { + Token::Dimension { + value, ref unit, .. + } if num_context.is_ok(context.parsing_mode, value) => { + NoCalcLength::parse_dimension(context, value, unit) + .map(Length::NoCalc) + .map_err(|()| location.new_unexpected_token_error(token.clone())) + }, + Token::Number { value, .. } if num_context.is_ok(context.parsing_mode, value) => { + if value != 0. && + !context.parsing_mode.allows_unitless_lengths() && + !allow_quirks.allowed(context.quirks_mode) + { + return Err( + location.new_custom_error(StyleParseErrorKind::UnspecifiedError) + ); + } + Ok(Length::NoCalc(NoCalcLength::Absolute(AbsoluteLength::Px( + value, + )))) + }, + Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => { + input.parse_nested_block(|input| { + CalcNode::parse_length(context, input, num_context) + .map(|calc| Length::Calc(Box::new(calc))) + }) + }, + ref token => return Err(location.new_unexpected_token_error(token.clone())), } - input.parse_nested_block(|input| { - CalcNode::parse_length(context, input, num_context) - .map(|calc| Length::Calc(Box::new(calc))) - }) } /// Parse a non-negative length @@ -809,44 +807,41 @@ impl LengthPercentage { num_context: AllowedNumericType, allow_quirks: AllowQuirks, ) -> Result> { - // FIXME: remove early returns when lifetimes are non-lexical - { - let location = input.current_source_location(); - let token = input.next()?; - match *token { - Token::Dimension { - value, ref unit, .. - } if num_context.is_ok(context.parsing_mode, value) => { - return NoCalcLength::parse_dimension(context, value, unit) - .map(LengthPercentage::Length) - .map_err(|()| location.new_unexpected_token_error(token.clone())); - }, - Token::Percentage { unit_value, .. } - if num_context.is_ok(context.parsing_mode, unit_value) => - { - return Ok(LengthPercentage::Percentage(computed::Percentage( - unit_value, - ))); - } - Token::Number { value, .. } if num_context.is_ok(context.parsing_mode, value) => { - if value != 0. && - !context.parsing_mode.allows_unitless_lengths() && - !allow_quirks.allowed(context.quirks_mode) - { - return Err(location.new_unexpected_token_error(token.clone())); - } else { - return Ok(LengthPercentage::Length(NoCalcLength::from_px(value))); - } - }, - Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {}, - _ => return Err(location.new_unexpected_token_error(token.clone())), + let location = input.current_source_location(); + let token = input.next()?; + match *token { + Token::Dimension { + value, ref unit, .. + } if num_context.is_ok(context.parsing_mode, value) => { + return NoCalcLength::parse_dimension(context, value, unit) + .map(LengthPercentage::Length) + .map_err(|()| location.new_unexpected_token_error(token.clone())); + }, + Token::Percentage { unit_value, .. } + if num_context.is_ok(context.parsing_mode, unit_value) => + { + return Ok(LengthPercentage::Percentage(computed::Percentage( + unit_value, + ))); } + Token::Number { value, .. } if num_context.is_ok(context.parsing_mode, value) => { + if value != 0. && + !context.parsing_mode.allows_unitless_lengths() && + !allow_quirks.allowed(context.quirks_mode) + { + return Err(location.new_unexpected_token_error(token.clone())); + } else { + return Ok(LengthPercentage::Length(NoCalcLength::from_px(value))); + } + }, + Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => { + let calc = input.parse_nested_block(|i| { + CalcNode::parse_length_or_percentage(context, i, num_context) + })?; + Ok(LengthPercentage::Calc(Box::new(calc))) + }, + _ => return Err(location.new_unexpected_token_error(token.clone())), } - - let calc = input.parse_nested_block(|i| { - CalcNode::parse_length_or_percentage(context, i, num_context) - })?; - Ok(LengthPercentage::Calc(Box::new(calc))) } /// Parses allowing the unitless length quirk. diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index d10a399dd01..bc7a8203dee 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -136,24 +136,22 @@ fn parse_number_with_clamping_mode<'i, 't>( clamping_mode: AllowedNumericType, ) -> Result> { let location = input.current_source_location(); - // FIXME: remove early returns when lifetimes are non-lexical match *input.next()? { Token::Number { value, .. } if clamping_mode.is_ok(context.parsing_mode, value) => { - return Ok(Number { + Ok(Number { value: value.min(f32::MAX).max(f32::MIN), calc_clamping_mode: None, - }); + }) }, - Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {}, - ref t => return Err(location.new_unexpected_token_error(t.clone())), + Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => { + let result = input.parse_nested_block(|i| CalcNode::parse_number(context, i))?; + Ok(Number { + value: result.min(f32::MAX).max(f32::MIN), + calc_clamping_mode: Some(clamping_mode), + }) + }, + ref t => Err(location.new_unexpected_token_error(t.clone())), } - - let result = input.parse_nested_block(|i| CalcNode::parse_number(context, i))?; - - Ok(Number { - value: result.min(f32::MAX).max(f32::MIN), - calc_clamping_mode: Some(clamping_mode), - }) } /// A CSS `` specified value. @@ -540,19 +538,16 @@ impl Parse for Integer { input: &mut Parser<'i, 't>, ) -> Result> { let location = input.current_source_location(); - - // FIXME: remove early returns when lifetimes are non-lexical match *input.next()? { Token::Number { int_value: Some(v), .. - } => return Ok(Integer::new(v)), - Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {}, - ref t => return Err(location.new_unexpected_token_error(t.clone())), + } => Ok(Integer::new(v)), + Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => { + let result = input.parse_nested_block(|i| CalcNode::parse_integer(context, i))?; + Ok(Integer::from_calc(result)) + }, + ref t => Err(location.new_unexpected_token_error(t.clone())), } - - let result = input.parse_nested_block(|i| CalcNode::parse_integer(context, i))?; - - Ok(Integer::from_calc(result)) } } diff --git a/components/style/values/specified/percentage.rs b/components/style/values/specified/percentage.rs index 64cb02bb424..ad596dd1404 100644 --- a/components/style/values/specified/percentage.rs +++ b/components/style/values/specified/percentage.rs @@ -111,25 +111,24 @@ impl Percentage { num_context: AllowedNumericType, ) -> Result> { let location = input.current_source_location(); - // FIXME: remove early returns when lifetimes are non-lexical match *input.next()? { Token::Percentage { unit_value, .. } if num_context.is_ok(context.parsing_mode, unit_value) => { - return Ok(Percentage::new(unit_value)); + Ok(Percentage::new(unit_value)) } - Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {}, - ref t => return Err(location.new_unexpected_token_error(t.clone())), + Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => { + let result = input.parse_nested_block(|i| CalcNode::parse_percentage(context, i))?; + + // TODO(emilio): -moz-image-rect is the only thing that uses + // the clamping mode... I guess we could disallow it... + Ok(Percentage { + value: result, + calc_clamping_mode: Some(num_context), + }) + }, + ref t => Err(location.new_unexpected_token_error(t.clone())), } - - let result = input.parse_nested_block(|i| CalcNode::parse_percentage(context, i))?; - - // TODO(emilio): -moz-image-rect is the only thing that uses - // the clamping mode... I guess we could disallow it... - Ok(Percentage { - value: result, - calc_clamping_mode: Some(num_context), - }) } /// Parses a percentage token, but rejects it if it's negative. diff --git a/components/style/values/specified/time.rs b/components/style/values/specified/time.rs index f46925bc5c1..09439c84d0c 100644 --- a/components/style/values/specified/time.rs +++ b/components/style/values/specified/time.rs @@ -83,27 +83,26 @@ impl Time { use style_traits::ParsingMode; let location = input.current_source_location(); - // FIXME: remove early returns when lifetimes are non-lexical - match input.next() { + match *input.next()? { // Note that we generally pass ParserContext to is_ok() to check // that the ParserMode of the ParserContext allows all numeric // values for SMIL regardless of clamping_mode, but in this Time // value case, the value does not animate for SMIL at all, so we use // ParsingMode::DEFAULT directly. - Ok(&Token::Dimension { + Token::Dimension { value, ref unit, .. - }) if clamping_mode.is_ok(ParsingMode::DEFAULT, value) => { - return Time::parse_dimension(value, unit, /* from_calc = */ false).map_err(|()| { + } if clamping_mode.is_ok(ParsingMode::DEFAULT, value) => { + Time::parse_dimension(value, unit, /* from_calc = */ false).map_err(|()| { location.new_custom_error(StyleParseErrorKind::UnspecifiedError) - }); + }) }, - Ok(&Token::Function(ref name)) if name.eq_ignore_ascii_case("calc") => {}, - Ok(t) => return Err(location.new_unexpected_token_error(t.clone())), - Err(e) => return Err(e.into()), - } - match input.parse_nested_block(|i| CalcNode::parse_time(context, i)) { - Ok(time) if clamping_mode.is_ok(ParsingMode::DEFAULT, time.seconds) => Ok(time), - _ => Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)), + Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => { + match input.parse_nested_block(|i| CalcNode::parse_time(context, i)) { + Ok(time) if clamping_mode.is_ok(ParsingMode::DEFAULT, time.seconds) => Ok(time), + _ => Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)), + } + }, + ref t => return Err(location.new_unexpected_token_error(t.clone())), } } From 22509ac3d03e287be18fc6244601f829e6197802 Mon Sep 17 00:00:00 2001 From: jeffin143 Date: Tue, 26 Nov 2019 04:50:04 +0000 Subject: [PATCH 36/45] style: convert NS_STYLE_TEXT_ORIENTATION_* to an enum class in nsStyleConsts.h Differential Revision: https://phabricator.services.mozilla.com/D54252 --- components/style/properties/longhands/inherited_box.mako.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/components/style/properties/longhands/inherited_box.mako.rs b/components/style/properties/longhands/inherited_box.mako.rs index 8db62bf56d6..2542ee5e376 100644 --- a/components/style/properties/longhands/inherited_box.mako.rs +++ b/components/style/properties/longhands/inherited_box.mako.rs @@ -51,6 +51,7 @@ ${helpers.single_keyword( "mixed upright sideways", engines="gecko", gecko_aliases="sideways-right=sideways", + gecko_enum_prefix="StyleTextOrientation", animation_value_type="none", spec="https://drafts.csswg.org/css-writing-modes/#propdef-text-orientation", )} From d06212c6c0058af222deef6e41dc50d3baf971d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 27 Nov 2019 20:51:20 +0000 Subject: [PATCH 37/45] style: Fix Servo_IsCssPropertyRecordedInUseCounter so that we also report disabled properties. When zoom is disabled, we still count it, but with the current code the testing function will throw instead of returning the right value, which means we'd fail layout/style/test/test_use_counters.html. Differential Revision: https://phabricator.services.mozilla.com/D55005 --- components/style/properties/properties.mako.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 484b52e8c87..0a26ae4555b 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -1813,8 +1813,8 @@ pub enum CountedUnknownProperty { } impl CountedUnknownProperty { - /// Parse the counted unknown property. - pub fn parse_for_test(property_name: &str) -> Option { + /// Parse the counted unknown property, for testing purposes only. + pub fn parse_for_testing(property_name: &str) -> Option { ascii_case_insensitive_phf_map! { unknown_id -> CountedUnknownProperty = { % for property in data.counted_unknown_properties: @@ -1826,6 +1826,7 @@ impl CountedUnknownProperty { } /// Returns the underlying index, used for use counter. + #[inline] pub fn bit(self) -> usize { self as usize } @@ -1842,9 +1843,16 @@ impl PropertyId { }) } - /// Returns a given property from the string `s`. + /// Returns a given property from the given name, _regardless of whether it + /// is enabled or not_, or Err(()) for unknown properties. /// - /// Returns Err(()) for unknown properties. + /// Do not use for non-testing purposes. + pub fn parse_unchecked_for_testing(name: &str) -> Result { + Self::parse_unchecked(name, None) + } + + /// Returns a given property from the given name, _regardless of whether it + /// is enabled or not_, or Err(()) for unknown properties. fn parse_unchecked( property_name: &str, use_counters: Option< &UseCounters>, From 001c511f9c08fe31be97d7e95288f3ca5bc28776 Mon Sep 17 00:00:00 2001 From: enordin Date: Fri, 29 Nov 2019 04:40:03 +0000 Subject: [PATCH 38/45] style: Have scale function and scale property accept percentage value. Differential Revision: https://phabricator.services.mozilla.com/D55012 --- .../style/values/specified/transform.rs | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/components/style/values/specified/transform.rs b/components/style/values/specified/transform.rs index 6a33b665463..44f1afe83d1 100644 --- a/components/style/values/specified/transform.rs +++ b/components/style/values/specified/transform.rs @@ -12,7 +12,7 @@ use crate::values::generics::transform::{Matrix, Matrix3D}; use crate::values::specified::position::{ HorizontalPositionKeyword, Side, VerticalPositionKeyword, }; -use crate::values::specified::{self, Angle, Integer, Length, LengthPercentage, Number}; +use crate::values::specified::{self, Angle, Integer, Length, LengthPercentage, Number, NumberOrPercentage}; use crate::Zero; use cssparser::Parser; use style_traits::{ParseError, StyleParseErrorKind}; @@ -163,32 +163,32 @@ impl Transform { Ok(generic::TransformOperation::Translate3D(tx, ty, tz)) }, "scale" => { - let sx = Number::parse(context, input)?; + let sx = NumberOrPercentage::parse(context, input)?.to_number(); if input.try(|input| input.expect_comma()).is_ok() { - let sy = Number::parse(context, input)?; + let sy = NumberOrPercentage::parse(context, input)?.to_number(); Ok(generic::TransformOperation::Scale(sx, sy)) } else { Ok(generic::TransformOperation::Scale(sx, sx)) } }, "scalex" => { - let sx = Number::parse(context, input)?; + let sx = NumberOrPercentage::parse(context, input)?.to_number(); Ok(generic::TransformOperation::ScaleX(sx)) }, "scaley" => { - let sy = Number::parse(context, input)?; + let sy = NumberOrPercentage::parse(context, input)?.to_number(); Ok(generic::TransformOperation::ScaleY(sy)) }, "scalez" => { - let sz = Number::parse(context, input)?; + let sz = NumberOrPercentage::parse(context, input)?.to_number(); Ok(generic::TransformOperation::ScaleZ(sz)) }, "scale3d" => { - let sx = Number::parse(context, input)?; + let sx = NumberOrPercentage::parse(context, input)?.to_number(); input.expect_comma()?; - let sy = Number::parse(context, input)?; + let sy = NumberOrPercentage::parse(context, input)?.to_number(); input.expect_comma()?; - let sz = Number::parse(context, input)?; + let sz = NumberOrPercentage::parse(context, input)?.to_number(); Ok(generic::TransformOperation::Scale3D(sx, sy, sz)) }, "rotate" => { @@ -445,6 +445,9 @@ impl Parse for Translate { pub type Scale = generic::Scale; impl Parse for Scale { + /// Scale accepts | , so we parse it as NumberOrPercentage, + /// and then convert into an Number if it's a Percentage. + /// https://github.com/w3c/csswg-drafts/pull/4396 fn parse<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, @@ -453,11 +456,12 @@ impl Parse for Scale { return Ok(generic::Scale::None); } - let sx = Number::parse(context, input)?; - if let Ok(sy) = input.try(|i| Number::parse(context, i)) { - if let Ok(sz) = input.try(|i| Number::parse(context, i)) { + let sx = NumberOrPercentage::parse(context, input)?.to_number(); + if let Ok(sy) = input.try(|i| NumberOrPercentage::parse(context, i)) { + let sy = sy.to_number(); + if let Ok(sz) = input.try(|i| NumberOrPercentage::parse(context, i)) { // 'scale: ' - return Ok(generic::Scale::Scale(sx, sy, sz)); + return Ok(generic::Scale::Scale(sx, sy, sz.to_number())); } // 'scale: ' From 31837a1efa276a735ef9d0371af762dcc4d0a540 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 27 Nov 2019 23:24:22 +0100 Subject: [PATCH 39/45] style: Make Rust static atoms able to be used in const contexts. I see atom dropping code generated in release builds for stuff like dropping the "class" atom here: https://searchfox.org/mozilla-central/rev/4df8821c1b824db5f40f381f48432f219d99ae36/servo/components/style/gecko/wrapper.rs#592 That is silly, and I hope making Atom be able to be used in const context will help the compiler see that yeah, we're not doing anything interesting and the atom shouldn't get dropped. It also allows us to get rid of a few lazy_static!s, so we should do it anyway. In order to accomplish this, compute the offset into gGkAtoms manually instead of going through the static_atoms() array and then back to the byte offset. Differential Revision: https://phabricator.services.mozilla.com/D55039 --- components/style/gecko/regen_atoms.py | 2 +- components/style/gecko_string_cache/mod.rs | 32 ++++++------ components/style/style_adjuster.rs | 57 ++++++++++------------ 3 files changed, 45 insertions(+), 46 deletions(-) diff --git a/components/style/gecko/regen_atoms.py b/components/style/gecko/regen_atoms.py index 0066d06d054..cf7cc77c16f 100755 --- a/components/style/gecko/regen_atoms.py +++ b/components/style/gecko/regen_atoms.py @@ -132,7 +132,7 @@ PRELUDE = ''' RULE_TEMPLATE = ''' ("{atom}") => {{{{ #[allow(unsafe_code)] #[allow(unused_unsafe)] - unsafe {{ $crate::string_cache::Atom::from_index({index}) }} + unsafe {{ $crate::string_cache::Atom::from_index_unchecked({index}) }} }}}}; '''[1:] diff --git a/components/style/gecko_string_cache/mod.rs b/components/style/gecko_string_cache/mod.rs index 5d849a96f6e..e1da47e937e 100644 --- a/components/style/gecko_string_cache/mod.rs +++ b/components/style/gecko_string_cache/mod.rs @@ -49,10 +49,15 @@ macro_rules! local_name { }; } -/// A handle to a Gecko atom. +/// A handle to a Gecko atom. This is a type that can represent either: +/// +/// * A strong reference to a dynamic atom (an `nsAtom` pointer), in which case +/// the `usize` just holds the pointer value. +/// +/// * A byte offset from `gGkAtoms` to the `nsStaticAtom` object (shifted to +/// the left one bit, and with the lower bit set to `1` to differentiate it +/// from the above), so `(offset << 1 | 1)`. /// -/// This is either a strong reference to a dynamic atom (an nsAtom pointer), -/// or an offset from gGkAtoms to the nsStaticAtom object. #[derive(Eq, PartialEq)] #[repr(C)] pub struct Atom(NonZeroUsize); @@ -87,7 +92,7 @@ fn static_atoms() -> &'static [nsStaticAtom; STATIC_ATOM_COUNT] { fn valid_static_atom_addr(addr: usize) -> bool { unsafe { let atoms = static_atoms(); - let start = atoms.get_unchecked(0) as *const _; + let start = atoms.as_ptr(); let end = atoms.get_unchecked(STATIC_ATOM_COUNT) as *const _; let in_range = addr >= start as usize && addr < end as usize; let aligned = addr % mem::align_of::() == 0; @@ -379,17 +384,16 @@ impl Atom { } /// Creates a static atom from its index in the static atom table, without - /// checking in release builds. + /// checking. #[inline] - pub unsafe fn from_index(index: u16) -> Self { - let ptr = static_atoms().get_unchecked(index as usize) as *const _; - let handle = make_static_handle(ptr); - let atom = Atom(handle); - debug_assert!(valid_static_atom_addr(ptr as usize)); - debug_assert!(atom.is_static()); - debug_assert!((*atom).is_static()); - debug_assert!(handle == make_handle(atom.as_ptr())); - atom + pub const unsafe fn from_index_unchecked(index: u16) -> Self { + // FIXME(emilio): No support for debug_assert! in const fn for now. Note + // that violating this invariant will debug-assert in the `Deref` impl + // though. + // + // debug_assert!((index as usize) < STATIC_ATOM_COUNT); + let offset = (index as usize) * std::mem::size_of::() + kGkAtomsArrayOffset as usize; + Atom(NonZeroUsize::new_unchecked((offset << 1) | 1)) } /// Creates an atom from an atom pointer. diff --git a/components/style/style_adjuster.rs b/components/style/style_adjuster.rs index ac528f4fb37..6a3dc75f4d8 100644 --- a/components/style/style_adjuster.rs +++ b/components/style/style_adjuster.rs @@ -63,43 +63,38 @@ where { use crate::Atom; - // FIXME(emilio): This should be an actual static. - lazy_static! { - static ref SPECIAL_HTML_ELEMENTS: [Atom; 16] = [ - atom!("br"), - atom!("wbr"), - atom!("meter"), - atom!("progress"), - atom!("canvas"), - atom!("embed"), - atom!("object"), - atom!("audio"), - atom!("iframe"), - atom!("img"), - atom!("video"), - atom!("frame"), - atom!("frameset"), - atom!("input"), - atom!("textarea"), - atom!("select"), - ]; - } + const SPECIAL_HTML_ELEMENTS: [Atom; 16] = [ + atom!("br"), + atom!("wbr"), + atom!("meter"), + atom!("progress"), + atom!("canvas"), + atom!("embed"), + atom!("object"), + atom!("audio"), + atom!("iframe"), + atom!("img"), + atom!("video"), + atom!("frame"), + atom!("frameset"), + atom!("input"), + atom!("textarea"), + atom!("select"), + ]; // https://drafts.csswg.org/css-display/#unbox-svg // // There's a note about "Unknown elements", but there's not a good way to // know what that means, or to get that information from here, and no other // UA implements this either. - lazy_static! { - static ref SPECIAL_SVG_ELEMENTS: [Atom; 6] = [ - atom!("svg"), - atom!("a"), - atom!("g"), - atom!("use"), - atom!("tspan"), - atom!("textPath"), - ]; - } + const SPECIAL_SVG_ELEMENTS: [Atom; 6] = [ + atom!("svg"), + atom!("a"), + atom!("g"), + atom!("use"), + atom!("tspan"), + atom!("textPath"), + ]; // https://drafts.csswg.org/css-display/#unbox-html if element.is_html_element() { From f8e924f86a5734c2e5e22fd2a8de200bbc03b238 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 30 Nov 2019 14:28:26 +0100 Subject: [PATCH 40/45] style: Don't specify minor version for cssparser. --- components/canvas/Cargo.toml | 2 +- components/canvas_traits/Cargo.toml | 2 +- components/malloc_size_of/Cargo.toml | 2 +- components/script/Cargo.toml | 2 +- components/selectors/Cargo.toml | 2 +- components/style/Cargo.toml | 2 +- components/style_traits/Cargo.toml | 2 +- components/to_shmem/Cargo.toml | 2 +- tests/unit/style/Cargo.toml | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/components/canvas/Cargo.toml b/components/canvas/Cargo.toml index 6c52fa07d1d..daa07fab430 100644 --- a/components/canvas/Cargo.toml +++ b/components/canvas/Cargo.toml @@ -22,7 +22,7 @@ bitflags = "1.0" byteorder = "1" canvas_traits = {path = "../canvas_traits"} crossbeam-channel = "0.3" -cssparser = "0.27.1" +cssparser = "0.27" embedder_traits = {path = "../embedder_traits"} euclid = "0.20" fnv = "1.0" diff --git a/components/canvas_traits/Cargo.toml b/components/canvas_traits/Cargo.toml index b4dae5e2017..7b412d993fa 100644 --- a/components/canvas_traits/Cargo.toml +++ b/components/canvas_traits/Cargo.toml @@ -14,7 +14,7 @@ path = "lib.rs" webgl_backtrace = [] [dependencies] -cssparser = "0.27.1" +cssparser = "0.27" euclid = "0.20" ipc-channel = "0.12" lazy_static = "1" diff --git a/components/malloc_size_of/Cargo.toml b/components/malloc_size_of/Cargo.toml index 9ba39c1e0d6..2e112805067 100644 --- a/components/malloc_size_of/Cargo.toml +++ b/components/malloc_size_of/Cargo.toml @@ -28,7 +28,7 @@ servo = [ app_units = "0.7" content-security-policy = {version = "0.3.0", features = ["serde"], optional = true} crossbeam-channel = { version = "0.3", optional = true } -cssparser = "0.27.1" +cssparser = "0.27" euclid = "0.20" hashglobe = { path = "../hashglobe" } hyper = { version = "0.12", optional = true } diff --git a/components/script/Cargo.toml b/components/script/Cargo.toml index d7032f290e0..cfa02f3363e 100644 --- a/components/script/Cargo.toml +++ b/components/script/Cargo.toml @@ -43,7 +43,7 @@ content-security-policy = {version = "0.3.0", features = ["serde"]} cookie = "0.11" chrono = "0.4" crossbeam-channel = "0.3" -cssparser = "0.27.1" +cssparser = "0.27" deny_public_fields = {path = "../deny_public_fields"} devtools_traits = {path = "../devtools_traits"} dom_struct = {path = "../dom_struct"} diff --git a/components/selectors/Cargo.toml b/components/selectors/Cargo.toml index d3dda6cb2c1..d32e82a2336 100644 --- a/components/selectors/Cargo.toml +++ b/components/selectors/Cargo.toml @@ -22,7 +22,7 @@ bench = [] [dependencies] bitflags = "1.0" matches = "0.1" -cssparser = "0.27.1" +cssparser = "0.27" derive_more = "0.13" log = "0.4" fxhash = "0.2" diff --git a/components/style/Cargo.toml b/components/style/Cargo.toml index b068e0f5899..4489bece8ad 100644 --- a/components/style/Cargo.toml +++ b/components/style/Cargo.toml @@ -32,7 +32,7 @@ arrayvec = "0.5" atomic_refcell = "0.1" bitflags = "1.0" byteorder = "1.0" -cssparser = "0.27.1" +cssparser = "0.27" crossbeam-channel = { version = "0.3", optional = true } derive_more = "0.13" new_debug_unreachable = "1.0" diff --git a/components/style_traits/Cargo.toml b/components/style_traits/Cargo.toml index f3dd0df744f..1f3f3e9ddb1 100644 --- a/components/style_traits/Cargo.toml +++ b/components/style_traits/Cargo.toml @@ -15,7 +15,7 @@ gecko = [] [dependencies] app_units = "0.7" -cssparser = "0.27.1" +cssparser = "0.27" bitflags = "1.0" euclid = "0.20" lazy_static = "1" diff --git a/components/to_shmem/Cargo.toml b/components/to_shmem/Cargo.toml index 71426507647..dd3614111e1 100644 --- a/components/to_shmem/Cargo.toml +++ b/components/to_shmem/Cargo.toml @@ -14,7 +14,7 @@ servo = ["cssparser/serde", "string_cache"] gecko = [] [dependencies] -cssparser = "0.27.1" +cssparser = "0.27" servo_arc = { path = "../servo_arc" } smallbitvec = "2.1.1" smallvec = "0.6.6" diff --git a/tests/unit/style/Cargo.toml b/tests/unit/style/Cargo.toml index f25e4a5b20f..2764c9d0408 100644 --- a/tests/unit/style/Cargo.toml +++ b/tests/unit/style/Cargo.toml @@ -11,7 +11,7 @@ doctest = false [dependencies] app_units = "0.7" -cssparser = "0.27.1" +cssparser = "0.27" euclid = "0.20" html5ever = "0.25" rayon = "1" From 59eef57eb77faea7ce11cd899647f40c3f333b8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 30 Nov 2019 14:30:01 +0100 Subject: [PATCH 41/45] style: Undo minor debugging change. --- components/style/rule_cache.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/components/style/rule_cache.rs b/components/style/rule_cache.rs index 117f63f9ea2..24039fb94f7 100644 --- a/components/style/rule_cache.rs +++ b/components/style/rule_cache.rs @@ -26,9 +26,7 @@ pub struct RuleCacheConditions { impl RuleCacheConditions { /// Sets the style as depending in the font-size value. pub fn set_font_size_dependency(&mut self, font_size: NonNegativeLength) { - if let Some(f) = &self.font_size { - debug_assert_eq!(*f, font_size); - } + debug_assert!(self.font_size.map_or(true, |f| f == font_size)); self.font_size = Some(font_size); } From 006417e40ace554db56d98b2df44f04f776268df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 30 Nov 2019 14:34:45 +0100 Subject: [PATCH 42/45] style: Rustfmt recent changes. --- components/style/applicable_declarations.rs | 14 +- components/style/gecko/snapshot_helpers.rs | 10 +- components/style/gecko/wrapper.rs | 6 +- components/style/gecko_string_cache/mod.rs | 3 +- .../style/invalidation/element/invalidator.rs | 2 +- components/style/rule_collector.rs | 10 +- components/style/rule_tree/mod.rs | 55 +++--- components/style/selector_map.rs | 4 +- components/style/stylist.rs | 7 +- components/style/values/animated/transform.rs | 168 +++++++++--------- components/style/values/generics/grid.rs | 5 +- components/style/values/generics/transform.rs | 32 ++-- components/style/values/specified/angle.rs | 4 +- components/style/values/specified/box.rs | 2 +- components/style/values/specified/calc.rs | 20 +-- components/style/values/specified/color.rs | 8 +- components/style/values/specified/counters.rs | 2 +- components/style/values/specified/font.rs | 120 ++++++------- components/style/values/specified/length.rs | 11 +- .../style/values/specified/percentage.rs | 5 +- components/style/values/specified/time.rs | 5 +- .../style/values/specified/transform.rs | 4 +- 22 files changed, 249 insertions(+), 248 deletions(-) diff --git a/components/style/applicable_declarations.rs b/components/style/applicable_declarations.rs index cc7d9c90db8..6353fb4c5e5 100644 --- a/components/style/applicable_declarations.rs +++ b/components/style/applicable_declarations.rs @@ -36,12 +36,15 @@ const CASCADE_LEVEL_SHIFT: usize = SOURCE_ORDER_BITS; /// Stores the source order of a block, the cascade level it belongs to, and the /// counter needed to handle Shadow DOM cascade order properly. -#[derive(Clone, Copy, Eq, MallocSizeOf, PartialEq, Debug)] +#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq)] struct ApplicableDeclarationBits(u32); impl ApplicableDeclarationBits { fn new(source_order: u32, cascade_level: CascadeLevel) -> Self { - Self((source_order & SOURCE_ORDER_MASK) | ((cascade_level.to_byte_lossy() as u32) << CASCADE_LEVEL_SHIFT)) + Self( + (source_order & SOURCE_ORDER_MASK) | + ((cascade_level.to_byte_lossy() as u32) << CASCADE_LEVEL_SHIFT), + ) } fn source_order(&self) -> u32 { @@ -87,12 +90,7 @@ impl ApplicableDeclarationBlock { /// Constructs an applicable declaration block from the given components #[inline] - pub fn new( - source: StyleSource, - order: u32, - level: CascadeLevel, - specificity: u32, - ) -> Self { + pub fn new(source: StyleSource, order: u32, level: CascadeLevel, specificity: u32) -> Self { ApplicableDeclarationBlock { source, bits: ApplicableDeclarationBits::new(order, level), diff --git a/components/style/gecko/snapshot_helpers.rs b/components/style/gecko/snapshot_helpers.rs index 24b3ebb1991..cb3056e7bd5 100644 --- a/components/style/gecko/snapshot_helpers.rs +++ b/components/style/gecko/snapshot_helpers.rs @@ -83,7 +83,10 @@ pub fn get_id(attrs: &[structs::AttrArray_InternalAttr]) -> Option<&WeakAtom> { } #[inline(always)] -pub(super) fn exported_part(attrs: &[structs::AttrArray_InternalAttr], name: &Atom) -> Option { +pub(super) fn exported_part( + attrs: &[structs::AttrArray_InternalAttr], + name: &Atom, +) -> Option { let attr = find_attr(attrs, &atom!("exportparts"))?; let atom = unsafe { bindings::Gecko_Element_ExportedPart(attr, name.as_ptr()) }; if atom.is_null() { @@ -93,7 +96,10 @@ pub(super) fn exported_part(attrs: &[structs::AttrArray_InternalAttr], name: &At } #[inline(always)] -pub(super) fn imported_part(attrs: &[structs::AttrArray_InternalAttr], name: &Atom) -> Option { +pub(super) fn imported_part( + attrs: &[structs::AttrArray_InternalAttr], + name: &Atom, +) -> Option { let attr = find_attr(attrs, &atom!("exportparts"))?; let atom = unsafe { bindings::Gecko_Element_ImportedPart(attr, name.as_ptr()) }; if atom.is_null() { diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index ac47927744b..bf4da090646 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -1244,7 +1244,8 @@ impl<'le> TElement for GeckoElement<'le> { #[inline] fn has_part_attr(&self) -> bool { - self.as_node().get_bool_flag(nsINode_BooleanFlag::ElementHasPart) + self.as_node() + .get_bool_flag(nsINode_BooleanFlag::ElementHasPart) } #[inline] @@ -2193,7 +2194,8 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { #[inline] fn is_link(&self) -> bool { - self.state().intersects(ElementState::IN_VISITED_OR_UNVISITED_STATE) + self.state() + .intersects(ElementState::IN_VISITED_OR_UNVISITED_STATE) } #[inline] diff --git a/components/style/gecko_string_cache/mod.rs b/components/style/gecko_string_cache/mod.rs index e1da47e937e..b9e3a0c608a 100644 --- a/components/style/gecko_string_cache/mod.rs +++ b/components/style/gecko_string_cache/mod.rs @@ -392,7 +392,8 @@ impl Atom { // though. // // debug_assert!((index as usize) < STATIC_ATOM_COUNT); - let offset = (index as usize) * std::mem::size_of::() + kGkAtomsArrayOffset as usize; + let offset = + (index as usize) * std::mem::size_of::() + kGkAtomsArrayOffset as usize; Atom(NonZeroUsize::new_unchecked((offset << 1) | 1)) } diff --git a/components/style/invalidation/element/invalidator.rs b/components/style/invalidation/element/invalidator.rs index 8bc898f9587..e094be281b4 100644 --- a/components/style/invalidation/element/invalidator.rs +++ b/components/style/invalidation/element/invalidator.rs @@ -8,10 +8,10 @@ use crate::context::StackLimitChecker; use crate::dom::{TElement, TNode, TShadowRoot}; use crate::selector_parser::SelectorImpl; -use selectors::OpaqueElement; use selectors::matching::matches_compound_selector_from; use selectors::matching::{CompoundSelectorMatchingResult, MatchingContext}; use selectors::parser::{Combinator, Component, Selector}; +use selectors::OpaqueElement; use smallvec::SmallVec; use std::fmt; diff --git a/components/style/rule_collector.rs b/components/style/rule_collector.rs index c0197572e0a..ab1fb88d7f2 100644 --- a/components/style/rule_collector.rs +++ b/components/style/rule_collector.rs @@ -4,7 +4,6 @@ //! Collects a series of applicable rules for a given element. -use crate::Atom; use crate::applicable_declarations::{ApplicableDeclarationBlock, ApplicableDeclarationList}; use crate::dom::{TElement, TNode, TShadowRoot}; use crate::properties::{AnimationRules, PropertyDeclarationBlock}; @@ -14,6 +13,7 @@ use crate::selector_parser::PseudoElement; use crate::shared_lock::Locked; use crate::stylesheets::Origin; use crate::stylist::{AuthorStylesEnabled, Rule, RuleInclusion, Stylist}; +use crate::Atom; use selectors::matching::{ElementSelectorFlags, MatchingContext, MatchingMode}; use servo_arc::ArcBorrow; use smallvec::SmallVec; @@ -261,7 +261,9 @@ where self.collect_rules_in_shadow_tree( shadow.host(), slotted_rules, - CascadeLevel::AuthorNormal { shadow_cascade_order }, + CascadeLevel::AuthorNormal { + shadow_cascade_order, + }, ); } } @@ -312,7 +314,9 @@ where self.collect_rules_in_shadow_tree( rule_hash_target, host_rules, - CascadeLevel::AuthorNormal { shadow_cascade_order }, + CascadeLevel::AuthorNormal { + shadow_cascade_order, + }, ); } diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs index 7236c32e30d..7cb153de375 100644 --- a/components/style/rule_tree/mod.rs +++ b/components/style/rule_tree/mod.rs @@ -283,7 +283,9 @@ impl RuleTree { if any_important { found_important = true; match level { - AuthorNormal { shadow_cascade_order } => { + AuthorNormal { + shadow_cascade_order, + } => { important_author.push((source.clone(), shadow_cascade_order)); }, UANormal => important_ua.push(source.clone()), @@ -344,9 +346,13 @@ impl RuleTree { } for (source, shadow_cascade_order) in important_author.drain() { - current = current.ensure_child(self.root.downgrade(), source, AuthorImportant { - shadow_cascade_order: -shadow_cascade_order, - }); + current = current.ensure_child( + self.root.downgrade(), + source, + AuthorImportant { + shadow_cascade_order: -shadow_cascade_order, + }, + ); } for source in important_user.drain() { @@ -496,12 +502,7 @@ impl RuleTree { if current.get().level == level { *important_rules_changed |= level.is_important(); - let current_decls = current - .get() - .source - .as_ref() - .unwrap() - .as_declarations(); + let current_decls = current.get().source.as_ref().unwrap().as_declarations(); // If the only rule at the level we're replacing is exactly the // same as `pdb`, we're done, and `path` is still valid. @@ -701,10 +702,14 @@ impl CascadeLevel { Self::UANormal => (0, 0), Self::UserNormal => (1, 0), Self::PresHints => (2, 0), - Self::AuthorNormal { shadow_cascade_order } => (3, shadow_cascade_order.0), + Self::AuthorNormal { + shadow_cascade_order, + } => (3, shadow_cascade_order.0), Self::SMILOverride => (4, 0), Self::Animations => (5, 0), - Self::AuthorImportant { shadow_cascade_order } => (6, shadow_cascade_order.0), + Self::AuthorImportant { + shadow_cascade_order, + } => (6, shadow_cascade_order.0), Self::UserImportant => (7, 0), Self::UAImportant => (8, 0), Self::Transitions => (9, 0), @@ -727,20 +732,28 @@ impl CascadeLevel { let order = { let abs = ((b & 0b01110000) >> 4) as i8; let negative = b & 0b10000000 != 0; - if negative { -abs } else { abs } + if negative { + -abs + } else { + abs + } }; let discriminant = b & 0xf; let level = match discriminant { 0 => Self::UANormal, 1 => Self::UserNormal, 2 => Self::PresHints, - 3 => return Self::AuthorNormal { - shadow_cascade_order: ShadowCascadeOrder(order), + 3 => { + return Self::AuthorNormal { + shadow_cascade_order: ShadowCascadeOrder(order), + } }, 4 => Self::SMILOverride, 5 => Self::Animations, - 6 => return Self::AuthorImportant { - shadow_cascade_order: ShadowCascadeOrder(order), + 6 => { + return Self::AuthorImportant { + shadow_cascade_order: ShadowCascadeOrder(order), + } }, 7 => Self::UserImportant, 8 => Self::UAImportant, @@ -1550,9 +1563,7 @@ impl StrongRuleNode { // FIXME(emilio): this looks wrong, this should // do: if color is not transparent, then return // true, or something. - if let PropertyDeclaration::BackgroundColor(ref color) = - *declaration - { + if let PropertyDeclaration::BackgroundColor(ref color) = *declaration { return *color == Color::transparent(); } } @@ -1567,9 +1578,7 @@ impl StrongRuleNode { // However, if it is inherited, then it might be // inherited from an author rule from an // ancestor element's rule nodes. - if declaration.get_css_wide_keyword() == - Some(CSSWideKeyword::Inherit) - { + if declaration.get_css_wide_keyword() == Some(CSSWideKeyword::Inherit) { have_explicit_ua_inherit = true; inherited_properties.insert(id); } diff --git a/components/style/selector_map.rs b/components/style/selector_map.rs index 81144c2a4bf..cd99852861c 100644 --- a/components/style/selector_map.rs +++ b/components/style/selector_map.rs @@ -271,9 +271,7 @@ impl SelectorMap { context, flags_setter, ) { - matching_rules.push( - rule.to_applicable_declaration_block(cascade_level), - ); + matching_rules.push(rule.to_applicable_declaration_block(cascade_level)); } } } diff --git a/components/style/stylist.rs b/components/style/stylist.rs index bd499027219..83db2ec4b17 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -2325,12 +2325,7 @@ impl Rule { level: CascadeLevel, ) -> ApplicableDeclarationBlock { let source = StyleSource::from_rule(self.style_rule.clone()); - ApplicableDeclarationBlock::new( - source, - self.source_order, - level, - self.specificity(), - ) + ApplicableDeclarationBlock::new(source, self.source_order, level, self.specificity()) } /// Creates a new Rule. diff --git a/components/style/values/animated/transform.rs b/components/style/values/animated/transform.rs index 5a6f3afb9c1..aceb70f6766 100644 --- a/components/style/values/animated/transform.rs +++ b/components/style/values/animated/transform.rs @@ -139,10 +139,10 @@ impl ComputeSquaredDistance for MatrixDecomposed2D { const RAD_PER_DEG: f64 = std::f64::consts::PI / 180.0; let angle1 = self.angle as f64 * RAD_PER_DEG; let angle2 = other.angle as f64 * RAD_PER_DEG; - Ok(self.translate.compute_squared_distance(&other.translate)? - + self.scale.compute_squared_distance(&other.scale)? - + angle1.compute_squared_distance(&angle2)? - + self.matrix.compute_squared_distance(&other.matrix)?) + Ok(self.translate.compute_squared_distance(&other.translate)? + + self.scale.compute_squared_distance(&other.scale)? + + angle1.compute_squared_distance(&angle2)? + + self.matrix.compute_squared_distance(&other.matrix)?) } } @@ -316,9 +316,9 @@ impl ComputeSquaredDistance for Skew { // ComputeSquaredDistance manually. #[inline] fn compute_squared_distance(&self, other: &Self) -> Result { - Ok(self.0.atan().compute_squared_distance(&other.0.atan())? - + self.1.atan().compute_squared_distance(&other.1.atan())? - + self.2.atan().compute_squared_distance(&other.2.atan())?) + Ok(self.0.atan().compute_squared_distance(&other.0.atan())? + + self.1.atan().compute_squared_distance(&other.1.atan())? + + self.2.atan().compute_squared_distance(&other.2.atan())?) } } @@ -394,9 +394,9 @@ impl Animate for Quaternion { debug_assert!( // Doule EPSILON since both this_weight and other_weght have calculation errors // which are approximately equal to EPSILON. - (this_weight + other_weight - 1.0f64).abs() <= f64::EPSILON * 2.0 - || other_weight == 1.0f64 - || other_weight == 0.0f64, + (this_weight + other_weight - 1.0f64).abs() <= f64::EPSILON * 2.0 || + other_weight == 1.0f64 || + other_weight == 0.0f64, "animate should only be used for interpolating or accumulating transforms" ); @@ -830,17 +830,17 @@ fn is_matched_operation( second: &ComputedTransformOperation, ) -> bool { match (first, second) { - (&TransformOperation::Matrix(..), &TransformOperation::Matrix(..)) - | (&TransformOperation::Matrix3D(..), &TransformOperation::Matrix3D(..)) - | (&TransformOperation::Skew(..), &TransformOperation::Skew(..)) - | (&TransformOperation::SkewX(..), &TransformOperation::SkewX(..)) - | (&TransformOperation::SkewY(..), &TransformOperation::SkewY(..)) - | (&TransformOperation::Rotate(..), &TransformOperation::Rotate(..)) - | (&TransformOperation::Rotate3D(..), &TransformOperation::Rotate3D(..)) - | (&TransformOperation::RotateX(..), &TransformOperation::RotateX(..)) - | (&TransformOperation::RotateY(..), &TransformOperation::RotateY(..)) - | (&TransformOperation::RotateZ(..), &TransformOperation::RotateZ(..)) - | (&TransformOperation::Perspective(..), &TransformOperation::Perspective(..)) => true, + (&TransformOperation::Matrix(..), &TransformOperation::Matrix(..)) | + (&TransformOperation::Matrix3D(..), &TransformOperation::Matrix3D(..)) | + (&TransformOperation::Skew(..), &TransformOperation::Skew(..)) | + (&TransformOperation::SkewX(..), &TransformOperation::SkewX(..)) | + (&TransformOperation::SkewY(..), &TransformOperation::SkewY(..)) | + (&TransformOperation::Rotate(..), &TransformOperation::Rotate(..)) | + (&TransformOperation::Rotate3D(..), &TransformOperation::Rotate3D(..)) | + (&TransformOperation::RotateX(..), &TransformOperation::RotateX(..)) | + (&TransformOperation::RotateY(..), &TransformOperation::RotateY(..)) | + (&TransformOperation::RotateZ(..), &TransformOperation::RotateZ(..)) | + (&TransformOperation::Perspective(..), &TransformOperation::Perspective(..)) => true, // Match functions that have the same primitive transform function (a, b) if a.is_translate() && b.is_translate() => true, (a, b) if a.is_scale() && b.is_scale() => true, @@ -895,21 +895,21 @@ impl Animate for ComputedTransform { Procedure::Add => { debug_assert!(false, "Should have already dealt with add by the point"); return Err(()); - } + }, Procedure::Interpolate { progress } => { result.push(TransformOperation::InterpolateMatrix { from_list: Transform(this_remainder.to_vec().into()), to_list: Transform(other_remainder.to_vec().into()), progress: Percentage(progress as f32), }); - } + }, Procedure::Accumulate { count } => { result.push(TransformOperation::AccumulateMatrix { from_list: Transform(this_remainder.to_vec().into()), to_list: Transform(other_remainder.to_vec().into()), count: cmp::min(count, i32::max_value() as u64) as i32, }); - } + }, }, // If there is a remainder from just one list, then one list must be shorter but // completely match the type of the corresponding functions in the longer list. @@ -925,8 +925,8 @@ impl Animate for ComputedTransform { match transform { // We can't interpolate/accumulate ___Matrix types directly with a // matrix. Instead we need to wrap it in another ___Matrix type. - TransformOperation::AccumulateMatrix { .. } - | TransformOperation::InterpolateMatrix { .. } => { + TransformOperation::AccumulateMatrix { .. } | + TransformOperation::InterpolateMatrix { .. } => { let transform_list = Transform(vec![transform.clone()].into()); let identity_list = Transform(vec![identity].into()); let (from_list, to_list) = if fill_right { @@ -943,7 +943,7 @@ impl Animate for ComputedTransform { to_list, progress: Percentage(progress as f32), }) - } + }, Procedure::Accumulate { count } => { Ok(TransformOperation::AccumulateMatrix { from_list, @@ -951,9 +951,9 @@ impl Animate for ComputedTransform { count: cmp::min(count, i32::max_value() as u64) as i32, }) - } + }, } - } + }, _ => { let (lhs, rhs) = if fill_right { (transform, &identity) @@ -961,13 +961,13 @@ impl Animate for ComputedTransform { (&identity, transform) }; lhs.animate(rhs, procedure) - } + }, } }) .collect::, _>>()?, ); - } - (None, None) => {} + }, + (None, None) => {}, } Ok(Transform(result.into())) @@ -999,10 +999,10 @@ impl Animate for ComputedTransformOperation { Ok(TransformOperation::Matrix3D( this.animate(other, procedure)?, )) - } + }, (&TransformOperation::Matrix(ref this), &TransformOperation::Matrix(ref other)) => { Ok(TransformOperation::Matrix(this.animate(other, procedure)?)) - } + }, ( &TransformOperation::Skew(ref fx, ref fy), &TransformOperation::Skew(ref tx, ref ty), @@ -1012,10 +1012,10 @@ impl Animate for ComputedTransformOperation { )), (&TransformOperation::SkewX(ref f), &TransformOperation::SkewX(ref t)) => { Ok(TransformOperation::SkewX(f.animate(t, procedure)?)) - } + }, (&TransformOperation::SkewY(ref f), &TransformOperation::SkewY(ref t)) => { Ok(TransformOperation::SkewY(f.animate(t, procedure)?)) - } + }, ( &TransformOperation::Translate3D(ref fx, ref fy, ref fz), &TransformOperation::Translate3D(ref tx, ref ty, ref tz), @@ -1033,13 +1033,13 @@ impl Animate for ComputedTransformOperation { )), (&TransformOperation::TranslateX(ref f), &TransformOperation::TranslateX(ref t)) => { Ok(TransformOperation::TranslateX(f.animate(t, procedure)?)) - } + }, (&TransformOperation::TranslateY(ref f), &TransformOperation::TranslateY(ref t)) => { Ok(TransformOperation::TranslateY(f.animate(t, procedure)?)) - } + }, (&TransformOperation::TranslateZ(ref f), &TransformOperation::TranslateZ(ref t)) => { Ok(TransformOperation::TranslateZ(f.animate(t, procedure)?)) - } + }, ( &TransformOperation::Scale3D(ref fx, ref fy, ref fz), &TransformOperation::Scale3D(ref tx, ref ty, ref tz), @@ -1072,25 +1072,25 @@ impl Animate for ComputedTransformOperation { .animate(&Rotate::Rotate3D(tx, ty, tz, ta), procedure)?; let (fx, fy, fz, fa) = ComputedRotate::resolve(&animated); Ok(TransformOperation::Rotate3D(fx, fy, fz, fa)) - } + }, (&TransformOperation::RotateX(fa), &TransformOperation::RotateX(ta)) => { Ok(TransformOperation::RotateX(fa.animate(&ta, procedure)?)) - } + }, (&TransformOperation::RotateY(fa), &TransformOperation::RotateY(ta)) => { Ok(TransformOperation::RotateY(fa.animate(&ta, procedure)?)) - } + }, (&TransformOperation::RotateZ(fa), &TransformOperation::RotateZ(ta)) => { Ok(TransformOperation::RotateZ(fa.animate(&ta, procedure)?)) - } + }, (&TransformOperation::Rotate(fa), &TransformOperation::Rotate(ta)) => { Ok(TransformOperation::Rotate(fa.animate(&ta, procedure)?)) - } + }, (&TransformOperation::Rotate(fa), &TransformOperation::RotateZ(ta)) => { Ok(TransformOperation::Rotate(fa.animate(&ta, procedure)?)) - } + }, (&TransformOperation::RotateZ(fa), &TransformOperation::Rotate(ta)) => { Ok(TransformOperation::Rotate(fa.animate(&ta, procedure)?)) - } + }, ( &TransformOperation::Perspective(ref fd), &TransformOperation::Perspective(ref td), @@ -1120,13 +1120,13 @@ impl Animate for ComputedTransformOperation { Ok(TransformOperation::Perspective(CSSPixelLength::new( used_value, ))) - } + }, _ if self.is_translate() && other.is_translate() => self .to_translate_3d() .animate(&other.to_translate_3d(), procedure), _ if self.is_scale() && other.is_scale() => { self.to_scale_3d().animate(&other.to_scale_3d(), procedure) - } + }, _ if self.is_rotate() && other.is_rotate() => self .to_rotate_3d() .animate(&other.to_rotate_3d(), procedure), @@ -1144,20 +1144,20 @@ impl ComputeSquaredDistance for ComputedTransformOperation { match (self, other) { (&TransformOperation::Matrix3D(ref this), &TransformOperation::Matrix3D(ref other)) => { this.compute_squared_distance(other) - } + }, (&TransformOperation::Matrix(ref this), &TransformOperation::Matrix(ref other)) => { let this: Matrix3D = (*this).into(); let other: Matrix3D = (*other).into(); this.compute_squared_distance(&other) - } + }, ( &TransformOperation::Skew(ref fx, ref fy), &TransformOperation::Skew(ref tx, ref ty), ) => Ok(fx.compute_squared_distance(&tx)? + fy.compute_squared_distance(&ty)?), - (&TransformOperation::SkewX(ref f), &TransformOperation::SkewX(ref t)) - | (&TransformOperation::SkewY(ref f), &TransformOperation::SkewY(ref t)) => { + (&TransformOperation::SkewX(ref f), &TransformOperation::SkewX(ref t)) | + (&TransformOperation::SkewY(ref f), &TransformOperation::SkewY(ref t)) => { f.compute_squared_distance(&t) - } + }, ( &TransformOperation::Translate3D(ref fx, ref fy, ref fz), &TransformOperation::Translate3D(ref tx, ref ty, ref tz), @@ -1173,33 +1173,33 @@ impl ComputeSquaredDistance for ComputedTransformOperation { let tx = tx.length_component().px(); let ty = ty.length_component().px(); - Ok(fx.compute_squared_distance(&tx)? - + fy.compute_squared_distance(&ty)? - + fz.compute_squared_distance(&tz)?) - } + Ok(fx.compute_squared_distance(&tx)? + + fy.compute_squared_distance(&ty)? + + fz.compute_squared_distance(&tz)?) + }, ( &TransformOperation::Scale3D(ref fx, ref fy, ref fz), &TransformOperation::Scale3D(ref tx, ref ty, ref tz), - ) => Ok(fx.compute_squared_distance(&tx)? - + fy.compute_squared_distance(&ty)? - + fz.compute_squared_distance(&tz)?), + ) => Ok(fx.compute_squared_distance(&tx)? + + fy.compute_squared_distance(&ty)? + + fz.compute_squared_distance(&tz)?), ( &TransformOperation::Rotate3D(fx, fy, fz, fa), &TransformOperation::Rotate3D(tx, ty, tz, ta), ) => Rotate::Rotate3D(fx, fy, fz, fa) .compute_squared_distance(&Rotate::Rotate3D(tx, ty, tz, ta)), - (&TransformOperation::RotateX(fa), &TransformOperation::RotateX(ta)) - | (&TransformOperation::RotateY(fa), &TransformOperation::RotateY(ta)) - | (&TransformOperation::RotateZ(fa), &TransformOperation::RotateZ(ta)) - | (&TransformOperation::Rotate(fa), &TransformOperation::Rotate(ta)) => { + (&TransformOperation::RotateX(fa), &TransformOperation::RotateX(ta)) | + (&TransformOperation::RotateY(fa), &TransformOperation::RotateY(ta)) | + (&TransformOperation::RotateZ(fa), &TransformOperation::RotateZ(ta)) | + (&TransformOperation::Rotate(fa), &TransformOperation::Rotate(ta)) => { fa.compute_squared_distance(&ta) - } + }, ( &TransformOperation::Perspective(ref fd), &TransformOperation::Perspective(ref td), ) => fd.compute_squared_distance(td), - (&TransformOperation::Perspective(ref p), &TransformOperation::Matrix3D(ref m)) - | (&TransformOperation::Matrix3D(ref m), &TransformOperation::Perspective(ref p)) => { + (&TransformOperation::Perspective(ref p), &TransformOperation::Matrix3D(ref m)) | + (&TransformOperation::Matrix3D(ref m), &TransformOperation::Perspective(ref p)) => { // FIXME(emilio): Is this right? Why interpolating this with // Perspective but not with anything else? let mut p_matrix = Matrix3D::identity(); @@ -1207,7 +1207,7 @@ impl ComputeSquaredDistance for ComputedTransformOperation { p_matrix.m34 = -1. / p.px(); } p_matrix.compute_squared_distance(&m) - } + }, // Gecko cross-interpolates amongst all translate and all scale // functions (See ToPrimitive in layout/style/StyleAnimationValue.cpp) // without falling back to InterpolateMatrix @@ -1260,7 +1260,7 @@ impl Animate for ComputedRotate { fz, fa.animate(&Angle::zero(), procedure)?, )) - } + }, (&Rotate::None, &Rotate::Rotate3D(tx, ty, tz, ta)) => { // Normalize direction vector first. let (tx, ty, tz, ta) = transform::get_normalized_vector_and_angle(tx, ty, tz, ta); @@ -1270,7 +1270,7 @@ impl Animate for ComputedRotate { tz, Angle::zero().animate(&ta, procedure)?, )) - } + }, (&Rotate::Rotate3D(_, ..), _) | (_, &Rotate::Rotate3D(_, ..)) => { let (from, to) = (self.resolve(), other.resolve()); let (mut fx, mut fy, mut fz, fa) = @@ -1306,12 +1306,12 @@ impl Animate for ComputedRotate { ); Ok(Rotate::Rotate3D(x, y, z, Angle::from_radians(angle))) - } + }, (&Rotate::Rotate(_), _) | (_, &Rotate::Rotate(_)) => { // If this is a 2D rotation, we just animate the let (from, to) = (self.resolve().3, other.resolve().3); Ok(Rotate::Rotate(from.animate(&to, procedure)?)) - } + }, } } } @@ -1321,10 +1321,10 @@ impl ComputeSquaredDistance for ComputedRotate { fn compute_squared_distance(&self, other: &Self) -> Result { match (self, other) { (&Rotate::None, &Rotate::None) => Ok(SquaredDistance::from_sqrt(0.)), - (&Rotate::Rotate3D(_, _, _, a), &Rotate::None) - | (&Rotate::None, &Rotate::Rotate3D(_, _, _, a)) => { + (&Rotate::Rotate3D(_, _, _, a), &Rotate::None) | + (&Rotate::None, &Rotate::Rotate3D(_, _, _, a)) => { a.compute_squared_distance(&Angle::zero()) - } + }, (&Rotate::Rotate3D(_, ..), _) | (_, &Rotate::Rotate3D(_, ..)) => { let (from, to) = (self.resolve(), other.resolve()); let (mut fx, mut fy, mut fz, angle1) = @@ -1351,7 +1351,7 @@ impl ComputeSquaredDistance for ComputedRotate { let q2 = Quaternion::from_direction_and_angle(&v2, angle2.radians64()); q1.compute_squared_distance(&q2) } - } + }, (&Rotate::Rotate(_), _) | (_, &Rotate::Rotate(_)) => self .resolve() .3 @@ -1390,7 +1390,7 @@ impl Animate for ComputedTranslate { from.1.animate(&to.1, procedure)?, from.2.animate(&to.2, procedure)?, )) - } + }, } } } @@ -1399,9 +1399,9 @@ impl ComputeSquaredDistance for ComputedTranslate { #[inline] fn compute_squared_distance(&self, other: &Self) -> Result { let (from, to) = (self.resolve(), other.resolve()); - Ok(from.0.compute_squared_distance(&to.0)? - + from.1.compute_squared_distance(&to.1)? - + from.2.compute_squared_distance(&to.2)?) + Ok(from.0.compute_squared_distance(&to.0)? + + from.1.compute_squared_distance(&to.1)? + + from.2.compute_squared_distance(&to.2)?) } } @@ -1439,7 +1439,7 @@ impl Animate for ComputedScale { animate_multiplicative_factor(from.1, to.1, procedure)?, animate_multiplicative_factor(from.2, to.2, procedure)?, )) - } + }, } } } @@ -1448,8 +1448,8 @@ impl ComputeSquaredDistance for ComputedScale { #[inline] fn compute_squared_distance(&self, other: &Self) -> Result { let (from, to) = (self.resolve(), other.resolve()); - Ok(from.0.compute_squared_distance(&to.0)? - + from.1.compute_squared_distance(&to.1)? - + from.2.compute_squared_distance(&to.2)?) + Ok(from.0.compute_squared_distance(&to.0)? + + from.1.compute_squared_distance(&to.1)? + + from.2.compute_squared_distance(&to.2)?) } } diff --git a/components/style/values/generics/grid.rs b/components/style/values/generics/grid.rs index 70b0a491b8e..5666e2c5818 100644 --- a/components/style/values/generics/grid.rs +++ b/components/style/values/generics/grid.rs @@ -543,7 +543,10 @@ impl TrackListValue { /// Returns true if `self` is the initial value. pub fn is_initial(&self) -> bool { - matches!(*self, TrackListValue::TrackSize(TrackSize::Breadth(TrackBreadth::Auto))) // FIXME: can't use Self::INITIAL_VALUE here yet: https://github.com/rust-lang/rust/issues/66585 + matches!( + *self, + TrackListValue::TrackSize(TrackSize::Breadth(TrackBreadth::Auto)) + ) // FIXME: can't use Self::INITIAL_VALUE here yet: https://github.com/rust-lang/rust/issues/66585 } } diff --git a/components/style/values/generics/transform.rs b/components/style/values/generics/transform.rs index cc401966a97..3322323e76e 100644 --- a/components/style/values/generics/transform.rs +++ b/components/style/values/generics/transform.rs @@ -303,7 +303,7 @@ where match *self { Translate(..) | Translate3D(..) | TranslateX(..) | TranslateY(..) | TranslateZ(..) => { true - } + }, _ => false, } } @@ -415,8 +415,8 @@ where fn is_3d(&self) -> bool { use self::TransformOperation::*; match *self { - Translate3D(..) | TranslateZ(..) | Rotate3D(..) | RotateX(..) | RotateY(..) - | RotateZ(..) | Scale3D(..) | ScaleZ(..) | Perspective(..) | Matrix3D(..) => true, + Translate3D(..) | TranslateZ(..) | Rotate3D(..) | RotateX(..) | RotateY(..) | + RotateZ(..) | Scale3D(..) | ScaleZ(..) | Perspective(..) | Matrix3D(..) => true, _ => false, } } @@ -444,23 +444,23 @@ where az as f64, euclid::Angle::radians(theta), ) - } + }, RotateX(theta) => { let theta = euclid::Angle::radians(TWO_PI - theta.radians64()); Transform3D::create_rotation(1., 0., 0., theta) - } + }, RotateY(theta) => { let theta = euclid::Angle::radians(TWO_PI - theta.radians64()); Transform3D::create_rotation(0., 1., 0., theta) - } + }, RotateZ(theta) | Rotate(theta) => { let theta = euclid::Angle::radians(TWO_PI - theta.radians64()); Transform3D::create_rotation(0., 0., 1., theta) - } + }, Perspective(ref d) => { let m = create_perspective_matrix(d.to_pixel_length(None)?); m.cast() - } + }, Scale3D(sx, sy, sz) => Transform3D::create_scale(sx.into(), sy.into(), sz.into()), Scale(sx, sy) => Transform3D::create_scale(sx.into(), sy.into(), 1.), ScaleX(s) => Transform3D::create_scale(s.into(), 1., 1.), @@ -470,23 +470,23 @@ where let tx = tx.to_pixel_length(reference_width)? as f64; let ty = ty.to_pixel_length(reference_height)? as f64; Transform3D::create_translation(tx, ty, tz.to_pixel_length(None)? as f64) - } + }, Translate(ref tx, ref ty) => { let tx = tx.to_pixel_length(reference_width)? as f64; let ty = ty.to_pixel_length(reference_height)? as f64; Transform3D::create_translation(tx, ty, 0.) - } + }, TranslateX(ref t) => { let t = t.to_pixel_length(reference_width)? as f64; Transform3D::create_translation(t, 0., 0.) - } + }, TranslateY(ref t) => { let t = t.to_pixel_length(reference_height)? as f64; Transform3D::create_translation(0., t, 0.) - } + }, TranslateZ(ref z) => { Transform3D::create_translation(0., 0., z.to_pixel_length(None)? as f64) - } + }, Skew(theta_x, theta_y) => Transform3D::create_skew( euclid::Angle::radians(theta_x.radians64()), euclid::Angle::radians(theta_y.radians64()), @@ -509,7 +509,7 @@ where // return an identity matrix. // Note: DOMMatrix doesn't go into this arm. Transform3D::identity() - } + }, }; Ok(matrix) } @@ -676,7 +676,7 @@ where } dest.write_char(' ')?; angle.to_css(dest) - } + }, } } } @@ -732,7 +732,7 @@ where z.to_css(dest)?; } Ok(()) - } + }, } } } diff --git a/components/style/values/specified/angle.rs b/components/style/values/specified/angle.rs index b616ed9c600..458f4178e59 100644 --- a/components/style/values/specified/angle.rs +++ b/components/style/values/specified/angle.rs @@ -218,7 +218,7 @@ impl Angle { Err(()) => { let t = t.clone(); Err(input.new_unexpected_token_error(t)) - } + }, } }, Token::Number { value, .. } if value == 0. => match allow_unitless_zero { @@ -234,7 +234,7 @@ impl Angle { ref t => { let t = t.clone(); Err(input.new_unexpected_token_error(t)) - } + }, } } } diff --git a/components/style/values/specified/box.rs b/components/style/values/specified/box.rs index 2173c8f5f2a..7fd5a250e06 100644 --- a/components/style/values/specified/box.rs +++ b/components/style/values/specified/box.rs @@ -1153,7 +1153,7 @@ fn change_bits_for_longhand(longhand: LonghandId) -> WillChangeBits { #[cfg(feature = "gecko")] LonghandId::Translate | LonghandId::Rotate | LonghandId::Scale | LonghandId::OffsetPath => { WillChangeBits::TRANSFORM - } + }, _ => WillChangeBits::empty(), }; diff --git a/components/style/values/specified/calc.rs b/components/style/values/specified/calc.rs index 87f88039506..f9feb616fff 100644 --- a/components/style/values/specified/calc.rs +++ b/components/style/values/specified/calc.rs @@ -176,13 +176,9 @@ impl CalcNode { value, ref unit, .. }, CalcUnit::LengthPercentage, - ) => { - NoCalcLength::parse_dimension(context, value, unit) - .map(CalcNode::Length) - .map_err(|()| { - location.new_custom_error(StyleParseErrorKind::UnspecifiedError) - }) - }, + ) => NoCalcLength::parse_dimension(context, value, unit) + .map(CalcNode::Length) + .map_err(|()| location.new_custom_error(StyleParseErrorKind::UnspecifiedError)), ( &Token::Dimension { value, ref unit, .. @@ -191,9 +187,7 @@ impl CalcNode { ) => { Angle::parse_dimension(value, unit, /* from_calc = */ true) .map(CalcNode::Angle) - .map_err(|()| { - location.new_custom_error(StyleParseErrorKind::UnspecifiedError) - }) + .map_err(|()| location.new_custom_error(StyleParseErrorKind::UnspecifiedError)) }, ( &Token::Dimension { @@ -203,9 +197,7 @@ impl CalcNode { ) => { Time::parse_dimension(value, unit, /* from_calc = */ true) .map(CalcNode::Time) - .map_err(|()| { - location.new_custom_error(StyleParseErrorKind::UnspecifiedError) - }) + .map_err(|()| location.new_custom_error(StyleParseErrorKind::UnspecifiedError)) }, (&Token::Percentage { unit_value, .. }, CalcUnit::LengthPercentage) | (&Token::Percentage { unit_value, .. }, CalcUnit::Percentage) => { @@ -252,7 +244,7 @@ impl CalcNode { ref t => { let t = t.clone(); return Err(input.new_unexpected_token_error(t)); - } + }, } }, _ => { diff --git a/components/style/values/specified/color.rs b/components/style/values/specified/color.rs index ddbdb5f4cdf..4e761f95338 100644 --- a/components/style/values/specified/color.rs +++ b/components/style/values/specified/color.rs @@ -260,13 +260,9 @@ impl SystemColor { SystemColor::MozVisitedhyperlinktext => prefs.mVisitedLinkColor, _ => unsafe { - bindings::Gecko_GetLookAndFeelSystemColor( - *self as i32, - cx.device().document() - ) - } + bindings::Gecko_GetLookAndFeelSystemColor(*self as i32, cx.device().document()) + }, }) - } } diff --git a/components/style/values/specified/counters.rs b/components/style/values/specified/counters.rs index 69a05da792b..09020e73448 100644 --- a/components/style/values/specified/counters.rs +++ b/components/style/values/specified/counters.rs @@ -203,7 +203,7 @@ impl Parse for Content { Ok(t) => { let t = t.clone(); return Err(input.new_unexpected_token_error(t)); - } + }, } } if content.is_empty() { diff --git a/components/style/values/specified/font.rs b/components/style/values/specified/font.rs index a6a67bc37de..c09d5a2c34c 100644 --- a/components/style/values/specified/font.rs +++ b/components/style/values/specified/font.rs @@ -1212,69 +1212,67 @@ impl Parse for FontVariantAlternates { parsed_alternates |= $flag; ) ); - while let Ok(_) = input.try(|input| { - match *input.next()? { - Token::Ident(ref value) if value.eq_ignore_ascii_case("historical-forms") => { - check_if_parsed!(input, VariantAlternatesParsingFlags::HISTORICAL_FORMS); - alternates.push(VariantAlternates::HistoricalForms); - Ok(()) - }, - Token::Function(ref name) => { - let name = name.clone(); - input.parse_nested_block(|i| { - match_ignore_ascii_case! { &name, - "swash" => { - check_if_parsed!(i, VariantAlternatesParsingFlags::SWASH); + while let Ok(_) = input.try(|input| match *input.next()? { + Token::Ident(ref value) if value.eq_ignore_ascii_case("historical-forms") => { + check_if_parsed!(input, VariantAlternatesParsingFlags::HISTORICAL_FORMS); + alternates.push(VariantAlternates::HistoricalForms); + Ok(()) + }, + Token::Function(ref name) => { + let name = name.clone(); + input.parse_nested_block(|i| { + match_ignore_ascii_case! { &name, + "swash" => { + check_if_parsed!(i, VariantAlternatesParsingFlags::SWASH); + let location = i.current_source_location(); + let ident = CustomIdent::from_ident(location, i.expect_ident()?, &[])?; + alternates.push(VariantAlternates::Swash(ident)); + Ok(()) + }, + "stylistic" => { + check_if_parsed!(i, VariantAlternatesParsingFlags::STYLISTIC); + let location = i.current_source_location(); + let ident = CustomIdent::from_ident(location, i.expect_ident()?, &[])?; + alternates.push(VariantAlternates::Stylistic(ident)); + Ok(()) + }, + "ornaments" => { + check_if_parsed!(i, VariantAlternatesParsingFlags::ORNAMENTS); + let location = i.current_source_location(); + let ident = CustomIdent::from_ident(location, i.expect_ident()?, &[])?; + alternates.push(VariantAlternates::Ornaments(ident)); + Ok(()) + }, + "annotation" => { + check_if_parsed!(i, VariantAlternatesParsingFlags::ANNOTATION); + let location = i.current_source_location(); + let ident = CustomIdent::from_ident(location, i.expect_ident()?, &[])?; + alternates.push(VariantAlternates::Annotation(ident)); + Ok(()) + }, + "styleset" => { + check_if_parsed!(i, VariantAlternatesParsingFlags::STYLESET); + let idents = i.parse_comma_separated(|i| { let location = i.current_source_location(); - let ident = CustomIdent::from_ident(location, i.expect_ident()?, &[])?; - alternates.push(VariantAlternates::Swash(ident)); - Ok(()) - }, - "stylistic" => { - check_if_parsed!(i, VariantAlternatesParsingFlags::STYLISTIC); + CustomIdent::from_ident(location, i.expect_ident()?, &[]) + })?; + alternates.push(VariantAlternates::Styleset(idents.into())); + Ok(()) + }, + "character-variant" => { + check_if_parsed!(i, VariantAlternatesParsingFlags::CHARACTER_VARIANT); + let idents = i.parse_comma_separated(|i| { let location = i.current_source_location(); - let ident = CustomIdent::from_ident(location, i.expect_ident()?, &[])?; - alternates.push(VariantAlternates::Stylistic(ident)); - Ok(()) - }, - "ornaments" => { - check_if_parsed!(i, VariantAlternatesParsingFlags::ORNAMENTS); - let location = i.current_source_location(); - let ident = CustomIdent::from_ident(location, i.expect_ident()?, &[])?; - alternates.push(VariantAlternates::Ornaments(ident)); - Ok(()) - }, - "annotation" => { - check_if_parsed!(i, VariantAlternatesParsingFlags::ANNOTATION); - let location = i.current_source_location(); - let ident = CustomIdent::from_ident(location, i.expect_ident()?, &[])?; - alternates.push(VariantAlternates::Annotation(ident)); - Ok(()) - }, - "styleset" => { - check_if_parsed!(i, VariantAlternatesParsingFlags::STYLESET); - let idents = i.parse_comma_separated(|i| { - let location = i.current_source_location(); - CustomIdent::from_ident(location, i.expect_ident()?, &[]) - })?; - alternates.push(VariantAlternates::Styleset(idents.into())); - Ok(()) - }, - "character-variant" => { - check_if_parsed!(i, VariantAlternatesParsingFlags::CHARACTER_VARIANT); - let idents = i.parse_comma_separated(|i| { - let location = i.current_source_location(); - CustomIdent::from_ident(location, i.expect_ident()?, &[]) - })?; - alternates.push(VariantAlternates::CharacterVariant(idents.into())); - Ok(()) - }, - _ => return Err(i.new_custom_error(StyleParseErrorKind::UnspecifiedError)), - } - }) - }, - _ => Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)), - } + CustomIdent::from_ident(location, i.expect_ident()?, &[]) + })?; + alternates.push(VariantAlternates::CharacterVariant(idents.into())); + Ok(()) + }, + _ => return Err(i.new_custom_error(StyleParseErrorKind::UnspecifiedError)), + } + }) + }, + _ => Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)), }) {} if parsed_alternates.is_empty() { diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index b553c7bd33e..ebf7746ee11 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -602,20 +602,17 @@ impl Length { !context.parsing_mode.allows_unitless_lengths() && !allow_quirks.allowed(context.quirks_mode) { - return Err( - location.new_custom_error(StyleParseErrorKind::UnspecifiedError) - ); + return Err(location.new_custom_error(StyleParseErrorKind::UnspecifiedError)); } Ok(Length::NoCalc(NoCalcLength::Absolute(AbsoluteLength::Px( value, )))) }, - Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => { - input.parse_nested_block(|input| { + Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => input + .parse_nested_block(|input| { CalcNode::parse_length(context, input, num_context) .map(|calc| Length::Calc(Box::new(calc))) - }) - }, + }), ref token => return Err(location.new_unexpected_token_error(token.clone())), } } diff --git a/components/style/values/specified/percentage.rs b/components/style/values/specified/percentage.rs index ad596dd1404..50ebbb3bcf6 100644 --- a/components/style/values/specified/percentage.rs +++ b/components/style/values/specified/percentage.rs @@ -116,9 +116,10 @@ impl Percentage { if num_context.is_ok(context.parsing_mode, unit_value) => { Ok(Percentage::new(unit_value)) - } + }, Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => { - let result = input.parse_nested_block(|i| CalcNode::parse_percentage(context, i))?; + let result = + input.parse_nested_block(|i| CalcNode::parse_percentage(context, i))?; // TODO(emilio): -moz-image-rect is the only thing that uses // the clamping mode... I guess we could disallow it... diff --git a/components/style/values/specified/time.rs b/components/style/values/specified/time.rs index 09439c84d0c..0006ec45923 100644 --- a/components/style/values/specified/time.rs +++ b/components/style/values/specified/time.rs @@ -92,9 +92,8 @@ impl Time { Token::Dimension { value, ref unit, .. } if clamping_mode.is_ok(ParsingMode::DEFAULT, value) => { - Time::parse_dimension(value, unit, /* from_calc = */ false).map_err(|()| { - location.new_custom_error(StyleParseErrorKind::UnspecifiedError) - }) + Time::parse_dimension(value, unit, /* from_calc = */ false) + .map_err(|()| location.new_custom_error(StyleParseErrorKind::UnspecifiedError)) }, Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => { match input.parse_nested_block(|i| CalcNode::parse_time(context, i)) { diff --git a/components/style/values/specified/transform.rs b/components/style/values/specified/transform.rs index 44f1afe83d1..a61f982cd2d 100644 --- a/components/style/values/specified/transform.rs +++ b/components/style/values/specified/transform.rs @@ -12,7 +12,9 @@ use crate::values::generics::transform::{Matrix, Matrix3D}; use crate::values::specified::position::{ HorizontalPositionKeyword, Side, VerticalPositionKeyword, }; -use crate::values::specified::{self, Angle, Integer, Length, LengthPercentage, Number, NumberOrPercentage}; +use crate::values::specified::{ + self, Angle, Integer, Length, LengthPercentage, Number, NumberOrPercentage, +}; use crate::Zero; use cssparser::Parser; use style_traits::{ParseError, StyleParseErrorKind}; From 85da1dda294970d6f96d3e16ec05698efbcd0b47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 30 Nov 2019 14:42:36 +0100 Subject: [PATCH 43/45] Remove no-longer-needed arrayvec feature. --- Cargo.lock | 8 ++++---- components/style/Cargo.toml | 5 +++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f6daa13a696..531fa815666 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5333,7 +5333,7 @@ name = "style" version = "0.0.1" dependencies = [ "app_units", - "arrayvec 0.4.6", + "arrayvec 0.5.1", "atomic_refcell", "bindgen", "bitflags", @@ -5941,11 +5941,11 @@ checksum = "fd2be2d6639d0f8fe6cdda291ad456e23629558d466e2789d2c3e9892bda285d" [[package]] name = "uluru" -version = "0.3.0" +version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d2606e9192f308ddc4f0b3c5d1bf3400e28a70fff956e9d9f46d23b094746d9f" +checksum = "6d7b39d0c32eba57d52d334e4bdd150df6e755264eefaa1ae2e7cd125f35e1ca" dependencies = [ - "arrayvec 0.4.6", + "arrayvec 0.5.1", ] [[package]] diff --git a/components/style/Cargo.toml b/components/style/Cargo.toml index 4489bece8ad..47a0d4392ef 100644 --- a/components/style/Cargo.toml +++ b/components/style/Cargo.toml @@ -18,8 +18,9 @@ doctest = false [features] gecko = ["style_traits/gecko", "fallible/known_system_malloc", "bindgen", "regex", "toml"] servo = ["serde", "style_traits/servo", "servo_atoms", "servo_config", "html5ever", - "cssparser/serde", "encoding_rs", "malloc_size_of/servo", "arrayvec/use_union", - "servo_url", "string_cache", "crossbeam-channel", "to_shmem/servo", "servo_arc/servo"] + "cssparser/serde", "encoding_rs", "malloc_size_of/servo", "servo_url", + "string_cache", "crossbeam-channel", "to_shmem/servo", + "servo_arc/servo"] servo-layout-2013 = [] servo-layout-2020 = [] gecko_debug = [] From 226c9807dfb657b33569662d25bad1aad0667ec0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 30 Nov 2019 15:02:48 +0100 Subject: [PATCH 44/45] style: Fix servo build. --- components/layout_thread/dom_wrapper.rs | 22 ++++++++++++++++++++ components/layout_thread_2020/dom_wrapper.rs | 22 ++++++++++++++++++++ components/script/dom/element.rs | 8 +++++++ components/style/servo/selector_parser.rs | 8 +++++++ 4 files changed, 60 insertions(+) diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index 5d815517532..466d844418a 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -514,6 +514,10 @@ impl<'le> TElement for ServoLayoutElement<'le> { false } + fn exports_any_part(&self) -> bool { + false + } + fn style_attribute(&self) -> Option>> { unsafe { (*self.element.style_attribute()) @@ -999,6 +1003,14 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { false } + fn exported_part(&self, _: &Atom) -> Option { + None + } + + fn imported_part(&self, _: &Atom) -> Option { + None + } + #[inline] fn has_class(&self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool { unsafe { self.element.has_class_for_layout(name, case_sensitivity) } @@ -1533,6 +1545,16 @@ impl<'le> ::selectors::Element for ServoThreadSafeLayoutElement<'le> { false } + fn exported_part(&self, _: &Atom) -> Option { + debug!("ServoThreadSafeLayoutElement::exported_part called"); + None + } + + fn imported_part(&self, _: &Atom) -> Option { + debug!("ServoThreadSafeLayoutElement::imported_part called"); + None + } + fn has_class(&self, _name: &Atom, _case_sensitivity: CaseSensitivity) -> bool { debug!("ServoThreadSafeLayoutElement::has_class called"); false diff --git a/components/layout_thread_2020/dom_wrapper.rs b/components/layout_thread_2020/dom_wrapper.rs index a9f09e11073..520014837db 100644 --- a/components/layout_thread_2020/dom_wrapper.rs +++ b/components/layout_thread_2020/dom_wrapper.rs @@ -521,6 +521,10 @@ impl<'le> TElement for ServoLayoutElement<'le> { false } + fn exports_any_part(&self) -> bool { + false + } + fn style_attribute(&self) -> Option>> { unsafe { (*self.element.style_attribute()) @@ -1006,6 +1010,14 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { false } + fn exported_part(&self, _: &Atom) -> Option { + None + } + + fn imported_part(&self, _: &Atom) -> Option { + None + } + #[inline] fn has_class(&self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool { unsafe { self.element.has_class_for_layout(name, case_sensitivity) } @@ -1540,6 +1552,16 @@ impl<'le> ::selectors::Element for ServoThreadSafeLayoutElement<'le> { false } + fn exported_part(&self, _: &Atom) -> Option { + debug!("ServoThreadSafeLayoutElement::exported_part called"); + None + } + + fn imported_part(&self, _: &Atom) -> Option { + debug!("ServoThreadSafeLayoutElement::imported_part called"); + None + } + 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 baf4fcd0878..f359c43a18f 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -3073,6 +3073,14 @@ impl<'a> SelectorsElement for DomRoot { false } + fn exported_part(&self, _: &Atom) -> Option { + None + } + + fn imported_part(&self, _: &Atom) -> Option { + None + } + fn has_class(&self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool { Element::has_class(&**self, name, case_sensitivity) } diff --git a/components/style/servo/selector_parser.rs b/components/style/servo/selector_parser.rs index 72c47e65dda..977a6c59a85 100644 --- a/components/style/servo/selector_parser.rs +++ b/components/style/servo/selector_parser.rs @@ -700,6 +700,14 @@ impl ElementSnapshot for ServoElementSnapshot { false } + fn exported_part(&self, _: &Atom) -> Option { + None + } + + fn imported_part(&self, _: &Atom) -> Option { + None + } + fn has_class(&self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool { self.get_attr(&ns!(), &local_name!("class")) .map_or(false, |v| { From 6712e316dfdb0ef783c9f52bd336b027c187f879 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 30 Nov 2019 20:45:53 +0100 Subject: [PATCH 45/45] style: Update wpt expectations. transform-scale-percent-001.html has been updated upstream, as the test is invalid per CSSWG resolution. --- .../animation/rotate-interpolation.html.ini | 6 -- .../animation/scale-interpolation.html.ini | 78 ------------------- .../translate-interpolation.html.ini | 48 ------------ .../parsing/scale-parsing-valid.html.ini | 7 -- .../parsing/translate-parsing-valid.html.ini | 9 --- .../transform-scale-percent-001.html.ini | 2 + 6 files changed, 2 insertions(+), 148 deletions(-) delete mode 100644 tests/wpt/metadata/css/css-transforms/parsing/scale-parsing-valid.html.ini create mode 100644 tests/wpt/metadata/css/css-transforms/transform-scale-percent-001.html.ini diff --git a/tests/wpt/metadata/css/css-transforms/animation/rotate-interpolation.html.ini b/tests/wpt/metadata/css/css-transforms/animation/rotate-interpolation.html.ini index e80c4cdd294..eb5fe0b2b88 100644 --- a/tests/wpt/metadata/css/css-transforms/animation/rotate-interpolation.html.ini +++ b/tests/wpt/metadata/css/css-transforms/animation/rotate-interpolation.html.ini @@ -95,9 +95,6 @@ [CSS Transitions: property from [1 0 0 0deg\] to [0 1 0 10deg\] at (-1) should be [0 1 0 -10deg\]] expected: FAIL - [CSS Transitions with transition: all: property from [none\] to [7 -8 9 400grad\] at (1) should be [0.5 -0.57 0.65 400grad\]] - expected: FAIL - [CSS Animations: property from [0 1 0 100deg\] to [0 1 0 -100deg\] at (0.25) should be [0 1 0 50deg\]] expected: FAIL @@ -821,9 +818,6 @@ [Web Animations: property from [100deg\] to [-100deg\] at (0) should be [100deg\]] expected: FAIL - [CSS Transitions: property from [none\] to [7 -8 9 400grad\] at (1) should be [0.5 -0.57 0.65 400grad\]] - expected: FAIL - [CSS Animations: property from [1 0 0 0deg\] to [0 1 0 10deg\] at (1) should be [0 1 0 10deg\]] expected: FAIL diff --git a/tests/wpt/metadata/css/css-transforms/animation/scale-interpolation.html.ini b/tests/wpt/metadata/css/css-transforms/animation/scale-interpolation.html.ini index bae37e59cb6..d25ccb121c8 100644 --- a/tests/wpt/metadata/css/css-transforms/animation/scale-interpolation.html.ini +++ b/tests/wpt/metadata/css/css-transforms/animation/scale-interpolation.html.ini @@ -98,9 +98,6 @@ [CSS Transitions: property from [2 30 400\] to [10 110 1200\] at (2) should be [18 190 2000\]] expected: FAIL - [CSS Transitions with transition: all: property from [2 0.5 1\] to [inherit\] at (0) should be [2 0.5\]] - expected: FAIL - [CSS Transitions with transition: all: property from [none\] to [4 3 2\] at (2) should be [7 5 3\]] expected: FAIL @@ -251,9 +248,6 @@ [Web Animations: property from [-10 5\] to [10 -5\] at (1) should be [10 -5\]] expected: FAIL - [CSS Transitions: property from [inherit\] to [2 0.5 1\] at (1) should be [2 0.5\]] - expected: FAIL - [CSS Transitions: property from [unset\] to [1.5 1\] at (2) should be [2 1\]] expected: FAIL @@ -272,9 +266,6 @@ [CSS Transitions with transition: all: property from [initial\] to [2 0.5 1\] at (2) should be [3 0\]] expected: FAIL - [CSS Transitions with transition: all: property from [-10 5 1\] to [1\] at (1) should be [1\]] - expected: FAIL - [CSS Transitions: property from [-10 5\] to [10 -5\] at (0.75) should be [5 -2.5\]] expected: FAIL @@ -311,9 +302,6 @@ [Web Animations: property from [2 0.5 1\] to [inherit\] at (2) should be [-1 1.5 3\]] expected: FAIL - [CSS Transitions with transition: all: property from [initial\] to [inherit\] at (0) should be [1\]] - expected: FAIL - [CSS Animations: property from [none\] to [none\] at (0.125) should be [none\]] expected: FAIL @@ -347,9 +335,6 @@ [CSS Transitions with transition: all: property from neutral to [1.5 1\] at (2) should be [1.9 1\]] expected: FAIL - [CSS Transitions with transition: all: property from [none\] to [4 3 2\] at (0) should be [1\]] - expected: FAIL - [CSS Animations: property from [initial\] to [inherit\] at (1) should be [0.5 1 2\]] expected: FAIL @@ -359,27 +344,15 @@ [Web Animations: property from [2 30 400\] to [10 110 1200\] at (-1) should be [-6 -50 -400\]] expected: FAIL - [CSS Transitions: property from [26 17 9\] to [2 1\] at (1) should be [2 1\]] - expected: FAIL - [CSS Transitions: property from [inherit\] to [2 0.5 1\] at (0.25) should be [0.875 0.875 1.75\]] expected: FAIL - [CSS Transitions with transition: all: property from [-10 5 1\] to [1\] at (0) should be [-10 5\]] - expected: FAIL - [CSS Animations: property from neutral to [1.5 1\] at (0.25) should be [1.2 1\]] expected: FAIL - [CSS Transitions: property from [-10 5 1\] to [1\] at (1) should be [1\]] - expected: FAIL - [CSS Transitions with transition: all: property from [1\] to [10 -5 0\] at (0.25) should be [3.25 -0.5 0.75\]] expected: FAIL - [CSS Transitions: property from [2 0.5 1\] to [initial\] at (0) should be [2 0.5\]] - expected: FAIL - [CSS Transitions: property from [none\] to [4 3 2\] at (0.875) should be [3.625 2.75 1.875\]] expected: FAIL @@ -479,9 +452,6 @@ [Web Animations: property from [2 0.5 1\] to [inherit\] at (-1) should be [3.5 0 0\]] expected: FAIL - [CSS Transitions: property from [initial\] to [2 0.5 1\] at (1) should be [2 0.5\]] - expected: FAIL - [Web Animations: property from [-10 5 1\] to [1\] at (0.75) should be [-1.75 2\]] expected: FAIL @@ -515,9 +485,6 @@ [CSS Transitions with transition: all: property from [none\] to [4 3 2\] at (0.125) should be [1.375 1.25 1.125\]] expected: FAIL - [CSS Transitions: property from [2 0.5 1\] to [initial\] at (1) should be [1\]] - expected: FAIL - [CSS Transitions with transition: all: property from [initial\] to [inherit\] at (0.75) should be [0.625 1 1.75\]] expected: FAIL @@ -533,9 +500,6 @@ [CSS Transitions with transition: all: property from [26 17 9\] to [2 1\] at (-1) should be [50 33 17\]] expected: FAIL - [CSS Transitions: property from [1\] to [10 -5 0\] at (0) should be [1\]] - expected: FAIL - [Web Animations: property from [unset\] to [1.5 1\] at (0.75) should be [1.375 1\]] expected: FAIL @@ -581,15 +545,6 @@ [Web Animations: property from [1\] to [10 -5 0\] at (2) should be [19 -11 -1\]] expected: FAIL - [CSS Transitions with transition: all: property from [inherit\] to [initial\] at (1) should be [1\]] - expected: FAIL - - [CSS Transitions with transition: all: property from [2 0.5 1\] to [initial\] at (1) should be [1\]] - expected: FAIL - - [CSS Transitions with transition: all: property from [initial\] to [2 0.5 1\] at (0) should be [1\]] - expected: FAIL - [Web Animations: property from [inherit\] to [initial\] at (1) should be [1\]] expected: FAIL @@ -605,9 +560,6 @@ [CSS Animations: property from [2 0.5 1\] to [inherit\] at (-1) should be [3.5 0 0\]] expected: FAIL - [CSS Transitions with transition: all: property from [initial\] to [2 0.5 1\] at (1) should be [2 0.5\]] - expected: FAIL - [CSS Animations: property from neutral to [1.5 1\] at (0.75) should be [1.4 1\]] expected: FAIL @@ -629,9 +581,6 @@ [CSS Transitions: property from [2 0.5 1\] to [initial\] at (-1) should be [3 0\]] expected: FAIL - [CSS Transitions: property from [initial\] to [inherit\] at (0) should be [1\]] - expected: FAIL - [Web Animations: property from [inherit\] to [initial\] at (0) should be [0.5 1 2\]] expected: FAIL @@ -698,9 +647,6 @@ [Web Animations: property from [none\] to [4 3 2\] at (0) should be [1\]] expected: FAIL - [CSS Transitions: property from [-10 5 1\] to [1\] at (0) should be [-10 5\]] - expected: FAIL - [Web Animations: property from [2 0.5 1\] to [inherit\] at (0) should be [2 0.5\]] expected: FAIL @@ -752,9 +698,6 @@ [Web Animations: property from [unset\] to [1.5 1\] at (1) should be [1.5 1\]] expected: FAIL - [CSS Transitions: property from [inherit\] to [initial\] at (1) should be [1\]] - expected: FAIL - [Web Animations: property from [2 0.5 1\] to [inherit\] at (0.25) should be [1.625 0.625 1.25\]] expected: FAIL @@ -791,9 +734,6 @@ [CSS Animations: property from [initial\] to [inherit\] at (0.75) should be [0.625 1 1.75\]] expected: FAIL - [CSS Transitions with transition: all: property from [1\] to [10 -5 0\] at (0) should be [1\]] - expected: FAIL - [CSS Transitions: property from [26 17 9\] to [2 1\] at (2) should be [-22 -15 -7\]] expected: FAIL @@ -809,9 +749,6 @@ [CSS Transitions with transition: all: property from [initial\] to [inherit\] at (0.25) should be [0.875 1 1.25\]] expected: FAIL - [CSS Transitions: property from [none\] to [4 3 2\] at (0) should be [1\]] - expected: FAIL - [CSS Animations: property from [unset\] to [1.5 1\] at (2) should be [2 1\]] expected: FAIL @@ -827,9 +764,6 @@ [CSS Transitions with transition: all: property from [2 0.5 1\] to [inherit\] at (-1) should be [3.5 0 0\]] expected: FAIL - [CSS Transitions with transition: all: property from [26 17 9\] to [2 1\] at (1) should be [2 1\]] - expected: FAIL - [CSS Animations: property from [initial\] to [inherit\] at (0.25) should be [0.875 1 1.25\]] expected: FAIL @@ -908,9 +842,6 @@ [CSS Animations: property from neutral to [1.5 1\] at (1) should be [1.5 1\]] expected: FAIL - [CSS Transitions: property from [2 0.5 1\] to [inherit\] at (0) should be [2 0.5\]] - expected: FAIL - [CSS Transitions with transition: all: property from [2 0.5 1\] to [initial\] at (0.25) should be [1.75 0.6251\]] expected: FAIL @@ -944,9 +875,6 @@ [CSS Transitions with transition: all: property from neutral to [1.5 1\] at (0.25) should be [1.2 1\]] expected: FAIL - [CSS Transitions with transition: all: property from [inherit\] to [2 0.5 1\] at (1) should be [2 0.5\]] - expected: FAIL - [CSS Transitions: property from [inherit\] to [2 0.5 1\] at (0.75) should be [1.625 0.625 1.25\]] expected: FAIL @@ -977,9 +905,6 @@ [CSS Transitions with transition: all: property from [initial\] to [inherit\] at (-1) should be [1.5 1 0\]] expected: FAIL - [CSS Transitions with transition: all: property from [2 0.5 1\] to [initial\] at (0) should be [2 0.5\]] - expected: FAIL - [CSS Animations: property from neutral to [1.5 1\] at (-1) should be [0.7 1\]] expected: FAIL @@ -1001,9 +926,6 @@ [CSS Transitions: property from [1\] to [10 -5 0\] at (0.75) should be [7.75 -3.5 0.25\]] expected: FAIL - [CSS Transitions: property from [initial\] to [2 0.5 1\] at (0) should be [1\]] - expected: FAIL - [CSS Animations: property from [2 30 400\] to [10 110 1200\] at (2) should be [18 190 2000\]] expected: FAIL diff --git a/tests/wpt/metadata/css/css-transforms/animation/translate-interpolation.html.ini b/tests/wpt/metadata/css/css-transforms/animation/translate-interpolation.html.ini index 8f6ece60c55..8987874f01c 100644 --- a/tests/wpt/metadata/css/css-transforms/animation/translate-interpolation.html.ini +++ b/tests/wpt/metadata/css/css-transforms/animation/translate-interpolation.html.ini @@ -74,15 +74,6 @@ [translate interpolation] expected: FAIL - [CSS Transitions: property from [480px 400px 320px\] to [240% 160%\] at (1) should be [240% 160%\]] - expected: FAIL - - [CSS Transitions with transition: all: property from [200px 100px 400px\] to [initial\] at (1) should be [0px\]] - expected: FAIL - - [CSS Transitions: property from [none\] to [8px 80% 800px\] at (0) should be [0px\]] - expected: FAIL - [CSS Transitions with transition: all: property from [-100px -50px\] to [100px 50px\] at (2) should be [300px 150px\]] expected: FAIL @@ -125,9 +116,6 @@ [CSS Transitions: property from [200px 100px 400px\] to [initial\] at (-1) should be [400px 200px 800px\]] expected: FAIL - [CSS Transitions with transition: all: property from [-100px -50px 100px\] to [0px\] at (1) should be [0px\]] - expected: FAIL - [CSS Animations: property from [200px 100px 200px\] to [inherit\] at (2) should be [0px 300px 400px\]] expected: FAIL @@ -161,9 +149,6 @@ [CSS Transitions with transition: all: property from [unset\] to [20px\] at (0.75) should be [15px\]] expected: FAIL - [CSS Transitions with transition: all: property from [480px 400px 320px\] to [240% 160%\] at (1) should be [240% 160%\]] - expected: FAIL - [CSS Animations: property from [200px 100px 400px\] to [initial\] at (0) should be [200px 100px 400px\]] expected: FAIL @@ -173,15 +158,9 @@ [Web Animations: property from [inherit\] to [initial\] at (0) should be [100px 200px 300px\]] expected: FAIL - [CSS Transitions: property from [0px\] to [-100px -50px 100px\] at (0) should be [0px\]] - expected: FAIL - [CSS Animations: property from [-100%\] to [100%\] at (0) should be [-100%\]] expected: FAIL - [CSS Transitions: property from [inherit\] to [initial\] at (1) should be [0px\]] - expected: FAIL - [Web Animations: property from [-100px -50px\] to [100px 50px\] at (0.25) should be [-50px -25px\]] expected: FAIL @@ -245,9 +224,6 @@ [CSS Animations: property from [220px 240px 260px\] to [300px 400px 500px\] at (0.125) should be [230px 260px 290px\]] expected: FAIL - [CSS Transitions with transition: all: property from [initial\] to [200px 100px 200px\] at (0) should be [0px\]] - expected: FAIL - [Web Animations: property from [-100px\] to [100px\] at (-1) should be [-300px\]] expected: FAIL @@ -341,9 +317,6 @@ [CSS Transitions: property from [200px 100px 200px\] to [inherit\] at (2) should be [0px 300px 400px\]] expected: FAIL - [CSS Transitions with transition: all: property from [initial\] to [inherit\] at (0) should be [0px\]] - expected: FAIL - [Web Animations: property from [none\] to [8px 80% 800px\] at (1) should be [8px 80% 800px\]] expected: FAIL @@ -437,9 +410,6 @@ [Web Animations: property from [-100px -50px 100px\] to [0px\] at (-1) should be [-200px -100px 200px\]] expected: FAIL - [CSS Transitions with transition: all: property from [0px\] to [-100px -50px 100px\] at (0) should be [0px\]] - expected: FAIL - [CSS Transitions with transition: all: property from [-100px -50px\] to [100px 50px\] at (0.75) should be [50px 25px\]] expected: FAIL @@ -527,9 +497,6 @@ [CSS Animations: property from neutral to [20px\] at (2) should be [30px\]] expected: FAIL - [CSS Transitions: property from [-100px -50px 100px\] to [0px\] at (1) should be [0px\]] - expected: FAIL - [CSS Transitions: property from [-100px -50px\] to [100px 50px\] at (2) should be [300px 150px\]] expected: FAIL @@ -635,9 +602,6 @@ [Web Animations: property from [inherit\] to [initial\] at (-1) should be [200px 400px 600px\]] expected: FAIL - [CSS Transitions with transition: all: property from [none\] to [8px 80% 800px\] at (0) should be [0px\]] - expected: FAIL - [CSS Animations: property from [0px\] to [-100px -50px 100px\] at (0.75) should be [-75px -37.5px 75px\]] expected: FAIL @@ -764,9 +728,6 @@ [Web Animations: property from [480px 400px 320px\] to [240% 160%\] at (0.875) should be [calc(210% + 60px) calc(140% + 50px) 40px\]] expected: FAIL - [CSS Transitions with transition: all: property from [inherit\] to [initial\] at (1) should be [0px\]] - expected: FAIL - [CSS Transitions: property from [480px 400px 320px\] to [240% 160%\] at (0.125) should be [calc(420px + 30%) calc(350px + 20%) 280px\]] expected: FAIL @@ -788,9 +749,6 @@ [CSS Animations: property from [200px 100px 200px\] to [inherit\] at (0.75) should be [125px 175px 275px\]] expected: FAIL - [CSS Transitions: property from [initial\] to [200px 100px 200px\] at (0) should be [0px\]] - expected: FAIL - [CSS Animations: property from [none\] to [8px 80% 800px\] at (0.125) should be [1px 10% 100px\]] expected: FAIL @@ -1037,9 +995,6 @@ [CSS Transitions with transition: all: property from [none\] to [8px 80% 800px\] at (2) should be [16px 160% 1600px\]] expected: FAIL - [CSS Transitions: property from [initial\] to [inherit\] at (0) should be [0px\]] - expected: FAIL - [CSS Animations: property from neutral to [20px\] at (0.75) should be [17.5px\]] expected: FAIL @@ -1064,9 +1019,6 @@ [Web Animations: property from [220px 240px 260px\] to [300px 400px 500px\] at (1) should be [300px 400px 500px\]] expected: FAIL - [CSS Transitions: property from [200px 100px 400px\] to [initial\] at (1) should be [0px\]] - expected: FAIL - [CSS Animations: property from [200px 100px 400px\] to [initial\] at (0.25) should be [150px 75px 300px\]] expected: FAIL diff --git a/tests/wpt/metadata/css/css-transforms/parsing/scale-parsing-valid.html.ini b/tests/wpt/metadata/css/css-transforms/parsing/scale-parsing-valid.html.ini deleted file mode 100644 index 14663c5a719..00000000000 --- a/tests/wpt/metadata/css/css-transforms/parsing/scale-parsing-valid.html.ini +++ /dev/null @@ -1,7 +0,0 @@ -[scale-parsing-valid.html] - [e.style['scale'\] = "100 200 1" should set the property value] - expected: FAIL - - [e.style['scale'\] = "100 100 1" should set the property value] - expected: FAIL - diff --git a/tests/wpt/metadata/css/css-transforms/parsing/translate-parsing-valid.html.ini b/tests/wpt/metadata/css/css-transforms/parsing/translate-parsing-valid.html.ini index 9bfdefbcc40..2e5706ee01d 100644 --- a/tests/wpt/metadata/css/css-transforms/parsing/translate-parsing-valid.html.ini +++ b/tests/wpt/metadata/css/css-transforms/parsing/translate-parsing-valid.html.ini @@ -8,12 +8,3 @@ [e.style['translate'\] = "100px calc(10px - 10%)" should set the property value] expected: FAIL - [e.style['translate'\] = "100px 200px 0px" should set the property value] - expected: FAIL - - [e.style['translate'\] = "100px 0px 0px" should set the property value] - expected: FAIL - - [e.style['translate'\] = "1px 2px 0" should set the property value] - expected: FAIL - diff --git a/tests/wpt/metadata/css/css-transforms/transform-scale-percent-001.html.ini b/tests/wpt/metadata/css/css-transforms/transform-scale-percent-001.html.ini new file mode 100644 index 00000000000..4ae92db68d0 --- /dev/null +++ b/tests/wpt/metadata/css/css-transforms/transform-scale-percent-001.html.ini @@ -0,0 +1,2 @@ +[transform-scale-percent-001.html] + expected: FAIL