Backed out changeset 67769dac78c4 for busting builds CLOSED TREE
authorWes Kocher <wkocher@mozilla.com>
Fri, 15 Sep 2017 13:38:03 -0700
changeset 665717 d6a3a83d9ce3c16773a5c818b5f754ff7728f665
parent 665716 a6d9e37a9122e0848b67c072b7a101fe4b058716
child 665718 15cadfb0d81411ec285e2d5232fb4dd90399682e
child 665749 f099658827ac421ca053630117d673a9e26346b6
child 665850 52e6fce3e6f625f08e0fc9b8b76222e8c7f8bcff
child 666326 3a520ced6fbf817faa9181b94ab1ee81017ec61d
child 667625 9cf658e9e0fd9d88cfa012a187706efebc4726f5
push id80148
push userbmo:emilio@crisal.io
push dateFri, 15 Sep 2017 20:59:22 +0000
milestone57.0a1
backs out67769dac78c4e757a95b41b1d8d517af61ea19f2
Backed out changeset 67769dac78c4 for busting builds CLOSED TREE MozReview-Commit-ID: 82QTBjYc0jC
servo/components/layout_thread/dom_wrapper.rs
servo/components/style/dom.rs
servo/components/style/gecko/generated/bindings.rs
servo/components/style/gecko/media_queries.rs
servo/components/style/gecko/wrapper.rs
servo/components/style/matching.rs
servo/components/style/servo/media_queries.rs
servo/components/style/values/specified/color.rs
--- a/servo/components/layout_thread/dom_wrapper.rs
+++ b/servo/components/layout_thread/dom_wrapper.rs
@@ -542,24 +542,16 @@ impl<'le> TElement for ServoLayoutElemen
         // so we can decide when to fall back to the Content-Language check.
         let element_lang = match override_lang {
             Some(Some(lang)) => lang,
             Some(None) => String::new(),
             None => self.element.get_lang_for_layout(),
         };
         extended_filtering(&element_lang, &*value)
     }
-
-    fn is_html_document_body_element(&self) -> bool {
-        // This is only used for the "tables inherit from body" quirk, which we
-        // don't implement.
-        //
-        // FIXME(emilio): We should be able to give the right answer though!
-        false
-    }
 }
 
 impl<'le> PartialEq for ServoLayoutElement<'le> {
     fn eq(&self, other: &Self) -> bool {
         self.as_node() == other.as_node()
     }
 }
 
--- a/servo/components/style/dom.rs
+++ b/servo/components/style/dom.rs
@@ -680,62 +680,55 @@ pub trait TElement : Eq + PartialEq + De
                                 -> HashMap<TransitionProperty, Arc<AnimationValue>>;
 
     /// Does a rough (and cheap) check for whether or not transitions might need to be updated that
     /// will quickly return false for the common case of no transitions specified or running. If
     /// this returns false, we definitely don't need to update transitions but if it returns true
     /// we can perform the more thoroughgoing check, needs_transitions_update, to further
     /// reduce the possibility of false positives.
     #[cfg(feature = "gecko")]
-    fn might_need_transitions_update(
-        &self,
-        old_values: Option<&ComputedValues>,
-        new_values: &ComputedValues
-    ) -> bool;
+    fn might_need_transitions_update(&self,
+                                     old_values: Option<&ComputedValues>,
+                                     new_values: &ComputedValues)
+                                     -> bool;
 
     /// Returns true if one of the transitions needs to be updated on this element. We check all
     /// the transition properties to make sure that updating transitions is necessary.
     /// This method should only be called if might_needs_transitions_update returns true when
     /// passed the same parameters.
     #[cfg(feature = "gecko")]
-    fn needs_transitions_update(
-        &self,
-        before_change_style: &ComputedValues,
-        after_change_style: &ComputedValues
-    ) -> bool;
+    fn needs_transitions_update(&self,
+                                before_change_style: &ComputedValues,
+                                after_change_style: &ComputedValues)
+                                -> bool;
 
     /// Returns true if we need to update transitions for the specified property on this element.
     #[cfg(feature = "gecko")]
-    fn needs_transitions_update_per_property(
-        &self,
-        property: &TransitionProperty,
-        combined_duration: f32,
-        before_change_style: &ComputedValues,
-        after_change_style: &ComputedValues,
-        existing_transitions: &HashMap<TransitionProperty, Arc<AnimationValue>>
-    ) -> bool;
+    fn needs_transitions_update_per_property(&self,
+                                             property: &TransitionProperty,
+                                             combined_duration: f32,
+                                             before_change_style: &ComputedValues,
+                                             after_change_style: &ComputedValues,
+                                             existing_transitions: &HashMap<TransitionProperty,
+                                                                            Arc<AnimationValue>>)
+                                             -> bool;
 
     /// Returns the value of the `xml:lang=""` attribute (or, if appropriate,
     /// the `lang=""` attribute) on this element.
     fn lang_attr(&self) -> Option<AttrValue>;
 
     /// Returns whether this element's language matches the language tag
     /// `value`.  If `override_lang` is not `None`, it specifies the value
     /// of the `xml:lang=""` or `lang=""` attribute to use in place of
     /// looking at the element and its ancestors.  (This argument is used
     /// to implement matching of `:lang()` against snapshots.)
-    fn match_element_lang(
-        &self,
-        override_lang: Option<Option<AttrValue>>,
-        value: &PseudoClassStringArg
-    ) -> bool;
-
-    /// Returns whether this element is the main body element of the HTML
-    /// document it is on.
-    fn is_html_document_body_element(&self) -> bool;
+    fn match_element_lang(&self,
+                          override_lang: Option<Option<AttrValue>>,
+                          value: &PseudoClassStringArg)
+                          -> bool;
 }
 
 /// TNode and TElement aren't Send because we want to be careful and explicit
 /// about our parallel traversal. However, there are certain situations
 /// (including but not limited to the traversal) where we need to send DOM
 /// objects to other threads.
 ///
 /// That's the reason why `SendNode` exists.
--- a/servo/components/style/gecko/generated/bindings.rs
+++ b/servo/components/style/gecko/generated/bindings.rs
@@ -1546,17 +1546,18 @@ extern "C" {
 extern "C" {
     pub fn Gecko_CSSCounterStyleRule_AddRef(aPtr: *mut nsCSSCounterStyleRule);
 }
 extern "C" {
     pub fn Gecko_CSSCounterStyleRule_Release(aPtr:
                                                  *mut nsCSSCounterStyleRule);
 }
 extern "C" {
-    pub fn Gecko_IsDocumentBody(element: RawGeckoElementBorrowed) -> bool;
+    pub fn Gecko_GetBody(pres_context: RawGeckoPresContextBorrowed)
+     -> RawGeckoElementBorrowedOrNull;
 }
 extern "C" {
     pub fn Gecko_GetLookAndFeelSystemColor(color_id: i32,
                                            pres_context:
                                                RawGeckoPresContextBorrowed)
      -> nscolor;
 }
 extern "C" {
--- a/servo/components/style/gecko/media_queries.rs
+++ b/servo/components/style/gecko/media_queries.rs
@@ -6,33 +6,33 @@
 
 use app_units::AU_PER_PX;
 use app_units::Au;
 use context::QuirksMode;
 use cssparser::{CssStringWriter, Parser, RGBA, Token, BasicParseError};
 use euclid::ScaleFactor;
 use euclid::Size2D;
 use font_metrics::get_metrics_provider_for_product;
-use gecko::values::{convert_nscolor_to_rgba, convert_rgba_to_nscolor};
+use gecko::values::convert_nscolor_to_rgba;
 use gecko_bindings::bindings;
 use gecko_bindings::structs;
 use gecko_bindings::structs::{nsCSSKeyword, nsCSSProps_KTableEntry, nsCSSValue, nsCSSUnit};
 use gecko_bindings::structs::{nsMediaExpression_Range, nsMediaFeature};
 use gecko_bindings::structs::{nsMediaFeature_ValueType, nsMediaFeature_RangeType, nsMediaFeature_RequirementFlags};
 use gecko_bindings::structs::{nsPresContext, RawGeckoPresContextOwned};
 use gecko_bindings::structs::nsIAtom;
 use media_queries::MediaType;
 use parser::ParserContext;
 use properties::{ComputedValues, StyleBuilder};
 use properties::longhands::font_size;
 use rule_cache::RuleCacheConditions;
 use servo_arc::Arc;
 use std::cell::RefCell;
 use std::fmt::{self, Write};
-use std::sync::atomic::{AtomicBool, AtomicIsize, AtomicUsize, Ordering};
+use std::sync::atomic::{AtomicBool, AtomicIsize, Ordering};
 use str::starts_with_ignore_ascii_case;
 use string_cache::Atom;
 use style_traits::{CSSPixel, DevicePixel};
 use style_traits::{ToCss, ParseError, StyleParseError};
 use style_traits::viewport::ViewportConstraints;
 use values::{CSSFloat, CustomIdent, serialize_dimension};
 use values::computed::{self, ToComputedValue};
 use values::specified::Length;
@@ -49,21 +49,16 @@ pub struct Device {
     /// This is set when computing the style of the root
     /// element, and used for rem units in other elements.
     ///
     /// When computing the style of the root element, there can't be any
     /// other style being computed at the same time, given we need the style of
     /// the parent to compute everything else. So it is correct to just use
     /// a relaxed atomic here.
     root_font_size: AtomicIsize,
-    /// The body text color, stored as an `nscolor`, used for the "tables
-    /// inherit from body" quirk.
-    ///
-    /// https://quirks.spec.whatwg.org/#the-tables-inherit-color-from-body-quirk
-    body_text_color: AtomicUsize,
     /// Whether any styles computed in the document relied on the root font-size
     /// by using rem units.
     used_root_font_size: AtomicBool,
     /// Whether any styles computed in the document relied on the viewport size
     /// by using vw/vh/vmin/vmax units.
     used_viewport_size: AtomicBool,
 }
 
@@ -74,17 +69,16 @@ impl Device {
     /// Trivially constructs a new `Device`.
     pub fn new(pres_context: RawGeckoPresContextOwned) -> Self {
         assert!(!pres_context.is_null());
         Device {
             pres_context: pres_context,
             default_values: ComputedValues::default_values(unsafe { &*pres_context }),
             // FIXME(bz): Seems dubious?
             root_font_size: AtomicIsize::new(font_size::get_initial_value().0.to_i32_au() as isize),
-            body_text_color: AtomicUsize::new(unsafe { &*pres_context }.mDefaultColor as usize),
             used_root_font_size: AtomicBool::new(false),
             used_viewport_size: AtomicBool::new(false),
         }
     }
 
     /// Tells the device that a new viewport rule has been found, and stores the
     /// relevant viewport constraints.
     pub fn account_for_viewport_rule(
@@ -111,28 +105,16 @@ impl Device {
         Au::new(self.root_font_size.load(Ordering::Relaxed) as i32)
     }
 
     /// Set the font size of the root element (for rem)
     pub fn set_root_font_size(&self, size: Au) {
         self.root_font_size.store(size.0 as isize, Ordering::Relaxed)
     }
 
-    /// Sets the body text color for the "inherit color from body" quirk.
-    ///
-    /// https://quirks.spec.whatwg.org/#the-tables-inherit-color-from-body-quirk
-    pub fn set_body_text_color(&self, color: RGBA) {
-        self.body_text_color.store(convert_rgba_to_nscolor(&color) as usize, Ordering::Relaxed)
-    }
-
-    /// Returns the body text color.
-    pub fn body_text_color(&self) -> RGBA {
-        convert_nscolor_to_rgba(self.body_text_color.load(Ordering::Relaxed) as u32)
-    }
-
     /// Gets the pres context associated with this document.
     pub fn pres_context(&self) -> &nsPresContext {
         unsafe { &*self.pres_context }
     }
 
     /// Recreates the default computed values.
     pub fn reset_computed_values(&mut self) {
         self.default_values = ComputedValues::default_values(self.pres_context());
--- a/servo/components/style/gecko/wrapper.rs
+++ b/servo/components/style/gecko/wrapper.rs
@@ -588,20 +588,16 @@ impl<'le> GeckoElement<'le> {
             !self.get_non_xul_xbl_binding_parent_raw_content().is_null()
         }
     }
 
     fn namespace_id(&self) -> i32 {
         self.as_node().node_info().mInner.mNamespaceID
     }
 
-    fn is_html_element(&self) -> bool {
-        self.namespace_id() == (structs::root::kNameSpaceID_XHTML as i32)
-    }
-
     fn is_xul_element(&self) -> bool {
         self.namespace_id() == (structs::root::kNameSpaceID_XUL as i32)
     }
 
     /// Sets the specified element data, return any existing data.
     ///
     /// Like `ensure_data`, only safe to call with exclusive access to the
     /// element.
@@ -1483,46 +1479,34 @@ impl<'le> TElement for GeckoElement<'le>
         let ptr = unsafe { bindings::Gecko_LangValue(self.0) };
         if ptr.is_null() {
             None
         } else {
             Some(unsafe { Atom::from_addrefed(ptr) })
         }
     }
 
-    fn match_element_lang(
-        &self,
-        override_lang: Option<Option<AttrValue>>,
-        value: &PseudoClassStringArg
-    ) -> bool {
+    fn match_element_lang(&self,
+                          override_lang: Option<Option<AttrValue>>,
+                          value: &PseudoClassStringArg)
+                          -> bool
+    {
         // Gecko supports :lang() from CSS Selectors 3, which only accepts a
         // single language tag, and which performs simple dash-prefix matching
         // on it.
         debug_assert!(value.len() > 0 && value[value.len() - 1] == 0,
                       "expected value to be null terminated");
         let override_lang_ptr = match &override_lang {
             &Some(Some(ref atom)) => atom.as_ptr(),
             _ => ptr::null_mut(),
         };
         unsafe {
             Gecko_MatchLang(self.0, override_lang_ptr, override_lang.is_some(), value.as_ptr())
         }
     }
-
-    fn is_html_document_body_element(&self) -> bool {
-        if self.get_local_name() != &*local_name!("body") {
-            return false;
-        }
-
-        if !self.is_html_element() {
-            return false;
-        }
-
-        unsafe { bindings::Gecko_IsDocumentBody(self.0) }
-    }
 }
 
 impl<'le> PartialEq for GeckoElement<'le> {
     fn eq(&self, other: &Self) -> bool {
         self.0 as *const _ == other.0 as *const _
     }
 }
 
@@ -1733,22 +1717,21 @@ impl<'le> ::selectors::Element for Gecko
             if let Some(el) = sibling_node.as_element() {
                 return Some(el)
             }
             sibling = sibling_node.next_sibling();
         }
         None
     }
 
-    fn attr_matches(
-        &self,
-        ns: &NamespaceConstraint<&Namespace>,
-        local_name: &Atom,
-        operation: &AttrSelectorOperation<&Atom>
-    ) -> bool {
+    fn attr_matches(&self,
+                    ns: &NamespaceConstraint<&Namespace>,
+                    local_name: &Atom,
+                    operation: &AttrSelectorOperation<&Atom>)
+                    -> bool {
         unsafe {
             match *operation {
                 AttrSelectorOperation::Exists => {
                     bindings::Gecko_HasAttr(self.0,
                                             ns.atom_or_null(),
                                             local_name.as_ptr())
                 }
                 AttrSelectorOperation::WithValue { operator, case_sensitivity, expected_value } => {
@@ -2022,18 +2005,20 @@ impl<'le> ::selectors::Element for Gecko
 
         snapshot_helpers::has_class(self.0,
                                     name,
                                     case_sensitivity,
                                     Gecko_ClassOrClassList)
     }
 
     fn is_html_element_in_html_document(&self) -> bool {
-        self.is_html_element() &&
-        self.as_node().owner_doc().mType == structs::root::nsIDocument_Type::eHTML
+        let node = self.as_node();
+        let node_info = node.node_info();
+        node_info.mInner.mNamespaceID == (structs::root::kNameSpaceID_XHTML as i32) &&
+        node.owner_doc().mType == structs::root::nsIDocument_Type::eHTML
     }
 
     fn ignores_nth_child_selectors(&self) -> bool {
         self.is_root_of_anonymous_subtree()
     }
 
     fn blocks_ancestor_combinators(&self) -> bool {
         if !self.is_root_of_anonymous_subtree() {
--- a/servo/components/style/matching.rs
+++ b/servo/components/style/matching.rs
@@ -2,18 +2,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! High-level interface to CSS selector matching.
 
 #![allow(unsafe_code)]
 #![deny(missing_docs)]
 
-use context::{ElementCascadeInputs, QuirksMode, SelectorFlagsMap};
-use context::{SharedStyleContext, StyleContext};
+use context::{ElementCascadeInputs, SelectorFlagsMap, SharedStyleContext, StyleContext};
 use data::ElementData;
 use dom::TElement;
 use invalidation::element::restyle_hints::{RESTYLE_CSS_ANIMATIONS, RESTYLE_CSS_TRANSITIONS};
 use invalidation::element::restyle_hints::{RESTYLE_SMIL, RESTYLE_STYLE_ATTRIBUTE};
 use invalidation::element::restyle_hints::RestyleHint;
 use properties::ComputedValues;
 use rule_tree::{CascadeLevel, StrongRuleNode};
 use selector_parser::{PseudoElement, RestyleDamage};
@@ -547,16 +546,20 @@ pub trait MatchMethods : TElement {
             important_rules_changed,
         );
 
         // First of all, update the styles.
         let old_styles = data.set_styles(new_styles);
 
         // Propagate the "can be fragmented" bit. It would be nice to
         // encapsulate this better.
+        //
+        // Note that this is technically not needed for pseudos since we already
+        // do that when we resolve the non-pseudo style, but it doesn't hurt
+        // anyway.
         if cfg!(feature = "servo") {
             let layout_parent =
                 self.inheritance_parent().map(|e| e.layout_parent());
             let layout_parent_data =
                 layout_parent.as_ref().and_then(|e| e.borrow_data());
             let layout_parent_style =
                 layout_parent_data.as_ref().map(|d| d.styles.primary());
 
@@ -582,31 +585,16 @@ pub trait MatchMethods : TElement {
                 // in the document did use rem units, ensure we recascade the
                 // entire tree.
                 if device.used_root_font_size() {
                     cascade_requirement = ChildCascadeRequirement::MustCascadeDescendants;
                 }
             }
         }
 
-        if context.shared.stylist.quirks_mode() == QuirksMode::Quirks {
-            if self.is_html_document_body_element() {
-                // NOTE(emilio): We _could_ handle dynamic changes to it if it
-                // changes and before we reach our children the cascade stops,
-                // but we don't track right now whether we use the document body
-                // color, and nobody else handles that properly anyway.
-
-                let device = context.shared.stylist.device();
-
-                // Needed for the "inherit from body" quirk.
-                let text_color = new_primary_style.get_color().clone_color();
-                device.set_body_text_color(text_color);
-            }
-        }
-
         // Don't accumulate damage if we're in a forgetful traversal.
         if context.shared.traversal_flags.contains(traversal_flags::Forgetful) {
             return ChildCascadeRequirement::MustCascadeChildren;
         }
 
         // Also, don't do anything if there was no style.
         let old_primary_style = match old_styles.primary {
             Some(s) => s,
--- a/servo/components/style/servo/media_queries.rs
+++ b/servo/components/style/servo/media_queries.rs
@@ -87,23 +87,16 @@ impl Device {
         Au::new(self.root_font_size.load(Ordering::Relaxed) as i32)
     }
 
     /// Set the font size of the root element (for rem)
     pub fn set_root_font_size(&self, size: Au) {
         self.root_font_size.store(size.0 as isize, Ordering::Relaxed)
     }
 
-    /// Sets the body text color for the "inherit color from body" quirk.
-    ///
-    /// https://quirks.spec.whatwg.org/#the-tables-inherit-color-from-body-quirk
-    pub fn set_body_text_color(&self, _color: RGBA) {
-        // Servo doesn't implement this quirk (yet)
-    }
-
     /// Returns whether we ever looked up the root font size of the Device.
     pub fn used_root_font_size(&self) -> bool {
         self.used_root_font_size.load(Ordering::Relaxed)
     }
 
     /// Returns the viewport size of the current device in app units, needed,
     /// among other things, to resolve viewport units.
     #[inline]
--- a/servo/components/style/values/specified/color.rs
+++ b/servo/components/style/values/specified/color.rs
@@ -284,17 +284,29 @@ impl ToComputedValue for Color {
                     Keyword::MozDefaultBackgroundColor => pres_context.mBackgroundColor,
                     Keyword::MozHyperlinktext => pres_context.mLinkColor,
                     Keyword::MozActiveHyperlinktext => pres_context.mActiveLinkColor,
                     Keyword::MozVisitedHyperlinktext => pres_context.mVisitedLinkColor,
                 })
             }
             #[cfg(feature = "gecko")]
             Color::InheritFromBodyQuirk => {
-                ComputedColor::rgba(context.device().body_text_color())
+                use dom::TElement;
+                use gecko::wrapper::GeckoElement;
+                use gecko_bindings::bindings::Gecko_GetBody;
+                let pres_context = context.device().pres_context();
+                let body = unsafe { Gecko_GetBody(pres_context) }.map(GeckoElement);
+                let data = body.as_ref().and_then(|wrap| wrap.borrow_data());
+                if let Some(data) = data {
+                    ComputedColor::rgba(data.styles.primary()
+                                            .get_color()
+                                            .clone_color())
+                } else {
+                    convert_nscolor_to_computedcolor(pres_context.mDefaultColor)
+                }
             },
         }
     }
 
     fn from_computed_value(computed: &ComputedColor) -> Self {
         if computed.is_numeric() {
             Color::rgba(computed.color)
         } else if computed.is_currentcolor() {