Auto merge of #17206 - heycam:lang-snapshots, r=emilio

match :lang() against snapshots correctly

Reviewed in https://bugzilla.mozilla.org/show_bug.cgi?id=1365162.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17206)
<!-- Reviewable:end -->

components/script/dom/bindings/str.rs

@@ -113,63 +113,6 @@ pub fn is_token(s: &[u8]) -> bool {
     })
 }
 
-/// Returns whether the language is matched, as defined by
-/// [RFC 4647](https://tools.ietf.org/html/rfc4647#section-3.3.2).
-pub fn extended_filtering(tag: &str, range: &str) -> bool {
-    let lang_ranges: Vec<&str> = range.split(',').collect();
-
-    lang_ranges.iter().any(|&lang_range| {
-        // step 1
-        let range_subtags: Vec<&str> = lang_range.split('\x2d').collect();
-        let tag_subtags: Vec<&str> = tag.split('\x2d').collect();
-
-        let mut range_iter = range_subtags.iter();
-        let mut tag_iter = tag_subtags.iter();
-
-        // step 2
-        // Note: [Level-4 spec](https://drafts.csswg.org/selectors/#lang-pseudo) check for wild card
-        if let (Some(range_subtag), Some(tag_subtag)) = (range_iter.next(), tag_iter.next()) {
-            if !(range_subtag.eq_ignore_ascii_case(tag_subtag) || range_subtag.eq_ignore_ascii_case("*")) {
-                return false;
-            }
-        }
-
-        let mut current_tag_subtag = tag_iter.next();
-
-        // step 3
-        for range_subtag in range_iter {
-            // step 3a
-            if range_subtag.eq_ignore_ascii_case("*") {
-                continue;
-            }
-            match current_tag_subtag.clone() {
-                Some(tag_subtag) => {
-                    // step 3c
-                    if range_subtag.eq_ignore_ascii_case(tag_subtag) {
-                        current_tag_subtag = tag_iter.next();
-                        continue;
-                    } else {
-                        // step 3d
-                        if tag_subtag.len() == 1 {
-                            return false;
-                        } else {
-                            // else step 3e - continue with loop
-                            current_tag_subtag = tag_iter.next();
-                            if current_tag_subtag.is_none() {
-                                return false;
-                            }
-                        }
-                    }
-                },
-                // step 3b
-                None => { return false; }
-            }
-        }
-        // step 4
-        true
-    })
-}
-
 
 /// A DOMString.
 ///

components/script/dom/element.rs

@@ -26,7 +26,7 @@ use dom::bindings::js::{JS, LayoutJS, MutNullableJS};
 use dom::bindings::js::{Root, RootedReference};
 use dom::bindings::refcounted::{Trusted, TrustedPromise};
 use dom::bindings::reflector::DomObject;
-use dom::bindings::str::{DOMString, extended_filtering};
+use dom::bindings::str::DOMString;
 use dom::bindings::xmlname::{namespace_from_domstring, validate_and_extract, xml_name_type};
 use dom::bindings::xmlname::XMLName::InvalidXMLName;
 use dom::characterdata::CharacterData;
@@ -106,6 +106,7 @@ use style::properties::longhands::{self, background_image, border_spacing, font_
 use style::restyle_hints::RestyleHint;
 use style::rule_tree::CascadeLevel;
 use style::selector_parser::{NonTSPseudoClass, PseudoElement, RestyleDamage, SelectorImpl, SelectorParser};
+use style::selector_parser::extended_filtering;
 use style::shared_lock::{SharedRwLock, Locked};
 use style::sink::Push;
 use style::stylearc::Arc;
@@ -2464,8 +2465,10 @@ impl<'a> ::selectors::Element for Root<Element> {
                     .map_or(false, |attr| attr.value().eq(expected_value))
             }
 
-            // FIXME(#15746): This is wrong, we need to instead use extended filtering as per RFC4647
+            // FIXME(heycam): This is wrong, since extended_filtering accepts
-            //                https://tools.ietf.org/html/rfc4647#section-3.3.2
+            // a string containing commas (separating each language tag in
+            // a list) but the pseudo-class instead should be parsing and
+            // storing separate <ident> or <string>s for each language tag.
             NonTSPseudoClass::Lang(ref lang) => extended_filtering(&*self.get_lang(), &*lang),
 
             NonTSPseudoClass::ReadOnly =>

components/script/layout_wrapper.rs

@@ -34,7 +34,6 @@ use atomic_refcell::{AtomicRef, AtomicRefCell};
 use dom::bindings::inheritance::{CharacterDataTypeId, ElementTypeId};
 use dom::bindings::inheritance::{HTMLElementTypeId, NodeTypeId};
 use dom::bindings::js::LayoutJS;
-use dom::bindings::str::extended_filtering;
 use dom::characterdata::LayoutCharacterDataHelpers;
 use dom::document::{Document, LayoutDocumentHelpers, PendingRestyle};
 use dom::element::{Element, LayoutElementHelpers, RawLayoutElementHelpers};
@@ -70,7 +69,8 @@ use style::dom::{PresentationalHintsSynthesizer, TElement, TNode, UnsafeNode};
 use style::element_state::*;
 use style::font_metrics::ServoMetricsProvider;
 use style::properties::{ComputedValues, PropertyDeclarationBlock};
-use style::selector_parser::{NonTSPseudoClass, PseudoElement, SelectorImpl};
+use style::selector_parser::{AttrValue as SelectorAttrValue, NonTSPseudoClass, PseudoClassStringArg};
+use style::selector_parser::{PseudoElement, SelectorImpl, extended_filtering};
 use style::shared_lock::{SharedRwLock as StyleSharedRwLock, Locked as StyleLocked};
 use style::sink::Push;
 use style::str::is_whitespace;
@@ -499,6 +499,39 @@ impl<'le> TElement for ServoLayoutElement<'le> {
     fn has_css_transitions(&self) -> bool {
         unreachable!("this should be only called on gecko");
     }
+
+    #[inline]
+    fn lang_attr(&self) -> Option<SelectorAttrValue> {
+        self.get_attr(&ns!(xml), &local_name!("lang"))
+            .or_else(|| self.get_attr(&ns!(), &local_name!("lang")))
+            .map(|v| String::from(v as &str))
+    }
+
+    fn match_element_lang(&self,
+                          override_lang: Option<Option<SelectorAttrValue>>,
+                          value: &PseudoClassStringArg)
+                          -> bool
+    {
+        // Servo supports :lang() from CSS Selectors 4, which can take a comma-
+        // separated list of language tags in the pseudo-class, and which
+        // performs RFC 4647 extended filtering matching on them.
+        //
+        // FIXME(heycam): This is wrong, since extended_filtering accepts
+        // a string containing commas (separating each language tag in
+        // a list) but the pseudo-class instead should be parsing and
+        // storing separate <ident> or <string>s for each language tag.
+        //
+        // FIXME(heycam): Look at `element`'s document's Content-Language
+        // HTTP header for language tags to match `value` against.  To
+        // do this, we should make `get_lang_for_layout` return an Option,
+        // 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)
+    }
 }
 
 impl<'le> PartialEq for ServoLayoutElement<'le> {
@@ -691,9 +724,7 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> {
             NonTSPseudoClass::AnyLink => self.is_link(),
             NonTSPseudoClass::Visited => false,
 
-            // FIXME(#15746): This is wrong, we need to instead use extended filtering as per RFC4647
+            NonTSPseudoClass::Lang(ref lang) => self.match_element_lang(None, &*lang),
-            //                https://tools.ietf.org/html/rfc4647#section-3.3.2
-            NonTSPseudoClass::Lang(ref lang) => extended_filtering(&*self.element.get_lang_for_layout(), &*lang),
 
             NonTSPseudoClass::ServoNonZeroBorder => unsafe {
                 match (*self.element.unsafe_get()).get_attr_for_layout(&ns!(), &local_name!("border")) {

components/style/dom.rs

@@ -17,7 +17,8 @@ use properties::{ComputedValues, PropertyDeclarationBlock};
 #[cfg(feature = "gecko")] use properties::animated_properties::AnimationValue;
 #[cfg(feature = "gecko")] use properties::animated_properties::TransitionProperty;
 use rule_tree::CascadeLevel;
-use selector_parser::{ElementExt, PreExistingComputedValues, PseudoElement};
+use selector_parser::{AttrValue, ElementExt, PreExistingComputedValues};
+use selector_parser::{PseudoClassStringArg, PseudoElement};
 use selectors::matching::{ElementSelectorFlags, VisitedHandlingMode};
 use shared_lock::Locked;
 use sink::Push;
@@ -597,6 +598,20 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone +
                                              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;
 }
 
 /// Trait abstracting over different kinds of dirty-descendants bits.

components/style/gecko/generated/bindings.rs

@@ -587,6 +587,12 @@ extern "C" {
     pub fn Gecko_GetElementId(element: RawGeckoElementBorrowed)
      -> *mut nsIAtom;
 }
+extern "C" {
+    pub fn Gecko_MatchLang(element: RawGeckoElementBorrowed,
+                           override_lang: *mut nsIAtom,
+                           has_override_lang: bool, value: *const u16)
+     -> bool;
+}
 extern "C" {
     pub fn Gecko_GetXMLLangValue(element: RawGeckoElementBorrowed)
      -> *mut nsIAtom;
@@ -595,6 +601,9 @@ extern "C" {
     pub fn Gecko_AtomAttrValue(element: RawGeckoElementBorrowed,
                                attribute: *mut nsIAtom) -> *mut nsIAtom;
 }
+extern "C" {
+    pub fn Gecko_LangValue(element: RawGeckoElementBorrowed) -> *mut nsIAtom;
+}
 extern "C" {
     pub fn Gecko_HasAttr(element: RawGeckoElementBorrowed, ns: *mut nsIAtom,
                          name: *mut nsIAtom) -> bool;
@@ -639,6 +648,10 @@ extern "C" {
                                        attribute: *mut nsIAtom)
      -> *mut nsIAtom;
 }
+extern "C" {
+    pub fn Gecko_SnapshotLangValue(element: *const ServoElementSnapshot)
+     -> *mut nsIAtom;
+}
 extern "C" {
     pub fn Gecko_SnapshotHasAttr(element: *const ServoElementSnapshot,
                                  ns: *mut nsIAtom, name: *mut nsIAtom)
@@ -2210,8 +2223,7 @@ extern "C" {
                                          from: *const RawGeckoGfxMatrix4x4,
                                          to: *const RawGeckoGfxMatrix4x4,
                                          progress: f64,
-                                         result:
+                                         result: *mut RawGeckoGfxMatrix4x4);
-                                             *mut RawGeckoGfxMatrix4x4);
 }
 extern "C" {
     pub fn Servo_AnimationValues_Interpolate(from:

components/style/gecko/selector_parser.rs

@@ -24,6 +24,9 @@ bitflags! {
     }
 }
 
+/// The type used for storing pseudo-class string arguments.
+pub type PseudoClassStringArg = Box<[u16]>;
+
 macro_rules! pseudo_class_name {
     (bare: [$(($css:expr, $name:ident, $gecko_type:tt, $state:tt, $flags:tt),)*],
      string: [$(($s_css:expr, $s_name:ident, $s_gecko_type:tt, $s_state:tt, $s_flags:tt),)*],
@@ -37,7 +40,7 @@ macro_rules! pseudo_class_name {
             )*
             $(
                 #[doc = $s_css]
-                $s_name(Box<[u16]>),
+                $s_name(PseudoClassStringArg),
             )*
             $(
                 #[doc = $k_css]
@@ -212,7 +215,8 @@ impl NonTSPseudoClass {
     pub fn is_attr_based(&self) -> bool {
         matches!(*self,
                  NonTSPseudoClass::MozTableBorderNonzero |
-                 NonTSPseudoClass::MozBrowserFrame)
+                 NonTSPseudoClass::MozBrowserFrame |
+                 NonTSPseudoClass::Lang(..))
     }
 }
 

components/style/gecko/snapshot.rs

@@ -181,4 +181,14 @@ impl ElementSnapshot for GeckoElementSnapshot {
                                      callback,
                                      bindings::Gecko_SnapshotClassOrClassList)
     }
+
+    #[inline]
+    fn lang_attr(&self) -> Option<Atom> {
+        let ptr = unsafe { bindings::Gecko_SnapshotLangValue(self) };
+        if ptr.is_null() {
+            None
+        } else {
+            Some(unsafe { Atom::from_addrefed(ptr) })
+        }
+    }
 }

components/style/gecko/wrapper.rs

@@ -45,6 +45,7 @@ use gecko_bindings::bindings::Gecko_GetStyleContext;
 use gecko_bindings::bindings::Gecko_GetUnvisitedLinkAttrDeclarationBlock;
 use gecko_bindings::bindings::Gecko_GetVisitedLinkAttrDeclarationBlock;
 use gecko_bindings::bindings::Gecko_IsSignificantChild;
+use gecko_bindings::bindings::Gecko_MatchLang;
 use gecko_bindings::bindings::Gecko_MatchStringArgPseudo;
 use gecko_bindings::bindings::Gecko_UnsetDirtyStyleAttr;
 use gecko_bindings::bindings::Gecko_UpdateAnimations;
@@ -66,7 +67,7 @@ use properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock};
 use properties::animated_properties::{AnimationValue, AnimationValueMap, TransitionProperty};
 use properties::style_structs::Font;
 use rule_tree::CascadeLevel as ServoCascadeLevel;
-use selector_parser::ElementExt;
+use selector_parser::{AttrValue, ElementExt, PseudoClassStringArg};
 use selectors::Element;
 use selectors::attr::{AttrSelectorOperation, AttrSelectorOperator, CaseSensitivity, NamespaceConstraint};
 use selectors::matching::{ElementSelectorFlags, MatchingContext, MatchingMode};
@@ -1023,6 +1024,35 @@ impl<'le> TElement for GeckoElement<'le> {
                                                    before_change_style,
                                                    after_change_style).does_animate()
     }
+
+    #[inline]
+    fn lang_attr(&self) -> Option<AttrValue> {
+        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
+    {
+        // 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())
+        }
+    }
 }
 
 impl<'le> PartialEq for GeckoElement<'le> {
@@ -1408,11 +1438,13 @@ impl<'le> ::selectors::Element for GeckoElement<'le> {
                     matches_complex_selector(s, 0, self, context, flags_setter)
                 })
             }
+            NonTSPseudoClass::Lang(ref lang_arg) => {
+                self.match_element_lang(None, lang_arg)
+            }
             NonTSPseudoClass::MozSystemMetric(ref s) |
             NonTSPseudoClass::MozLocaleDir(ref s) |
             NonTSPseudoClass::MozEmptyExceptChildrenWithLocalname(ref s) |
-            NonTSPseudoClass::Dir(ref s) |
+            NonTSPseudoClass::Dir(ref s) => {
-            NonTSPseudoClass::Lang(ref s) => {
                 unsafe {
                     let mut set_slow_selector = false;
                     let matches = Gecko_MatchStringArgPseudo(self.0,

components/style/restyle_hints.rs

@@ -555,8 +555,12 @@ pub trait ElementSnapshot : Sized {
     /// only be called if `has_attrs()` returns true.
     fn each_class<F>(&self, F)
         where F: FnMut(&Atom);
+
+    /// The `xml:lang=""` or `lang=""` attribute value per this snapshot.
+    fn lang_attr(&self) -> Option<AttrValue>;
 }
 
+#[derive(Clone)]
 struct ElementWrapper<'a, E>
     where E: TElement,
 {
@@ -606,6 +610,26 @@ impl<'a, E> ElementWrapper<'a, E>
             None => ElementState::empty(),
         }
     }
+
+    /// Returns the value of the `xml:lang=""` (or, if appropriate, `lang=""`)
+    /// attribute from this element's snapshot or the closest ancestor
+    /// element snapshot with the attribute specified.
+    fn get_lang(&self) -> Option<AttrValue> {
+        let mut current = self.clone();
+        loop {
+            let lang = match self.snapshot() {
+                Some(snapshot) if snapshot.has_attrs() => snapshot.lang_attr(),
+                _ => current.element.lang_attr(),
+            };
+            if lang.is_some() {
+                return lang;
+            }
+            match current.parent_element() {
+                Some(parent) => current = parent,
+                None => return None,
+            }
+        }
+    }
 }
 
 impl<'a, E> fmt::Debug for ElementWrapper<'a, E>
@@ -707,6 +731,12 @@ impl<'a, E> Element for ElementWrapper<'a, E>
                 }
             }
 
+            // :lang() needs to match using the closest ancestor xml:lang="" or
+            // lang="" attribtue from snapshots.
+            NonTSPseudoClass::Lang(ref lang_arg) => {
+                return self.element.match_element_lang(Some(self.get_lang()), lang_arg);
+            }
+
             _ => {}
         }
 

components/style/servo/selector_parser.rs

@@ -13,11 +13,12 @@ use dom::{OpaqueNode, TElement, TNode};
 use element_state::ElementState;
 use fnv::FnvHashMap;
 use restyle_hints::ElementSnapshot;
-use selector_parser::{ElementExt, PseudoElementCascadeType, SelectorParser};
+use selector_parser::{AttrValue as SelectorAttrValue, ElementExt, PseudoElementCascadeType, SelectorParser};
 use selectors::Element;
 use selectors::attr::{AttrSelectorOperation, NamespaceConstraint};
 use selectors::parser::SelectorMethods;
 use selectors::visitor::SelectorVisitor;
+use std::ascii::AsciiExt;
 use std::borrow::Cow;
 use std::fmt;
 use std::fmt::Debug;
@@ -159,6 +160,9 @@ impl PseudoElement {
     }
 }
 
+/// The type used for storing pseudo-class string arguments.
+pub type PseudoClassStringArg = Box<str>;
+
 /// A non tree-structural pseudo-class.
 /// See https://drafts.csswg.org/selectors-4/#structural-pseudos
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
@@ -174,7 +178,7 @@ pub enum NonTSPseudoClass {
     Fullscreen,
     Hover,
     Indeterminate,
-    Lang(Box<str>),
+    Lang(PseudoClassStringArg),
     Link,
     PlaceholderShown,
     ReadWrite,
@@ -272,7 +276,7 @@ impl NonTSPseudoClass {
     /// Returns true if the evaluation of the pseudo-class depends on the
     /// element's attributes.
     pub fn is_attr_based(&self) -> bool {
-        false
+        matches!(*self, NonTSPseudoClass::Lang(..))
     }
 }
 
@@ -584,6 +588,12 @@ impl ElementSnapshot for ServoElementSnapshot {
             }
         }
     }
+
+    fn lang_attr(&self) -> Option<SelectorAttrValue> {
+        self.get_attr(&ns!(xml), &local_name!("lang"))
+            .or_else(|| self.get_attr(&ns!(), &local_name!("lang")))
+            .map(|v| String::from(v as &str))
+    }
 }
 
 impl ServoElementSnapshot {
@@ -611,3 +621,55 @@ impl<E: Element<Impl=SelectorImpl> + Debug> ElementExt for E {
         true
     }
 }
+
+/// Returns whether the language is matched, as defined by
+/// [RFC 4647](https://tools.ietf.org/html/rfc4647#section-3.3.2).
+pub fn extended_filtering(tag: &str, range: &str) -> bool {
+    range.split(',').any(|lang_range| {
+        // step 1
+        let mut range_subtags = lang_range.split('\x2d');
+        let mut tag_subtags = tag.split('\x2d');
+
+        // step 2
+        // Note: [Level-4 spec](https://drafts.csswg.org/selectors/#lang-pseudo) check for wild card
+        if let (Some(range_subtag), Some(tag_subtag)) = (range_subtags.next(), tag_subtags.next()) {
+            if !(range_subtag.eq_ignore_ascii_case(tag_subtag) || range_subtag.eq_ignore_ascii_case("*")) {
+                return false;
+            }
+        }
+
+        let mut current_tag_subtag = tag_subtags.next();
+
+        // step 3
+        for range_subtag in range_subtags {
+            // step 3a
+            if range_subtag == "*" {
+                continue;
+            }
+            match current_tag_subtag.clone() {
+                Some(tag_subtag) => {
+                    // step 3c
+                    if range_subtag.eq_ignore_ascii_case(tag_subtag) {
+                        current_tag_subtag = tag_subtags.next();
+                        continue;
+                    }
+                    // step 3d
+                    if tag_subtag.len() == 1 {
+                        return false;
+                    }
+                    // else step 3e - continue with loop
+                    current_tag_subtag = tag_subtags.next();
+                    if current_tag_subtag.is_none() {
+                        return false;
+                    }
+                },
+                // step 3b
+                None => {
+                    return false;
+                }
+            }
+        }
+        // step 4
+        true
+    })
+}