From 0b1e51de5335961234c8f54a7299b61937dfc479 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 17 May 2017 15:03:26 +0200 Subject: [PATCH 01/12] Rename CaseSensitivity::CaseInsensitive to AsciiCaseInsensitive --- components/selectors/matching.rs | 2 +- components/selectors/parser.rs | 6 +++--- tests/unit/style/stylesheets.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs index 9c60e6b78d7..beaf0d06ed2 100644 --- a/components/selectors/matching.rs +++ b/components/selectors/matching.rs @@ -415,7 +415,7 @@ fn matches_simple_selector( Component::AttrEqual(ref attr, ref value, case_sensitivity) => { match case_sensitivity { CaseSensitivity::CaseSensitive => element.match_attr_equals(attr, value), - CaseSensitivity::CaseInsensitive => element.match_attr_equals_ignore_ascii_case(attr, value), + CaseSensitivity::AsciiCaseInsensitive => element.match_attr_equals_ignore_ascii_case(attr, value), } } Component::AttrIncludes(ref attr, ref value) => { diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index 01f1eafc07c..0c9a54defdf 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -608,7 +608,7 @@ impl Component { #[derive(Eq, PartialEq, Clone, Copy, Debug)] pub enum CaseSensitivity { CaseSensitive, // Selectors spec says language-defined, but HTML says sensitive. - CaseInsensitive, + AsciiCaseInsensitive, } @@ -754,7 +754,7 @@ impl ToCss for Component { AttrEqual(ref a, ref v, case) => { attr_selector_to_css(a, " = ", v, match case { CaseSensitivity::CaseSensitive => None, - CaseSensitivity::CaseInsensitive => Some(" i"), + CaseSensitivity::AsciiCaseInsensitive => Some(" i"), }, dest) } AttrDashMatch(ref a, ref v) => attr_selector_to_css(a, " |= ", v, None, dest), @@ -1322,7 +1322,7 @@ fn parse_attribute_flags(input: &mut CssParser) -> Result { match input.next() { Err(()) => Ok(CaseSensitivity::CaseSensitive), Ok(Token::Ident(ref value)) if value.eq_ignore_ascii_case("i") => { - Ok(CaseSensitivity::CaseInsensitive) + Ok(CaseSensitivity::AsciiCaseInsensitive) } _ => Err(()) } diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index 6d2a2d72bee..fde49c6acb9 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -104,7 +104,7 @@ fn test_parse_stylesheet() { prefix: None, url: ns!() }), - }, "hidden".to_owned(), CaseSensitivity::CaseInsensitive) + }, "hidden".to_owned(), CaseSensitivity::AsciiCaseInsensitive) ]), (0 << 20) + (1 << 10) + (1 << 0) ), From 2ca2c2d2be88176897f7fc76c7b7112f14cb6c9a Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 17 May 2017 17:10:20 +0200 Subject: [PATCH 02/12] Fix [foo=bar i] selectors for Stylo --- components/style/gecko/wrapper.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 7aa879c3d11..99999ecdc87 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -1393,7 +1393,7 @@ impl<'le> ::selectors::MatchAttr for GeckoElement<'le> { attr.ns_or_null(), attr.select_name(self.is_html_element_in_html_document()), value.as_ptr(), - /* ignoreCase = */ false) + /* ignoreCase = */ true) } } fn match_attr_includes(&self, attr: &AttrSelector, value: &Atom) -> bool { From 83c7824fda2377ea2b52414de35d146dd4f6e64d Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 17 May 2017 19:41:21 +0200 Subject: [PATCH 03/12] Simplify rust-selectors API for attribute selectors --- components/script/dom/element.rs | 59 +++---- components/script/layout_wrapper.rs | 95 ++++++----- .../script_layout_interface/wrapper_traits.rs | 3 + components/selectors/attr.rs | 114 +++++++++++++ components/selectors/lib.rs | 2 +- components/selectors/matching.rs | 66 ++++++-- components/selectors/parser.rs | 10 +- components/selectors/tree.rs | 121 +------------- components/style/attr.rs | 8 + components/style/gecko/snapshot.rs | 137 +++++++-------- components/style/gecko/wrapper.rs | 158 ++++++++---------- components/style/restyle_hints.rs | 111 +++--------- components/style/servo/selector_parser.rs | 39 +++-- servo-tidy.toml | 3 - tests/unit/style/stylesheets.rs | 1 + 15 files changed, 447 insertions(+), 480 deletions(-) create mode 100644 components/selectors/attr.rs diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 6b844dd83a2..9c6d84ba485 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -86,9 +86,10 @@ use net_traits::request::CorsSettings; use ref_filter_map::ref_filter_map; use script_layout_interface::message::ReflowQueryType; use script_thread::Runnable; +use selectors::attr::AttrSelectorOperation; use selectors::matching::{ElementSelectorFlags, MatchingContext, MatchingMode, matches_selector_list}; use selectors::matching::{HAS_EDGE_CHILD_SELECTOR, HAS_SLOW_SELECTOR, HAS_SLOW_SELECTOR_LATER_SIBLINGS}; -use selectors::parser::{AttrSelector, NamespaceConstraint}; +use selectors::parser::NamespaceConstraint; use servo_atoms::Atom; use std::ascii::AsciiExt; use std::borrow::Cow; @@ -288,7 +289,7 @@ pub trait RawLayoutElementHelpers { -> Option<&'a AttrValue>; unsafe fn get_attr_val_for_layout<'a>(&'a self, namespace: &Namespace, name: &LocalName) -> Option<&'a str>; - unsafe fn get_attr_vals_for_layout<'a>(&'a self, name: &LocalName) -> Vec<&'a str>; + unsafe fn get_attr_vals_for_layout<'a>(&'a self, name: &LocalName) -> Vec<&'a AttrValue>; } #[inline] @@ -314,6 +315,7 @@ impl RawLayoutElementHelpers for Element { }) } + #[inline] unsafe fn get_attr_val_for_layout<'a>(&'a self, namespace: &Namespace, name: &LocalName) -> Option<&'a str> { get_attr_for_layout(self, namespace, name).map(|attr| { @@ -322,12 +324,12 @@ impl RawLayoutElementHelpers for Element { } #[inline] - unsafe fn get_attr_vals_for_layout<'a>(&'a self, name: &LocalName) -> Vec<&'a str> { + unsafe fn get_attr_vals_for_layout<'a>(&'a self, name: &LocalName) -> Vec<&'a AttrValue> { let attrs = self.attrs.borrow_for_layout(); attrs.iter().filter_map(|attr| { let attr = attr.to_layout(); if *name == attr.local_name_atom_forever() { - Some(attr.value_ref_forever()) + Some(attr.value_forever()) } else { None } @@ -2352,37 +2354,9 @@ impl VirtualMethods for Element { } } -impl<'a> ::selectors::MatchAttrGeneric for Root { +impl<'a> ::selectors::Element for Root { type Impl = SelectorImpl; - fn match_attr(&self, attr: &AttrSelector, test: F) -> bool - where F: Fn(&str) -> bool - { - use ::selectors::Element; - let local_name = { - if self.is_html_element_in_html_document() { - &attr.lower_name - } else { - &attr.name - } - }; - match attr.namespace { - NamespaceConstraint::Specific(ref ns) => { - self.get_attribute(&ns.url, local_name) - .map_or(false, |attr| { - test(&attr.value()) - }) - }, - NamespaceConstraint::Any => { - self.attrs.borrow().iter().any(|attr| { - attr.local_name() == local_name && test(&attr.value()) - }) - } - } - } -} - -impl<'a> ::selectors::Element for Root { fn parent_element(&self) -> Option> { self.upcast::().GetParentElement() } @@ -2412,6 +2386,25 @@ impl<'a> ::selectors::Element for Root { self.node.following_siblings().filter_map(Root::downcast).next() } + fn attr_matches(&self, + ns: &NamespaceConstraint, + local_name: &LocalName, + operation: &AttrSelectorOperation) + -> bool { + match *ns { + NamespaceConstraint::Specific(ref ns) => { + self.get_attribute(&ns.url, local_name) + .map_or(false, |attr| attr.value().eval_selector(operation)) + } + NamespaceConstraint::Any => { + self.attrs.borrow().iter().any(|attr| { + attr.local_name() == local_name && + attr.value().eval_selector(operation) + }) + } + } + } + fn is_root(&self) -> bool { match self.node.GetParentNode() { None => false, diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index 4d1810978d8..773937fce00 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -50,8 +50,9 @@ use script_layout_interface::{HTMLCanvasData, LayoutNodeType, SVGSVGData, Truste use script_layout_interface::{OpaqueStyleAndLayoutData, PartialPersistentLayoutData}; use script_layout_interface::wrapper_traits::{DangerousThreadSafeLayoutNode, GetLayoutData, LayoutNode}; use script_layout_interface::wrapper_traits::{PseudoElementType, ThreadSafeLayoutElement, ThreadSafeLayoutNode}; +use selectors::attr::AttrSelectorOperation; use selectors::matching::{ElementSelectorFlags, MatchingContext}; -use selectors::parser::{AttrSelector, NamespaceConstraint}; +use selectors::parser::NamespaceConstraint; use servo_atoms::Atom; use servo_url::ServoUrl; use std::fmt; @@ -509,6 +510,13 @@ impl<'le> ServoLayoutElement<'le> { } } + #[inline] + fn get_attr_enum(&self, namespace: &Namespace, name: &LocalName) -> Option<&AttrValue> { + unsafe { + (*self.element.unsafe_get()).get_attr_for_layout(namespace, name) + } + } + #[inline] fn get_attr(&self, namespace: &Namespace, name: &LocalName) -> Option<&str> { unsafe { @@ -558,32 +566,9 @@ fn as_element<'le>(node: LayoutJS) -> Option> { node.downcast().map(ServoLayoutElement::from_layout_js) } -impl<'le> ::selectors::MatchAttrGeneric for ServoLayoutElement<'le> { +impl<'le> ::selectors::Element for ServoLayoutElement<'le> { type Impl = SelectorImpl; - fn match_attr(&self, attr: &AttrSelector, test: F) -> bool - where F: Fn(&str) -> bool { - use ::selectors::Element; - let name = if self.is_html_element_in_html_document() { - &attr.lower_name - } else { - &attr.name - }; - match attr.namespace { - NamespaceConstraint::Specific(ref ns) => { - self.get_attr(&ns.url, name).map_or(false, |attr| test(attr)) - }, - NamespaceConstraint::Any => { - let attrs = unsafe { - (*self.element.unsafe_get()).get_attr_vals_for_layout(name) - }; - attrs.iter().any(|attr| test(*attr)) - } - } - } -} - -impl<'le> ::selectors::Element for ServoLayoutElement<'le> { fn parent_element(&self) -> Option> { unsafe { self.element.upcast().parent_node_ref().and_then(as_element) @@ -620,6 +605,25 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { None } + fn attr_matches(&self, + ns: &NamespaceConstraint, + local_name: &LocalName, + operation: &AttrSelectorOperation) + -> bool { + match *ns { + NamespaceConstraint::Specific(ref ns) => { + self.get_attr_enum(&ns.url, local_name) + .map_or(false, |value| value.eval_selector(operation)) + } + NamespaceConstraint::Any => { + let values = unsafe { + (*self.element.unsafe_get()).get_attr_vals_for_layout(local_name) + }; + values.iter().any(|value| value.eval_selector(operation)) + } + } + } + fn is_root(&self) -> bool { match self.as_node().parent_node() { None => false, @@ -1075,6 +1079,10 @@ impl<'le> ThreadSafeLayoutElement for ServoThreadSafeLayoutElement<'le> { self.element } + fn get_attr_enum(&self, namespace: &Namespace, name: &LocalName) -> Option<&AttrValue> { + self.element.get_attr_enum(namespace, name) + } + fn get_attr<'a>(&'a self, namespace: &Namespace, name: &LocalName) -> Option<&'a str> { self.element.get_attr(namespace, name) } @@ -1096,25 +1104,9 @@ impl<'le> ThreadSafeLayoutElement for ServoThreadSafeLayoutElement<'le> { /// /// Note that the element implementation is needed only for selector matching, /// not for inheritance (styles are inherited appropiately). -impl<'le> ::selectors::MatchAttrGeneric for ServoThreadSafeLayoutElement<'le> { +impl<'le> ::selectors::Element for ServoThreadSafeLayoutElement<'le> { type Impl = SelectorImpl; - fn match_attr(&self, attr: &AttrSelector, test: F) -> bool - where F: Fn(&str) -> bool { - match attr.namespace { - NamespaceConstraint::Specific(ref ns) => { - self.get_attr(&ns.url, &attr.name).map_or(false, |attr| test(attr)) - }, - NamespaceConstraint::Any => { - unsafe { - (*self.element.element.unsafe_get()).get_attr_vals_for_layout(&attr.name).iter() - .any(|attr| test(*attr)) - } - } - } - } -} -impl<'le> ::selectors::Element for ServoThreadSafeLayoutElement<'le> { fn parent_element(&self) -> Option { warn!("ServoThreadSafeLayoutElement::parent_element called"); None @@ -1166,6 +1158,25 @@ impl<'le> ::selectors::Element for ServoThreadSafeLayoutElement<'le> { false } + fn attr_matches(&self, + ns: &NamespaceConstraint, + local_name: &LocalName, + operation: &AttrSelectorOperation) + -> bool { + match *ns { + NamespaceConstraint::Specific(ref ns) => { + self.get_attr_enum(&ns.url, local_name) + .map_or(false, |value| value.eval_selector(operation)) + } + NamespaceConstraint::Any => { + let values = unsafe { + (*self.element.element.unsafe_get()).get_attr_vals_for_layout(local_name) + }; + values.iter().any(|v| v.eval_selector(operation)) + } + } + } + fn match_non_ts_pseudo_class(&self, _: &NonTSPseudoClass, _: &mut MatchingContext, diff --git a/components/script_layout_interface/wrapper_traits.rs b/components/script_layout_interface/wrapper_traits.rs index 2ba72231269..40b719f6dbd 100644 --- a/components/script_layout_interface/wrapper_traits.rs +++ b/components/script_layout_interface/wrapper_traits.rs @@ -15,6 +15,7 @@ use msg::constellation_msg::{BrowsingContextId, PipelineId}; use range::Range; use servo_url::ServoUrl; use std::fmt::Debug; +use style::attr::AttrValue; use style::computed_values::display; use style::context::SharedStyleContext; use style::data::ElementData; @@ -335,6 +336,8 @@ pub trait ThreadSafeLayoutElement: Clone + Copy + Sized + Debug + #[inline] fn get_attr(&self, namespace: &Namespace, name: &LocalName) -> Option<&str>; + fn get_attr_enum(&self, namespace: &Namespace, name: &LocalName) -> Option<&AttrValue>; + fn get_style_data(&self) -> Option<&AtomicRefCell>; #[inline] diff --git a/components/selectors/attr.rs b/components/selectors/attr.rs new file mode 100644 index 00000000000..90707c54351 --- /dev/null +++ b/components/selectors/attr.rs @@ -0,0 +1,114 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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/. */ + +use parser::SelectorImpl; +use std::ascii::AsciiExt; + +pub enum AttrSelectorOperation<'a, Impl: SelectorImpl> where Impl::AttrValue: 'a { + Exists, + WithValue { + operator: AttrSelectorOperator, + case_sensitivity: CaseSensitivity, + expected_value: &'a Impl::AttrValue, + } +} + +impl<'a, Impl: SelectorImpl> AttrSelectorOperation<'a, Impl> { + pub fn eval_str(&self, element_attr_value: &str) -> bool + where Impl::AttrValue: AsRef { + match *self { + AttrSelectorOperation::Exists => true, + AttrSelectorOperation::WithValue { operator, case_sensitivity, expected_value } => { + operator.eval_str(element_attr_value, expected_value.as_ref(), case_sensitivity) + } + } + } +} + +#[derive(Eq, PartialEq, Clone, Copy)] +pub enum AttrSelectorOperator { + Equal, // = + Includes, // ~= + DashMatch, // |= + Prefix, // ^= + Substring, // *= + Suffix, // $= +} + +impl AttrSelectorOperator { + pub fn eval_str(self, element_attr_value: &str, attr_selector_value: &str, + case_sensitivity: CaseSensitivity) -> bool { + let e = element_attr_value.as_bytes(); + let s = attr_selector_value.as_bytes(); + let case = case_sensitivity; + match self { + AttrSelectorOperator::Equal => { + case.eq(e, s) + } + AttrSelectorOperator::Prefix => { + e.len() >= s.len() && case.eq(&e[..s.len()], s) + } + AttrSelectorOperator::Suffix => { + e.len() >= s.len() && case.eq(&e[(e.len() - s.len())..], s) + } + AttrSelectorOperator::Substring => { + case.contains(element_attr_value, attr_selector_value) + } + AttrSelectorOperator::Includes => { + element_attr_value.split(SELECTOR_WHITESPACE) + .any(|part| case.eq(part.as_bytes(), s)) + } + AttrSelectorOperator::DashMatch => { + case.eq(e, s) || ( + e.get(s.len()) == Some(&b'-') && + case.eq(&e[..s.len()], s) + ) + } + } + } +} + +/// The definition of whitespace per CSS Selectors Level 3 § 4. +pub static SELECTOR_WHITESPACE: &'static [char] = &[' ', '\t', '\n', '\r', '\x0C']; + +#[derive(Eq, PartialEq, Clone, Copy, Debug)] +pub enum CaseSensitivity { + CaseSensitive, // Selectors spec says language-defined, but HTML says sensitive. + AsciiCaseInsensitive, +} + +impl CaseSensitivity { + pub fn eq(self, a: &[u8], b: &[u8]) -> bool { + match self { + CaseSensitivity::CaseSensitive => a == b, + CaseSensitivity::AsciiCaseInsensitive => a.eq_ignore_ascii_case(b) + } + } + + pub fn contains(self, haystack: &str, needle: &str) -> bool { + match self { + CaseSensitivity::CaseSensitive => haystack.contains(needle), + CaseSensitivity::AsciiCaseInsensitive => { + if let Some((&n_first_byte, n_rest)) = needle.as_bytes().split_first() { + haystack.bytes().enumerate().any(|(i, byte)| { + if !byte.eq_ignore_ascii_case(&n_first_byte) { + return false + } + let after_this_byte = &haystack.as_bytes()[i + 1..]; + match after_this_byte.get(..n_rest.len()) { + None => false, + Some(haystack_slice) => { + haystack_slice.eq_ignore_ascii_case(n_rest) + } + } + }) + } else { + // any_str.contains("") == true, + // though these cases should be handled with *NeverMatches and never go here. + true + } + } + } + } +} diff --git a/components/selectors/lib.rs b/components/selectors/lib.rs index d352d2f1d3c..5e7b4c99ce8 100644 --- a/components/selectors/lib.rs +++ b/components/selectors/lib.rs @@ -11,6 +11,7 @@ extern crate precomputed_hash; extern crate smallvec; pub mod arcslice; +pub mod attr; pub mod bloom; pub mod matching; pub mod parser; @@ -21,4 +22,3 @@ pub mod visitor; pub use parser::{SelectorImpl, Parser, SelectorList}; pub use tree::Element; -pub use tree::{MatchAttr, MatchAttrGeneric}; diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs index beaf0d06ed2..1ec969b08e5 100644 --- a/components/selectors/matching.rs +++ b/components/selectors/matching.rs @@ -1,9 +1,11 @@ /* This Source Code Form is subject to the terms of the Mozilla Public * 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/. */ + +use attr::{AttrSelectorOperation, AttrSelectorOperator, CaseSensitivity}; use bloom::BloomFilter; -use parser::{CaseSensitivity, Combinator, ComplexSelector, Component, LocalName}; -use parser::{Selector, SelectorInner, SelectorIter, SelectorImpl}; +use parser::{Combinator, ComplexSelector, Component, LocalName, NamespaceConstraint}; +use parser::{Selector, SelectorInner, SelectorIter, SelectorImpl, AttrSelector}; use std::borrow::Borrow; use tree::Element; @@ -410,28 +412,56 @@ fn matches_simple_selector( element.has_class(class) } Component::AttrExists(ref attr) => { - element.match_attr_has(attr) + let (ns, n) = unpack_attr_selector(element, attr); + element.attr_matches(ns, n, &AttrSelectorOperation::Exists) } Component::AttrEqual(ref attr, ref value, case_sensitivity) => { - match case_sensitivity { - CaseSensitivity::CaseSensitive => element.match_attr_equals(attr, value), - CaseSensitivity::AsciiCaseInsensitive => element.match_attr_equals_ignore_ascii_case(attr, value), - } + let (ns, n) = unpack_attr_selector(element, attr); + element.attr_matches(ns, n, &AttrSelectorOperation::WithValue { + operator: AttrSelectorOperator::Equal, + case_sensitivity: case_sensitivity, + expected_value: value + }) } Component::AttrIncludes(ref attr, ref value) => { - element.match_attr_includes(attr, value) + let (ns, n) = unpack_attr_selector(element, attr); + element.attr_matches(ns, n, &AttrSelectorOperation::WithValue { + operator: AttrSelectorOperator::Includes, + case_sensitivity: CaseSensitivity::CaseSensitive, + expected_value: value + }) } Component::AttrDashMatch(ref attr, ref value) => { - element.match_attr_dash(attr, value) + let (ns, n) = unpack_attr_selector(element, attr); + element.attr_matches(ns, n, &AttrSelectorOperation::WithValue { + operator: AttrSelectorOperator::DashMatch, + case_sensitivity: CaseSensitivity::CaseSensitive, + expected_value: value + }) } Component::AttrPrefixMatch(ref attr, ref value) => { - element.match_attr_prefix(attr, value) + let (ns, n) = unpack_attr_selector(element, attr); + element.attr_matches(ns, n, &AttrSelectorOperation::WithValue { + operator: AttrSelectorOperator::Prefix, + case_sensitivity: CaseSensitivity::CaseSensitive, + expected_value: value + }) } Component::AttrSubstringMatch(ref attr, ref value) => { - element.match_attr_substring(attr, value) + let (ns, n) = unpack_attr_selector(element, attr); + element.attr_matches(ns, n, &AttrSelectorOperation::WithValue { + operator: AttrSelectorOperator::Substring, + case_sensitivity: CaseSensitivity::CaseSensitive, + expected_value: value + }) } Component::AttrSuffixMatch(ref attr, ref value) => { - element.match_attr_suffix(attr, value) + let (ns, n) = unpack_attr_selector(element, attr); + element.attr_matches(ns, n, &AttrSelectorOperation::WithValue { + operator: AttrSelectorOperator::Suffix, + case_sensitivity: CaseSensitivity::CaseSensitive, + expected_value: value + }) } Component::AttrIncludesNeverMatch(..) | Component::AttrPrefixNeverMatch(..) | @@ -489,6 +519,18 @@ fn matches_simple_selector( } } +fn unpack_attr_selector<'a, E>(element: &E, attr: &'a AttrSelector) + -> (&'a NamespaceConstraint, + &'a ::LocalName) +where E: Element { + let name = if element.is_html_element_in_html_document() { + &attr.lower_name + } else { + &attr.name + }; + (&attr.namespace, name) +} + #[inline] fn matches_generic_nth_child(element: &E, a: i32, diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index 0c9a54defdf..96081a50b2b 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -3,6 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use arcslice::ArcSlice; +use attr::{CaseSensitivity, SELECTOR_WHITESPACE}; use cssparser::{Token, Parser as CssParser, parse_nth, ToCss, serialize_identifier, CssStringWriter}; use precomputed_hash::PrecomputedHash; use smallvec::SmallVec; @@ -13,7 +14,6 @@ use std::fmt::{self, Display, Debug, Write}; use std::iter::Rev; use std::ops::Add; use std::slice; -use tree::SELECTOR_WHITESPACE; use visitor::SelectorVisitor; /// A trait that represents a pseudo-element. @@ -560,7 +560,6 @@ pub enum Component { OnlyOfType, NonTSPseudoClass(Impl::NonTSPseudoClass), PseudoElement(Impl::PseudoElement), - // ... } impl Component { @@ -605,13 +604,6 @@ impl Component { } } -#[derive(Eq, PartialEq, Clone, Copy, Debug)] -pub enum CaseSensitivity { - CaseSensitive, // Selectors spec says language-defined, but HTML says sensitive. - AsciiCaseInsensitive, -} - - #[derive(Eq, PartialEq, Clone)] pub struct LocalName { pub name: Impl::LocalName, diff --git a/components/selectors/tree.rs b/components/selectors/tree.rs index e5a4539e3cc..42659bfc615 100644 --- a/components/selectors/tree.rs +++ b/components/selectors/tree.rs @@ -5,122 +5,13 @@ //! Traits that nodes must implement. Breaks the otherwise-cyclic dependency between layout and //! style. +use attr::AttrSelectorOperation; use matching::{ElementSelectorFlags, MatchingContext}; -use parser::{AttrSelector, SelectorImpl}; -use std::ascii::AsciiExt; +use parser::{NamespaceConstraint, SelectorImpl}; -/// The definition of whitespace per CSS Selectors Level 3 § 4. -pub static SELECTOR_WHITESPACE: &'static [char] = &[' ', '\t', '\n', '\r', '\x0C']; - -// Attribute matching routines. Consumers with simple implementations can implement -// MatchAttrGeneric instead. -pub trait MatchAttr { +pub trait Element: Sized { type Impl: SelectorImpl; - fn match_attr_has( - &self, - attr: &AttrSelector) -> bool; - - fn match_attr_equals( - &self, - attr: &AttrSelector, - value: &::AttrValue) -> bool; - - fn match_attr_equals_ignore_ascii_case( - &self, - attr: &AttrSelector, - value: &::AttrValue) -> bool; - - fn match_attr_includes( - &self, - attr: &AttrSelector, - value: &::AttrValue) -> bool; - - fn match_attr_dash( - &self, - attr: &AttrSelector, - value: &::AttrValue) -> bool; - - fn match_attr_prefix( - &self, - attr: &AttrSelector, - value: &::AttrValue) -> bool; - - fn match_attr_substring( - &self, - attr: &AttrSelector, - value: &::AttrValue) -> bool; - - fn match_attr_suffix( - &self, - attr: &AttrSelector, - value: &::AttrValue) -> bool; -} - -pub trait MatchAttrGeneric { - type Impl: SelectorImpl; - fn match_attr(&self, attr: &AttrSelector, test: F) -> bool where F: Fn(&str) -> bool; -} - -impl MatchAttr for T where T: MatchAttrGeneric, T::Impl: SelectorImpl { - type Impl = T::Impl; - - fn match_attr_has(&self, attr: &AttrSelector) -> bool { - self.match_attr(attr, |_| true) - } - - fn match_attr_equals(&self, attr: &AttrSelector, value: &String) -> bool { - self.match_attr(attr, |v| v == value) - } - - fn match_attr_equals_ignore_ascii_case(&self, attr: &AttrSelector, - value: &String) -> bool { - self.match_attr(attr, |v| v.eq_ignore_ascii_case(value)) - } - - fn match_attr_includes(&self, attr: &AttrSelector, value: &String) -> bool { - self.match_attr(attr, |attr_value| { - attr_value.split(SELECTOR_WHITESPACE).any(|v| v == value) - }) - } - - fn match_attr_dash(&self, attr: &AttrSelector, value: &String) -> bool { - self.match_attr(attr, |attr_value| { - // The attribute must start with the pattern. - if !attr_value.starts_with(value) { - return false - } - - // If the strings are the same, we're done. - if attr_value.len() == value.len() { - return true - } - - // The attribute is long than the pattern, so the next character must be '-'. - attr_value.as_bytes()[value.len()] == '-' as u8 - }) - } - - fn match_attr_prefix(&self, attr: &AttrSelector, value: &String) -> bool { - self.match_attr(attr, |attr_value| { - attr_value.starts_with(value) - }) - } - - fn match_attr_substring(&self, attr: &AttrSelector, value: &String) -> bool { - self.match_attr(attr, |attr_value| { - attr_value.contains(value) - }) - } - - fn match_attr_suffix(&self, attr: &AttrSelector, value: &String) -> bool { - self.match_attr(attr, |attr_value| { - attr_value.ends_with(value) - }) - } -} - -pub trait Element: MatchAttr + Sized { fn parent_element(&self) -> Option; /// The parent of a given pseudo-element, after matching a pseudo-element @@ -147,6 +38,12 @@ pub trait Element: MatchAttr + Sized { fn get_local_name(&self) -> &::BorrowedLocalName; fn get_namespace(&self) -> &::BorrowedNamespaceUrl; + fn attr_matches(&self, + ns: &NamespaceConstraint, + local_name: &::LocalName, + operation: &AttrSelectorOperation) + -> bool; + fn match_non_ts_pseudo_class(&self, pc: &::NonTSPseudoClass, context: &mut MatchingContext, diff --git a/components/style/attr.rs b/components/style/attr.rs index bbf8d53a430..e3df55f3d09 100644 --- a/components/style/attr.rs +++ b/components/style/attr.rs @@ -12,6 +12,8 @@ use cssparser::{self, Color, RGBA}; use euclid::num::Zero; use num_traits::ToPrimitive; use properties::PropertyDeclarationBlock; +use selector_parser::SelectorImpl; +use selectors::attr::AttrSelectorOperation; use servo_url::ServoUrl; use shared_lock::Locked; use std::ascii::AsciiExt; @@ -349,6 +351,12 @@ impl AttrValue { panic!("Uint not found"); } } + + pub fn eval_selector(&self, selector: &AttrSelectorOperation) -> bool { + // FIXME(SimonSapin) this can be more efficient by matching on `(self, selector)` variants + // and doing Atom comparisons instead of string comparisons where possible. + selector.eval_str(self) + } } impl ::std::ops::Deref for AttrValue { diff --git a/components/style/gecko/snapshot.rs b/components/style/gecko/snapshot.rs index ea4871bc2a7..33abdcc5bd9 100644 --- a/components/style/gecko/snapshot.rs +++ b/components/style/gecko/snapshot.rs @@ -8,14 +8,15 @@ use dom::TElement; use element_state::ElementState; use gecko::snapshot_helpers; -use gecko::wrapper::{AttrSelectorHelpers, GeckoElement}; +use gecko::wrapper::{NamespaceConstraintHelpers, GeckoElement}; use gecko_bindings::bindings; use gecko_bindings::structs::ServoElementSnapshot; use gecko_bindings::structs::ServoElementSnapshotFlags as Flags; use gecko_bindings::structs::ServoElementSnapshotTable; use restyle_hints::ElementSnapshot; use selector_parser::SelectorImpl; -use selectors::parser::AttrSelector; +use selectors::attr::{AttrSelectorOperation, AttrSelectorOperator, CaseSensitivity}; +use selectors::parser::NamespaceConstraint; use string_cache::Atom; /// A snapshot of a Gecko element. @@ -47,11 +48,6 @@ impl SnapshotMap { } impl GeckoElementSnapshot { - #[inline] - fn is_html_element_in_html_document(&self) -> bool { - self.mIsHTMLElementInHTMLDocument - } - #[inline] fn has_any(&self, flags: Flags) -> bool { (self.mContains as u8 & flags as u8) != 0 @@ -60,76 +56,67 @@ impl GeckoElementSnapshot { fn as_ptr(&self) -> *const Self { self } -} -impl ::selectors::MatchAttr for GeckoElementSnapshot { - type Impl = SelectorImpl; - - fn match_attr_has(&self, attr: &AttrSelector) -> bool { + /// selectors::Element::attr_matches + pub fn attr_matches(&self, + ns: &NamespaceConstraint, + local_name: &Atom, + operation: &AttrSelectorOperation) + -> bool { unsafe { - bindings::Gecko_SnapshotHasAttr(self, - attr.ns_or_null(), - attr.select_name(self.is_html_element_in_html_document())) - } - } - - fn match_attr_equals(&self, attr: &AttrSelector, value: &Atom) -> bool { - unsafe { - bindings::Gecko_SnapshotAttrEquals(self, - attr.ns_or_null(), - attr.select_name(self.is_html_element_in_html_document()), - value.as_ptr(), - /* ignoreCase = */ false) - } - } - - fn match_attr_equals_ignore_ascii_case(&self, attr: &AttrSelector, value: &Atom) -> bool { - unsafe { - bindings::Gecko_SnapshotAttrEquals(self, - attr.ns_or_null(), - attr.select_name(self.is_html_element_in_html_document()), - value.as_ptr(), - /* ignoreCase = */ true) - } - } - fn match_attr_includes(&self, attr: &AttrSelector, value: &Atom) -> bool { - unsafe { - bindings::Gecko_SnapshotAttrIncludes(self, - attr.ns_or_null(), - attr.select_name(self.is_html_element_in_html_document()), - value.as_ptr()) - } - } - fn match_attr_dash(&self, attr: &AttrSelector, value: &Atom) -> bool { - unsafe { - bindings::Gecko_SnapshotAttrDashEquals(self, - attr.ns_or_null(), - attr.select_name(self.is_html_element_in_html_document()), - value.as_ptr()) - } - } - fn match_attr_prefix(&self, attr: &AttrSelector, value: &Atom) -> bool { - unsafe { - bindings::Gecko_SnapshotAttrHasPrefix(self, - attr.ns_or_null(), - attr.select_name(self.is_html_element_in_html_document()), - value.as_ptr()) - } - } - fn match_attr_substring(&self, attr: &AttrSelector, value: &Atom) -> bool { - unsafe { - bindings::Gecko_SnapshotAttrHasSubstring(self, - attr.ns_or_null(), - attr.select_name(self.is_html_element_in_html_document()), - value.as_ptr()) - } - } - fn match_attr_suffix(&self, attr: &AttrSelector, value: &Atom) -> bool { - unsafe { - bindings::Gecko_SnapshotAttrHasSuffix(self, - attr.ns_or_null(), - attr.select_name(self.is_html_element_in_html_document()), - value.as_ptr()) + match *operation { + AttrSelectorOperation::Exists => { + bindings:: Gecko_SnapshotHasAttr(self, + ns.atom_or_null(), + local_name.as_ptr()) + } + AttrSelectorOperation::WithValue { operator, case_sensitivity, expected_value } => { + let ignore_case = match case_sensitivity { + CaseSensitivity::CaseSensitive => false, + CaseSensitivity::AsciiCaseInsensitive => true, + }; + // FIXME: case sensitivity for operators other than Equal + match operator { + AttrSelectorOperator::Equal => bindings::Gecko_SnapshotAttrEquals( + self, + ns.atom_or_null(), + local_name.as_ptr(), + expected_value.as_ptr(), + ignore_case + ), + AttrSelectorOperator::Includes => bindings::Gecko_SnapshotAttrIncludes( + self, + ns.atom_or_null(), + local_name.as_ptr(), + expected_value.as_ptr(), + ), + AttrSelectorOperator::DashMatch => bindings::Gecko_SnapshotAttrDashEquals( + self, + ns.atom_or_null(), + local_name.as_ptr(), + expected_value.as_ptr(), + ), + AttrSelectorOperator::Prefix => bindings::Gecko_SnapshotAttrHasPrefix( + self, + ns.atom_or_null(), + local_name.as_ptr(), + expected_value.as_ptr(), + ), + AttrSelectorOperator::Suffix => bindings::Gecko_SnapshotAttrHasSuffix( + self, + ns.atom_or_null(), + local_name.as_ptr(), + expected_value.as_ptr(), + ), + AttrSelectorOperator::Substring => bindings::Gecko_SnapshotAttrHasSubstring( + self, + ns.atom_or_null(), + local_name.as_ptr(), + expected_value.as_ptr(), + ), + } + } + } } } } diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 99999ecdc87..ac4452a0e03 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -64,8 +64,9 @@ use properties::style_structs::Font; use rule_tree::CascadeLevel as ServoCascadeLevel; use selector_parser::ElementExt; use selectors::Element; +use selectors::attr::{AttrSelectorOperation, AttrSelectorOperator, CaseSensitivity}; use selectors::matching::{ElementSelectorFlags, MatchingContext, MatchingMode}; -use selectors::parser::{AttrSelector, NamespaceConstraint}; +use selectors::parser::NamespaceConstraint; use shared_lock::Locked; use sink::Push; use std::cell::RefCell; @@ -1082,6 +1083,8 @@ impl<'le> PresentationalHintsSynthesizer for GeckoElement<'le> { } impl<'le> ::selectors::Element for GeckoElement<'le> { + type Impl = SelectorImpl; + fn parent_element(&self) -> Option { let parent_node = self.as_node().parent_node(); parent_node.and_then(|n| n.as_element()) @@ -1136,6 +1139,68 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { None } + fn attr_matches(&self, + ns: &NamespaceConstraint, + local_name: &Atom, + operation: &AttrSelectorOperation) + -> 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 } => { + let ignore_case = match case_sensitivity { + CaseSensitivity::CaseSensitive => false, + CaseSensitivity::AsciiCaseInsensitive => true, + }; + // FIXME: case sensitivity for operators other than Equal + match operator { + AttrSelectorOperator::Equal => bindings::Gecko_AttrEquals( + self.0, + ns.atom_or_null(), + local_name.as_ptr(), + expected_value.as_ptr(), + ignore_case + ), + AttrSelectorOperator::Includes => bindings::Gecko_AttrIncludes( + self.0, + ns.atom_or_null(), + local_name.as_ptr(), + expected_value.as_ptr(), + ), + AttrSelectorOperator::DashMatch => bindings::Gecko_AttrDashEquals( + self.0, + ns.atom_or_null(), + local_name.as_ptr(), + expected_value.as_ptr(), + ), + AttrSelectorOperator::Prefix => bindings::Gecko_AttrHasPrefix( + self.0, + ns.atom_or_null(), + local_name.as_ptr(), + expected_value.as_ptr(), + ), + AttrSelectorOperator::Suffix => bindings::Gecko_AttrHasSuffix( + self.0, + ns.atom_or_null(), + local_name.as_ptr(), + expected_value.as_ptr(), + ), + AttrSelectorOperator::Substring => bindings::Gecko_AttrHasSubstring( + self.0, + ns.atom_or_null(), + local_name.as_ptr(), + expected_value.as_ptr(), + ), + } + } + } + } + } + fn is_root(&self) -> bool { unsafe { Gecko_IsRootElement(self.0) @@ -1343,99 +1408,18 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { } /// A few helpers to help with attribute selectors and snapshotting. -pub trait AttrSelectorHelpers { +pub trait NamespaceConstraintHelpers { /// Returns the namespace of the selector, or null otherwise. - fn ns_or_null(&self) -> *mut nsIAtom; - /// Returns the proper selector name depending on whether the requesting - /// element is an HTML element in an HTML document or not. - fn select_name(&self, is_html_element_in_html_document: bool) -> *mut nsIAtom; + fn atom_or_null(&self) -> *mut nsIAtom; } -impl AttrSelectorHelpers for AttrSelector { - fn ns_or_null(&self) -> *mut nsIAtom { - match self.namespace { +impl NamespaceConstraintHelpers for NamespaceConstraint { + fn atom_or_null(&self) -> *mut nsIAtom { + match *self { NamespaceConstraint::Any => ptr::null_mut(), NamespaceConstraint::Specific(ref ns) => ns.url.0.as_ptr(), } } - - fn select_name(&self, is_html_element_in_html_document: bool) -> *mut nsIAtom { - if is_html_element_in_html_document { - self.lower_name.as_ptr() - } else { - self.name.as_ptr() - } - } -} - -impl<'le> ::selectors::MatchAttr for GeckoElement<'le> { - type Impl = SelectorImpl; - - fn match_attr_has(&self, attr: &AttrSelector) -> bool { - unsafe { - bindings::Gecko_HasAttr(self.0, - attr.ns_or_null(), - attr.select_name(self.is_html_element_in_html_document())) - } - } - fn match_attr_equals(&self, attr: &AttrSelector, value: &Atom) -> bool { - unsafe { - bindings::Gecko_AttrEquals(self.0, - attr.ns_or_null(), - attr.select_name(self.is_html_element_in_html_document()), - value.as_ptr(), - /* ignoreCase = */ false) - } - } - fn match_attr_equals_ignore_ascii_case(&self, attr: &AttrSelector, value: &Atom) -> bool { - unsafe { - bindings::Gecko_AttrEquals(self.0, - attr.ns_or_null(), - attr.select_name(self.is_html_element_in_html_document()), - value.as_ptr(), - /* ignoreCase = */ true) - } - } - fn match_attr_includes(&self, attr: &AttrSelector, value: &Atom) -> bool { - unsafe { - bindings::Gecko_AttrIncludes(self.0, - attr.ns_or_null(), - attr.select_name(self.is_html_element_in_html_document()), - value.as_ptr()) - } - } - fn match_attr_dash(&self, attr: &AttrSelector, value: &Atom) -> bool { - unsafe { - bindings::Gecko_AttrDashEquals(self.0, - attr.ns_or_null(), - attr.select_name(self.is_html_element_in_html_document()), - value.as_ptr()) - } - } - fn match_attr_prefix(&self, attr: &AttrSelector, value: &Atom) -> bool { - unsafe { - bindings::Gecko_AttrHasPrefix(self.0, - attr.ns_or_null(), - attr.select_name(self.is_html_element_in_html_document()), - value.as_ptr()) - } - } - fn match_attr_substring(&self, attr: &AttrSelector, value: &Atom) -> bool { - unsafe { - bindings::Gecko_AttrHasSubstring(self.0, - attr.ns_or_null(), - attr.select_name(self.is_html_element_in_html_document()), - value.as_ptr()) - } - } - fn match_attr_suffix(&self, attr: &AttrSelector, value: &Atom) -> bool { - unsafe { - bindings::Gecko_AttrHasSuffix(self.0, - attr.ns_or_null(), - attr.select_name(self.is_html_element_in_html_document()), - value.as_ptr()) - } - } } impl<'le> ElementExt for GeckoElement<'le> { diff --git a/components/style/restyle_hints.rs b/components/style/restyle_hints.rs index 23a3cc9f2c3..d5e2564d3de 100644 --- a/components/style/restyle_hints.rs +++ b/components/style/restyle_hints.rs @@ -7,18 +7,20 @@ #![deny(missing_docs)] use Atom; +use LocalName; use dom::TElement; use element_state::*; #[cfg(feature = "gecko")] use gecko_bindings::structs::nsRestyleHint; #[cfg(feature = "servo")] use heapsize::HeapSizeOf; -use selector_parser::{AttrValue, NonTSPseudoClass, PseudoElement, SelectorImpl, Snapshot, SnapshotMap}; -use selectors::{Element, MatchAttr}; +use selector_parser::{NonTSPseudoClass, PseudoElement, SelectorImpl, Snapshot, SnapshotMap}; +use selectors::Element; +use selectors::attr::AttrSelectorOperation; use selectors::matching::{ElementSelectorFlags, MatchingContext, MatchingMode}; use selectors::matching::matches_selector; -use selectors::parser::{AttrSelector, Combinator, Component, Selector}; -use selectors::parser::{SelectorInner, SelectorMethods}; +use selectors::parser::{Combinator, Component, Selector}; +use selectors::parser::{SelectorInner, SelectorMethods, NamespaceConstraint}; use selectors::visitor::SelectorVisitor; use smallvec::SmallVec; use std::borrow::Borrow; @@ -170,7 +172,7 @@ impl HeapSizeOf for RestyleHint { /// still need to take the ElementWrapper approach for attribute-dependent /// style. So we do it the same both ways for now to reduce complexity, but it's /// worth measuring the performance impact (if any) of the mStateMask approach. -pub trait ElementSnapshot : Sized + MatchAttr { +pub trait ElementSnapshot : Sized { /// The state of the snapshot, if any. fn state(&self) -> Option; @@ -242,90 +244,6 @@ impl<'a, E> ElementWrapper<'a, E> } } -impl<'a, E> MatchAttr for ElementWrapper<'a, E> - where E: TElement, -{ - type Impl = SelectorImpl; - - fn match_attr_has(&self, attr: &AttrSelector) -> bool { - match self.snapshot() { - Some(snapshot) if snapshot.has_attrs() - => snapshot.match_attr_has(attr), - _ => self.element.match_attr_has(attr) - } - } - - fn match_attr_equals(&self, - attr: &AttrSelector, - value: &AttrValue) -> bool { - match self.snapshot() { - Some(snapshot) if snapshot.has_attrs() - => snapshot.match_attr_equals(attr, value), - _ => self.element.match_attr_equals(attr, value) - } - } - - fn match_attr_equals_ignore_ascii_case(&self, - attr: &AttrSelector, - value: &AttrValue) -> bool { - match self.snapshot() { - Some(snapshot) if snapshot.has_attrs() - => snapshot.match_attr_equals_ignore_ascii_case(attr, value), - _ => self.element.match_attr_equals_ignore_ascii_case(attr, value) - } - } - - fn match_attr_includes(&self, - attr: &AttrSelector, - value: &AttrValue) -> bool { - match self.snapshot() { - Some(snapshot) if snapshot.has_attrs() - => snapshot.match_attr_includes(attr, value), - _ => self.element.match_attr_includes(attr, value) - } - } - - fn match_attr_dash(&self, - attr: &AttrSelector, - value: &AttrValue) -> bool { - match self.snapshot() { - Some(snapshot) if snapshot.has_attrs() - => snapshot.match_attr_dash(attr, value), - _ => self.element.match_attr_dash(attr, value) - } - } - - fn match_attr_prefix(&self, - attr: &AttrSelector, - value: &AttrValue) -> bool { - match self.snapshot() { - Some(snapshot) if snapshot.has_attrs() - => snapshot.match_attr_prefix(attr, value), - _ => self.element.match_attr_prefix(attr, value) - } - } - - fn match_attr_substring(&self, - attr: &AttrSelector, - value: &AttrValue) -> bool { - match self.snapshot() { - Some(snapshot) if snapshot.has_attrs() - => snapshot.match_attr_substring(attr, value), - _ => self.element.match_attr_substring(attr, value) - } - } - - fn match_attr_suffix(&self, - attr: &AttrSelector, - value: &AttrValue) -> bool { - match self.snapshot() { - Some(snapshot) if snapshot.has_attrs() - => snapshot.match_attr_suffix(attr, value), - _ => self.element.match_attr_suffix(attr, value) - } - } -} - #[cfg(feature = "gecko")] fn dir_selector_to_state(s: &[u16]) -> ElementState { // Jump through some hoops to deal with our Box<[u16]> thing. @@ -345,6 +263,8 @@ fn dir_selector_to_state(s: &[u16]) -> ElementState { impl<'a, E> Element for ElementWrapper<'a, E> where E: TElement, { + type Impl = SelectorImpl; + fn match_non_ts_pseudo_class(&self, pseudo_class: &NonTSPseudoClass, context: &mut MatchingContext, @@ -450,6 +370,19 @@ impl<'a, E> Element for ElementWrapper<'a, E> self.element.get_namespace() } + fn attr_matches(&self, + ns: &NamespaceConstraint, + local_name: &LocalName, + operation: &AttrSelectorOperation) + -> bool { + match self.snapshot() { + Some(snapshot) if snapshot.has_attrs() => { + snapshot.attr_matches(ns, local_name, operation) + } + _ => self.element.attr_matches(ns, local_name, operation) + } + } + fn get_id(&self) -> Option { match self.snapshot() { Some(snapshot) if snapshot.has_attrs() diff --git a/components/style/servo/selector_parser.rs b/components/style/servo/selector_parser.rs index 6fd498ee9aa..f86a9f16469 100644 --- a/components/style/servo/selector_parser.rs +++ b/components/style/servo/selector_parser.rs @@ -14,9 +14,10 @@ use element_state::ElementState; use fnv::FnvHashMap; use restyle_hints::ElementSnapshot; use selector_parser::{ElementExt, PseudoElementCascadeType, SelectorParser}; -use selectors::{Element, MatchAttrGeneric}; +use selectors::{Element}; +use selectors::attr::AttrSelectorOperation; use selectors::matching::{MatchingContext, MatchingMode}; -use selectors::parser::{AttrSelector, SelectorMethods}; +use selectors::parser::{SelectorMethods, NamespaceConstraint}; use selectors::visitor::SelectorVisitor; use std::borrow::Cow; use std::fmt; @@ -519,10 +520,10 @@ impl ServoElementSnapshot { .map(|&(_, ref v)| v) } - fn get_attr_ignore_ns(&self, name: &LocalName) -> Option<&AttrValue> { + fn any_attr_ignore_ns(&self, name: &LocalName, mut f: F) -> bool + where F: FnMut(&AttrValue) -> bool { self.attrs.as_ref().unwrap().iter() - .find(|&&(ref ident, _)| ident.local_name == *name) - .map(|&(_, ref v)| v) + .any(|&(ref ident, ref v)| ident.local_name == *name && f(v)) } } @@ -555,19 +556,23 @@ impl ElementSnapshot for ServoElementSnapshot { } } -impl MatchAttrGeneric for ServoElementSnapshot { - type Impl = SelectorImpl; - - fn match_attr(&self, attr: &AttrSelector, test: F) -> bool - where F: Fn(&str) -> bool - { +impl ServoElementSnapshot { + /// selectors::Element::attr_matches + pub fn attr_matches(&self, + ns: &NamespaceConstraint, + local_name: &LocalName, + operation: &AttrSelectorOperation) + -> bool { use selectors::parser::NamespaceConstraint; - let html = self.is_html_element_in_html_document; - let local_name = if html { &attr.lower_name } else { &attr.name }; - match attr.namespace { - NamespaceConstraint::Specific(ref ns) => self.get_attr(&ns.url, local_name), - NamespaceConstraint::Any => self.get_attr_ignore_ns(local_name), - }.map_or(false, |v| test(v)) + match *ns { + NamespaceConstraint::Specific(ref ns) => { + self.get_attr(&ns.url, local_name) + .map_or(false, |value| value.eval_selector(operation)) + } + NamespaceConstraint::Any => { + self.any_attr_ignore_ns(local_name, |value| value.eval_selector(operation)) + } + } } } diff --git a/servo-tidy.toml b/servo-tidy.toml index 1b07e242ff9..55357dfe1ce 100644 --- a/servo-tidy.toml +++ b/servo-tidy.toml @@ -56,9 +56,6 @@ files = [ "./tests/wpt/mozilla/tests/css/fonts", "./tests/wpt/mozilla/tests/css/pre_with_tab.html", "./tests/wpt/mozilla/tests/mozilla/textarea_placeholder.html", - # Tidy complains about taking &String instead of &str, but they aren't - # equivalent given the way the traits are set up. - "./components/selectors/tree.rs", ] # Directories that are ignored for the non-WPT tidy check. directories = [ diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index fde49c6acb9..52030bc21ac 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -6,6 +6,7 @@ use cssparser::{self, Parser as CssParser, SourcePosition, SourceLocation}; use html5ever::{Namespace as NsAtom}; use media_queries::CSSErrorReporterTest; use parking_lot::RwLock; +use selectors::attr::CaseSensitivity; use selectors::parser::*; use servo_atoms::Atom; use servo_url::ServoUrl; From 9376abdd2c0ca75cdb351f5565964908e1e2446a Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 17 May 2017 22:21:00 +0200 Subject: [PATCH 04/12] Shrink selectors::Component, add case-insensitive for other attr selectors * https://bugzilla.mozilla.org/show_bug.cgi?id=1364148 * https://bugzilla.mozilla.org/show_bug.cgi?id=1364162 --- components/script/dom/element.rs | 9 +- components/script/layout_wrapper.rs | 15 +- components/selectors/attr.rs | 67 +++- components/selectors/matching.rs | 134 ++++---- components/selectors/parser.rs | 368 +++++++++++----------- components/selectors/size_of_tests.rs | 3 +- components/selectors/tree.rs | 20 +- components/selectors/visitor.rs | 11 +- components/style/attr.rs | 6 +- components/style/gecko/snapshot.rs | 10 +- components/style/gecko/wrapper.rs | 11 +- components/style/restyle_hints.rs | 22 +- components/style/servo/selector_parser.rs | 13 +- components/style/stylist.rs | 27 +- tests/unit/style/stylesheets.rs | 18 +- 15 files changed, 377 insertions(+), 357 deletions(-) diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 9c6d84ba485..bdd2b49a751 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -86,10 +86,9 @@ use net_traits::request::CorsSettings; use ref_filter_map::ref_filter_map; use script_layout_interface::message::ReflowQueryType; use script_thread::Runnable; -use selectors::attr::AttrSelectorOperation; +use selectors::attr::{AttrSelectorOperation, NamespaceConstraint}; use selectors::matching::{ElementSelectorFlags, MatchingContext, MatchingMode, matches_selector_list}; use selectors::matching::{HAS_EDGE_CHILD_SELECTOR, HAS_SLOW_SELECTOR, HAS_SLOW_SELECTOR_LATER_SIBLINGS}; -use selectors::parser::NamespaceConstraint; use servo_atoms::Atom; use std::ascii::AsciiExt; use std::borrow::Cow; @@ -2387,13 +2386,13 @@ impl<'a> ::selectors::Element for Root { } fn attr_matches(&self, - ns: &NamespaceConstraint, + ns: &NamespaceConstraint<&Namespace>, local_name: &LocalName, - operation: &AttrSelectorOperation) + operation: &AttrSelectorOperation<&String>) -> bool { match *ns { NamespaceConstraint::Specific(ref ns) => { - self.get_attribute(&ns.url, local_name) + self.get_attribute(ns, local_name) .map_or(false, |attr| attr.value().eval_selector(operation)) } NamespaceConstraint::Any => { diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index 773937fce00..ac59921d2ea 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -50,9 +50,8 @@ use script_layout_interface::{HTMLCanvasData, LayoutNodeType, SVGSVGData, Truste use script_layout_interface::{OpaqueStyleAndLayoutData, PartialPersistentLayoutData}; use script_layout_interface::wrapper_traits::{DangerousThreadSafeLayoutNode, GetLayoutData, LayoutNode}; use script_layout_interface::wrapper_traits::{PseudoElementType, ThreadSafeLayoutElement, ThreadSafeLayoutNode}; -use selectors::attr::AttrSelectorOperation; +use selectors::attr::{AttrSelectorOperation, NamespaceConstraint}; use selectors::matching::{ElementSelectorFlags, MatchingContext}; -use selectors::parser::NamespaceConstraint; use servo_atoms::Atom; use servo_url::ServoUrl; use std::fmt; @@ -606,13 +605,13 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { } fn attr_matches(&self, - ns: &NamespaceConstraint, + ns: &NamespaceConstraint<&Namespace>, local_name: &LocalName, - operation: &AttrSelectorOperation) + operation: &AttrSelectorOperation<&String>) -> bool { match *ns { NamespaceConstraint::Specific(ref ns) => { - self.get_attr_enum(&ns.url, local_name) + self.get_attr_enum(ns, local_name) .map_or(false, |value| value.eval_selector(operation)) } NamespaceConstraint::Any => { @@ -1159,13 +1158,13 @@ impl<'le> ::selectors::Element for ServoThreadSafeLayoutElement<'le> { } fn attr_matches(&self, - ns: &NamespaceConstraint, + ns: &NamespaceConstraint<&Namespace>, local_name: &LocalName, - operation: &AttrSelectorOperation) + operation: &AttrSelectorOperation<&String>) -> bool { match *ns { NamespaceConstraint::Specific(ref ns) => { - self.get_attr_enum(&ns.url, local_name) + self.get_attr_enum(ns, local_name) .map_or(false, |value| value.eval_selector(operation)) } NamespaceConstraint::Any => { diff --git a/components/selectors/attr.rs b/components/selectors/attr.rs index 90707c54351..f18219b2204 100644 --- a/components/selectors/attr.rs +++ b/components/selectors/attr.rs @@ -2,24 +2,54 @@ * 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/. */ +use cssparser::ToCss; use parser::SelectorImpl; use std::ascii::AsciiExt; +use std::fmt; -pub enum AttrSelectorOperation<'a, Impl: SelectorImpl> where Impl::AttrValue: 'a { +#[derive(Eq, PartialEq, Clone)] +pub struct AttrSelectorWithNamespace { + pub namespace: NamespaceConstraint<(Impl::NamespacePrefix, Impl::NamespaceUrl)>, + pub local_name: Impl::LocalName, + pub local_name_lower: Impl::LocalName, + pub operation: AttrSelectorOperation, + pub never_matches: bool, +} + +impl AttrSelectorWithNamespace { + pub fn namespace(&self) -> NamespaceConstraint<&Impl::NamespaceUrl> { + match self.namespace { + NamespaceConstraint::Any => NamespaceConstraint::Any, + NamespaceConstraint::Specific((_, ref url)) => { + NamespaceConstraint::Specific(url) + } + } + } +} + +#[derive(Eq, PartialEq, Clone)] +pub enum NamespaceConstraint { + Any, + + /// Empty string for no namespace + Specific(NamespaceUrl), +} + +#[derive(Eq, PartialEq, Clone)] +pub enum AttrSelectorOperation { Exists, WithValue { operator: AttrSelectorOperator, case_sensitivity: CaseSensitivity, - expected_value: &'a Impl::AttrValue, + expected_value: AttrValue, } } -impl<'a, Impl: SelectorImpl> AttrSelectorOperation<'a, Impl> { - pub fn eval_str(&self, element_attr_value: &str) -> bool - where Impl::AttrValue: AsRef { +impl AttrSelectorOperation { + pub fn eval_str(&self, element_attr_value: &str) -> bool where AttrValue: AsRef { match *self { AttrSelectorOperation::Exists => true, - AttrSelectorOperation::WithValue { operator, case_sensitivity, expected_value } => { + AttrSelectorOperation::WithValue { operator, case_sensitivity, ref expected_value } => { operator.eval_str(element_attr_value, expected_value.as_ref(), case_sensitivity) } } @@ -28,12 +58,25 @@ impl<'a, Impl: SelectorImpl> AttrSelectorOperation<'a, Impl> { #[derive(Eq, PartialEq, Clone, Copy)] pub enum AttrSelectorOperator { - Equal, // = - Includes, // ~= - DashMatch, // |= - Prefix, // ^= - Substring, // *= - Suffix, // $= + Equal, + Includes, + DashMatch, + Prefix, + Substring, + Suffix, +} + +impl ToCss for AttrSelectorOperator { + fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + dest.write_str(match *self { + AttrSelectorOperator::Equal => " = ", + AttrSelectorOperator::Includes => " ~= ", + AttrSelectorOperator::DashMatch => " |= ", + AttrSelectorOperator::Prefix => " ^= ", + AttrSelectorOperator::Substring => " *= ", + AttrSelectorOperator::Suffix => " $= ", + }) + } } impl AttrSelectorOperator { diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs index 1ec969b08e5..a11708818b3 100644 --- a/components/selectors/matching.rs +++ b/components/selectors/matching.rs @@ -2,10 +2,10 @@ * 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/. */ -use attr::{AttrSelectorOperation, AttrSelectorOperator, CaseSensitivity}; +use attr::{AttrSelectorOperation, NamespaceConstraint}; use bloom::BloomFilter; -use parser::{Combinator, ComplexSelector, Component, LocalName, NamespaceConstraint}; -use parser::{Selector, SelectorInner, SelectorIter, SelectorImpl, AttrSelector}; +use parser::{Combinator, ComplexSelector, Component, LocalName}; +use parser::{Selector, SelectorInner, SelectorIter, SelectorImpl}; use std::borrow::Borrow; use tree::Element; @@ -387,8 +387,7 @@ fn matches_simple_selector( element.match_pseudo_element(pseudo, context) } Component::LocalName(LocalName { ref name, ref lower_name }) => { - let name = if element.is_html_element_in_html_document() { lower_name } else { name }; - element.get_local_name() == name.borrow() + element.get_local_name() == select_name(element, name, lower_name).borrow() } Component::ExplicitUniversalType | Component::ExplicitAnyNamespace => { @@ -399,9 +398,8 @@ fn matches_simple_selector( element.get_namespace() == url.borrow() } Component::ExplicitNoNamespace => { - // Rust type’s default, not default namespace - let empty_string = ::NamespaceUrl::default(); - element.get_namespace() == empty_string.borrow() + let ns = ::parser::namespace_empty_string::(); + element.get_namespace() == ns.borrow() } // TODO: case-sensitivity depends on the document type and quirks mode Component::ID(ref id) => { @@ -411,63 +409,58 @@ fn matches_simple_selector( Component::Class(ref class) => { element.has_class(class) } - Component::AttrExists(ref attr) => { - let (ns, n) = unpack_attr_selector(element, attr); - element.attr_matches(ns, n, &AttrSelectorOperation::Exists) + Component::AttributeInNoNamespaceExists { ref local_name, ref local_name_lower } => { + element.attr_matches( + &NamespaceConstraint::Specific(&::parser::namespace_empty_string::()), + select_name(element, local_name, local_name_lower), + &AttrSelectorOperation::Exists + ) } - Component::AttrEqual(ref attr, ref value, case_sensitivity) => { - let (ns, n) = unpack_attr_selector(element, attr); - element.attr_matches(ns, n, &AttrSelectorOperation::WithValue { - operator: AttrSelectorOperator::Equal, - case_sensitivity: case_sensitivity, - expected_value: value - }) + Component::AttributeInNoNamespace { + ref local_name, + ref local_name_lower, + ref value, + operator, + case_sensitivity, + never_matches, + } => { + if never_matches { + false + } else { + element.attr_matches( + &NamespaceConstraint::Specific(&::parser::namespace_empty_string::()), + select_name(element, local_name, local_name_lower), + &AttrSelectorOperation::WithValue { + operator: operator, + case_sensitivity: case_sensitivity, + expected_value: value, + } + ) + } } - Component::AttrIncludes(ref attr, ref value) => { - let (ns, n) = unpack_attr_selector(element, attr); - element.attr_matches(ns, n, &AttrSelectorOperation::WithValue { - operator: AttrSelectorOperator::Includes, - case_sensitivity: CaseSensitivity::CaseSensitive, - expected_value: value - }) - } - Component::AttrDashMatch(ref attr, ref value) => { - let (ns, n) = unpack_attr_selector(element, attr); - element.attr_matches(ns, n, &AttrSelectorOperation::WithValue { - operator: AttrSelectorOperator::DashMatch, - case_sensitivity: CaseSensitivity::CaseSensitive, - expected_value: value - }) - } - Component::AttrPrefixMatch(ref attr, ref value) => { - let (ns, n) = unpack_attr_selector(element, attr); - element.attr_matches(ns, n, &AttrSelectorOperation::WithValue { - operator: AttrSelectorOperator::Prefix, - case_sensitivity: CaseSensitivity::CaseSensitive, - expected_value: value - }) - } - Component::AttrSubstringMatch(ref attr, ref value) => { - let (ns, n) = unpack_attr_selector(element, attr); - element.attr_matches(ns, n, &AttrSelectorOperation::WithValue { - operator: AttrSelectorOperator::Substring, - case_sensitivity: CaseSensitivity::CaseSensitive, - expected_value: value - }) - } - Component::AttrSuffixMatch(ref attr, ref value) => { - let (ns, n) = unpack_attr_selector(element, attr); - element.attr_matches(ns, n, &AttrSelectorOperation::WithValue { - operator: AttrSelectorOperator::Suffix, - case_sensitivity: CaseSensitivity::CaseSensitive, - expected_value: value - }) - } - Component::AttrIncludesNeverMatch(..) | - Component::AttrPrefixNeverMatch(..) | - Component::AttrSubstringNeverMatch(..) | - Component::AttrSuffixNeverMatch(..) => { - false + Component::AttributeOther(ref attr_sel) => { + if attr_sel.never_matches { + return false + } else { + element.attr_matches( + &attr_sel.namespace(), + select_name(element, &attr_sel.local_name, &attr_sel.local_name_lower), + &match attr_sel.operation { + AttrSelectorOperation::Exists => AttrSelectorOperation::Exists, + AttrSelectorOperation::WithValue { + operator, + case_sensitivity, + ref expected_value, + } => { + AttrSelectorOperation::WithValue { + operator: operator, + case_sensitivity: case_sensitivity, + expected_value: expected_value, + } + } + } + ) + } } Component::NonTSPseudoClass(ref pc) => { element.match_non_ts_pseudo_class(pc, context, flags_setter) @@ -519,16 +512,15 @@ fn matches_simple_selector( } } -fn unpack_attr_selector<'a, E>(element: &E, attr: &'a AttrSelector) - -> (&'a NamespaceConstraint, - &'a ::LocalName) +fn select_name<'a, E>(element: &E, local_name: &'a ::LocalName, + local_name_lower: &'a ::LocalName) + -> &'a ::LocalName where E: Element { - let name = if element.is_html_element_in_html_document() { - &attr.lower_name + if element.is_html_element_in_html_document() { + local_name_lower } else { - &attr.name - }; - (&attr.namespace, name) + local_name + } } #[inline] diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index 96081a50b2b..befbfd084cc 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -3,7 +3,8 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use arcslice::ArcSlice; -use attr::{CaseSensitivity, SELECTOR_WHITESPACE}; +use attr::{AttrSelectorWithNamespace, AttrSelectorOperation, AttrSelectorOperator}; +use attr::{CaseSensitivity, SELECTOR_WHITESPACE, NamespaceConstraint}; use cssparser::{Token, Parser as CssParser, parse_nth, ToCss, serialize_identifier, CssStringWriter}; use precomputed_hash::PrecomputedHash; use smallvec::SmallVec; @@ -313,18 +314,28 @@ impl SelectorMethods for Component { return false; } } - }, - AttrExists(ref selector) | - AttrEqual(ref selector, _, _) | - AttrIncludes(ref selector, _) | - AttrDashMatch(ref selector, _) | - AttrPrefixMatch(ref selector, _) | - AttrSubstringMatch(ref selector, _) | - AttrSuffixMatch(ref selector, _) => { - if !visitor.visit_attribute_selector(selector) { + } + + AttributeInNoNamespace { ref local_name, ref local_name_lower, .. } | + AttributeInNoNamespaceExists { ref local_name, ref local_name_lower } => { + if !visitor.visit_attribute_selector( + &NamespaceConstraint::Specific(&namespace_empty_string::()), + local_name, + local_name_lower, + ) { return false; } } + AttributeOther(ref attr_selector) => { + if !visitor.visit_attribute_selector( + &attr_selector.namespace(), + &attr_selector.local_name, + &attr_selector.local_name_lower, + ) { + return false; + } + } + NonTSPseudoClass(ref pseudo_class) => { if !pseudo_class.visit(visitor) { return false; @@ -337,6 +348,11 @@ impl SelectorMethods for Component { } } +pub fn namespace_empty_string() -> Impl::NamespaceUrl { + // Rust type’s default, not default namespace + Impl::NamespaceUrl::default() +} + /// A ComplexSelectors stores a sequence of simple selectors and combinators. The /// iterator classes allow callers to iterate at either the raw sequence level or /// at the level of sequences of simple selectors separated by combinators. Most @@ -523,19 +539,20 @@ pub enum Component { ID(Impl::Identifier), Class(Impl::ClassName), - // Attribute selectors - AttrExists(AttrSelector), // [foo] - AttrEqual(AttrSelector, Impl::AttrValue, CaseSensitivity), // [foo=bar] - AttrIncludes(AttrSelector, Impl::AttrValue), // [foo~=bar] - AttrDashMatch(AttrSelector, Impl::AttrValue), // [foo|=bar] - AttrPrefixMatch(AttrSelector, Impl::AttrValue), // [foo^=bar] - AttrSubstringMatch(AttrSelector, Impl::AttrValue), // [foo*=bar] - AttrSuffixMatch(AttrSelector, Impl::AttrValue), // [foo$=bar] - - AttrIncludesNeverMatch(AttrSelector, Impl::AttrValue), // empty value or with whitespace - AttrPrefixNeverMatch(AttrSelector, Impl::AttrValue), // empty value - AttrSubstringNeverMatch(AttrSelector, Impl::AttrValue), // empty value - AttrSuffixNeverMatch(AttrSelector, Impl::AttrValue), // empty value + AttributeInNoNamespaceExists { + local_name: Impl::LocalName, + local_name_lower: Impl::LocalName, + }, + AttributeInNoNamespace { + local_name: Impl::LocalName, + local_name_lower: Impl::LocalName, + operator: AttrSelectorOperator, + value: Impl::AttrValue, + case_sensitivity: CaseSensitivity, + never_matches: bool, + }, + // Use a Box in the less common cases with more data to keep size_of::() small. + AttributeOther(Box>), // Pseudo-classes // @@ -610,35 +627,6 @@ pub struct LocalName { pub lower_name: Impl::LocalName, } -#[derive(Eq, PartialEq, Clone)] -pub struct AttrSelector { - pub name: Impl::LocalName, - pub lower_name: Impl::LocalName, - pub namespace: NamespaceConstraint, -} - -#[derive(Eq, PartialEq, Clone, Debug)] -pub enum NamespaceConstraint { - Any, - Specific(Namespace), -} - -#[derive(Eq, PartialEq, Clone)] -pub struct Namespace { - pub prefix: Option, - pub url: Impl::NamespaceUrl, -} - -impl Default for Namespace { - fn default() -> Self { - Namespace { - prefix: None, - url: Impl::NamespaceUrl::default(), // empty string - } - } -} - - impl Debug for Selector { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.write_str("Selector(")?; @@ -656,10 +644,7 @@ impl Debug for ComplexSelector { impl Debug for Component { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { self.to_css(f) } } -impl Debug for AttrSelector { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { self.to_css(f) } -} -impl Debug for Namespace { +impl Debug for AttrSelectorWithNamespace { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { self.to_css(f) } } impl Debug for LocalName { @@ -737,27 +722,25 @@ impl ToCss for Component { dest.write_char('|') } - // Attribute selectors - AttrExists(ref a) => { + AttributeInNoNamespaceExists { ref local_name, .. } => { dest.write_char('[')?; - a.to_css(dest)?; + display_to_css_identifier(local_name, dest)?; dest.write_char(']') } - AttrEqual(ref a, ref v, case) => { - attr_selector_to_css(a, " = ", v, match case { - CaseSensitivity::CaseSensitive => None, - CaseSensitivity::AsciiCaseInsensitive => Some(" i"), - }, dest) + AttributeInNoNamespace { ref local_name, operator, ref value, case_sensitivity, .. } => { + dest.write_char('[')?; + display_to_css_identifier(local_name, dest)?; + operator.to_css(dest)?; + dest.write_char('"')?; + write!(CssStringWriter::new(dest), "{}", value)?; + dest.write_char('"')?; + match case_sensitivity { + CaseSensitivity::CaseSensitive => {}, + CaseSensitivity::AsciiCaseInsensitive => dest.write_str(" i")?, + } + dest.write_char(']') } - AttrDashMatch(ref a, ref v) => attr_selector_to_css(a, " |= ", v, None, dest), - AttrIncludesNeverMatch(ref a, ref v) | - AttrIncludes(ref a, ref v) => attr_selector_to_css(a, " ~= ", v, None, dest), - AttrPrefixNeverMatch(ref a, ref v) | - AttrPrefixMatch(ref a, ref v) => attr_selector_to_css(a, " ^= ", v, None, dest), - AttrSubstringNeverMatch(ref a, ref v) | - AttrSubstringMatch(ref a, ref v) => attr_selector_to_css(a, " *= ", v, None, dest), - AttrSuffixNeverMatch(ref a, ref v) | - AttrSuffixMatch(ref a, ref v) => attr_selector_to_css(a, " $= ", v, None, dest), + AttributeOther(ref attr_selector) => attr_selector.to_css(dest), // Pseudo-classes Negation(ref arg) => { @@ -786,22 +769,33 @@ impl ToCss for Component { } } -impl ToCss for AttrSelector { +impl ToCss for AttrSelectorWithNamespace { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - if let NamespaceConstraint::Specific(ref ns) = self.namespace { - ns.to_css(dest)?; + dest.write_char('[')?; + match self.namespace { + NamespaceConstraint::Specific((ref prefix, _)) => { + display_to_css_identifier(prefix, dest)?; + dest.write_char('|')? + } + NamespaceConstraint::Any => { + dest.write_str("*|")? + } } - display_to_css_identifier(&self.name, dest) - } -} - -impl ToCss for Namespace { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - if let Some(ref prefix) = self.prefix { - display_to_css_identifier(prefix, dest)?; - dest.write_char('|')?; + display_to_css_identifier(&self.local_name, dest)?; + match self.operation { + AttrSelectorOperation::Exists => {}, + AttrSelectorOperation::WithValue { operator, case_sensitivity, ref expected_value } => { + operator.to_css(dest)?; + dest.write_char('"')?; + write!(CssStringWriter::new(dest), "{}", expected_value)?; + dest.write_char('"')?; + match case_sensitivity { + CaseSensitivity::CaseSensitive => {}, + CaseSensitivity::AsciiCaseInsensitive => dest.write_str(" i")?, + } + }, } - Ok(()) + dest.write_char(']') } } @@ -811,26 +805,6 @@ impl ToCss for LocalName { } } -fn attr_selector_to_css(attr: &AttrSelector, - operator: &str, - value: &Impl::AttrValue, - modifier: Option<&str>, - dest: &mut W) - -> fmt::Result -where Impl: SelectorImpl, W: fmt::Write -{ - dest.write_char('[')?; - attr.to_css(dest)?; - dest.write_str(operator)?; - dest.write_char('"')?; - write!(CssStringWriter::new(dest), "{}", value)?; - dest.write_char('"')?; - if let Some(m) = modifier { - dest.write_str(m)?; - } - dest.write_char(']') -} - /// Serialize the output of Display as a CSS identifier fn display_to_css_identifier(x: &T, dest: &mut W) -> fmt::Result { // FIXME(SimonSapin): it is possible to avoid this heap allocation @@ -926,18 +900,9 @@ fn complex_selector_specificity(selector: &ComplexSelector) specificity.id_selectors += 1 } Component::Class(..) | - Component::AttrExists(..) | - Component::AttrEqual(..) | - Component::AttrIncludes(..) | - Component::AttrDashMatch(..) | - Component::AttrPrefixMatch(..) | - Component::AttrSubstringMatch(..) | - Component::AttrSuffixMatch(..) | - - Component::AttrIncludesNeverMatch(..) | - Component::AttrPrefixNeverMatch(..) | - Component::AttrSubstringNeverMatch(..) | - Component::AttrSuffixNeverMatch(..) | + Component::AttributeInNoNamespace { .. } | + Component::AttributeInNoNamespaceExists { .. } | + Component::AttributeOther(..) | Component::FirstChild | Component::LastChild | Component::OnlyChild | Component::Root | @@ -1223,89 +1188,118 @@ fn parse_attribute_selector(parser: &P, input: &mut CssParser) -> Result, ()> where P: Parser, Impl: SelectorImpl { - let attr = match parse_qualified_name(parser, input, /* in_attr_selector = */ true)? { + let namespace; + let local_name; + let local_name_lower; + match parse_qualified_name(parser, input, /* in_attr_selector = */ true)? { None => return Err(()), Some((_, None)) => unreachable!(), - Some((namespace, Some(local_name))) => AttrSelector { - namespace: match namespace { + Some((ns, Some(ln))) => { + local_name_lower = from_ascii_lowercase(&ln); + local_name = from_cow_str(ln); + namespace = match ns { QNamePrefix::ImplicitNoNamespace | QNamePrefix::ExplicitNoNamespace => { - NamespaceConstraint::Specific(Namespace { - prefix: None, - url: Impl::NamespaceUrl::default(), - }) + None } QNamePrefix::ExplicitNamespace(prefix, url) => { - NamespaceConstraint::Specific(Namespace { - prefix: Some(prefix), - url: url, - }) + Some(NamespaceConstraint::Specific((prefix, url))) } QNamePrefix::ExplicitAnyNamespace => { - NamespaceConstraint::Any + Some(NamespaceConstraint::Any) } QNamePrefix::ImplicitAnyNamespace | QNamePrefix::ImplicitDefaultNamespace(_) => { unreachable!() // Not returned with in_attr_selector = true } - }, - lower_name: from_ascii_lowercase(&local_name), - name: from_cow_str(local_name), - }, - }; + } + } + } + let operator; + let value; + let never_matches; match input.next() { // [foo] - Err(()) => Ok(Component::AttrExists(attr)), + Err(()) => { + if let Some(namespace) = namespace { + return Ok(Component::AttributeOther(Box::new(AttrSelectorWithNamespace { + namespace: namespace, + local_name: local_name, + local_name_lower: local_name_lower, + operation: AttrSelectorOperation::Exists, + never_matches: false, + }))) + } else { + return Ok(Component::AttributeInNoNamespaceExists { + local_name: local_name, + local_name_lower: local_name_lower, + }) + } + } // [foo=bar] Ok(Token::Delim('=')) => { - let value = input.expect_ident_or_string()?; - let flags = parse_attribute_flags(input)?; - Ok(Component::AttrEqual(attr, from_cow_str(value), flags)) + value = input.expect_ident_or_string()?; + never_matches = false; + operator = AttrSelectorOperator::Equal; } // [foo~=bar] Ok(Token::IncludeMatch) => { - let value = input.expect_ident_or_string()?; - if value.is_empty() || value.contains(SELECTOR_WHITESPACE) { - Ok(Component::AttrIncludesNeverMatch(attr, from_cow_str(value))) - } else { - Ok(Component::AttrIncludes(attr, from_cow_str(value))) - } + value = input.expect_ident_or_string()?; + never_matches = value.is_empty() || value.contains(SELECTOR_WHITESPACE); + operator = AttrSelectorOperator::Includes; } // [foo|=bar] Ok(Token::DashMatch) => { - let value = input.expect_ident_or_string()?; - Ok(Component::AttrDashMatch(attr, from_cow_str(value))) + value = input.expect_ident_or_string()?; + never_matches = false; + operator = AttrSelectorOperator::DashMatch; } // [foo^=bar] Ok(Token::PrefixMatch) => { - let value = input.expect_ident_or_string()?; - if value.is_empty() { - Ok(Component::AttrPrefixNeverMatch(attr, from_cow_str(value))) - } else { - Ok(Component::AttrPrefixMatch(attr, from_cow_str(value))) - } + value = input.expect_ident_or_string()?; + never_matches = value.is_empty(); + operator = AttrSelectorOperator::Prefix; } // [foo*=bar] Ok(Token::SubstringMatch) => { - let value = input.expect_ident_or_string()?; - if value.is_empty() { - Ok(Component::AttrSubstringNeverMatch(attr, from_cow_str(value))) - } else { - Ok(Component::AttrSubstringMatch(attr, from_cow_str(value))) - } + value = input.expect_ident_or_string()?; + never_matches = value.is_empty(); + operator = AttrSelectorOperator::Substring; } // [foo$=bar] Ok(Token::SuffixMatch) => { - let value = input.expect_ident_or_string()?; - if value.is_empty() { - Ok(Component::AttrSuffixNeverMatch(attr, from_cow_str(value))) - } else { - Ok(Component::AttrSuffixMatch(attr, from_cow_str(value))) - } + value = input.expect_ident_or_string()?; + never_matches = value.is_empty(); + operator = AttrSelectorOperator::Suffix; } - _ => Err(()) + _ => return Err(()) + } + + let case_sensitivity = parse_attribute_flags(input)?; + let value = from_cow_str(value); + if let Some(namespace) = namespace { + Ok(Component::AttributeOther(Box::new(AttrSelectorWithNamespace { + namespace: namespace, + local_name: local_name, + local_name_lower: local_name_lower, + never_matches: never_matches, + operation: AttrSelectorOperation::WithValue { + operator: operator, + case_sensitivity: case_sensitivity, + expected_value: value, + } + }))) + } else { + Ok(Component::AttributeInNoNamespace { + local_name: local_name, + local_name_lower: local_name_lower, + operator: operator, + value: value, + case_sensitivity: case_sensitivity, + never_matches: never_matches, + }) } } @@ -1845,14 +1839,12 @@ pub mod tests { // https://github.com/mozilla/servo/pull/1652 let mut parser = DummyParser::default(); assert_eq!(parse_ns("[Foo]", &parser), Ok(SelectorList(vec!(Selector { - inner: SelectorInner::from_vec(vec!( - Component::AttrExists(AttrSelector { - name: DummyAtom::from("Foo"), - lower_name: DummyAtom::from("foo"), - namespace: NamespaceConstraint::Specific(Namespace { - prefix: None, - url: "".into(), - }) }))), + inner: SelectorInner::from_vec(vec![ + Component::AttributeInNoNamespaceExists { + local_name: DummyAtom::from("Foo"), + local_name_lower: DummyAtom::from("foo"), + } + ]), specificity_and_flags: specificity(0, 1, 0), })))); assert_eq!(parse_ns("svg|circle", &parser), Err(())); @@ -1885,14 +1877,10 @@ pub mod tests { inner: SelectorInner::from_vec( vec![ Component::DefaultNamespace(MATHML.into()), - Component::AttrExists(AttrSelector { - name: DummyAtom::from("Foo"), - lower_name: DummyAtom::from("foo"), - namespace: NamespaceConstraint::Specific(Namespace { - prefix: None, - url: "".into(), - }), - }), + Component::AttributeInNoNamespaceExists { + local_name: DummyAtom::from("Foo"), + local_name_lower: DummyAtom::from("foo"), + }, ]), specificity_and_flags: specificity(0, 1, 0), })))); @@ -1960,14 +1948,14 @@ pub mod tests { assert_eq!(parse("[attr |= \"foo\"]"), Ok(SelectorList(vec![Selector { inner: SelectorInner::from_vec( vec![ - Component::AttrDashMatch(AttrSelector { - name: DummyAtom::from("attr"), - lower_name: DummyAtom::from("attr"), - namespace: NamespaceConstraint::Specific(Namespace { - prefix: None, - url: "".into(), - }), - }, DummyAtom::from("foo")) + Component::AttributeInNoNamespace { + local_name: DummyAtom::from("attr"), + local_name_lower: DummyAtom::from("attr"), + operator: AttrSelectorOperator::DashMatch, + value: DummyAtom::from("foo"), + never_matches: false, + case_sensitivity: CaseSensitivity::CaseSensitive, + } ]), specificity_and_flags: specificity(0, 1, 0), }]))); diff --git a/components/selectors/size_of_tests.rs b/components/selectors/size_of_tests.rs index c03a0c80db5..5fec3085fcb 100644 --- a/components/selectors/size_of_tests.rs +++ b/components/selectors/size_of_tests.rs @@ -16,8 +16,7 @@ size_of_test!(size_of_pseudo_element, gecko_like_types::PseudoElement, 1); size_of_test!(size_of_selector_inner, SelectorInner, 40); size_of_test!(size_of_complex_selector, ComplexSelector, 24); -size_of_test!(size_of_component, Component, 64); -size_of_test!(size_of_attr_selector, AttrSelector, 48); +size_of_test!(size_of_component, Component, 32); size_of_test!(size_of_pseudo_class, PseudoClass, 24); impl parser::PseudoElement for gecko_like_types::PseudoElement { diff --git a/components/selectors/tree.rs b/components/selectors/tree.rs index 42659bfc615..689d5c2359c 100644 --- a/components/selectors/tree.rs +++ b/components/selectors/tree.rs @@ -5,9 +5,9 @@ //! Traits that nodes must implement. Breaks the otherwise-cyclic dependency between layout and //! style. -use attr::AttrSelectorOperation; +use attr::{AttrSelectorOperation, NamespaceConstraint}; use matching::{ElementSelectorFlags, MatchingContext}; -use parser::{NamespaceConstraint, SelectorImpl}; +use parser::SelectorImpl; pub trait Element: Sized { type Impl: SelectorImpl; @@ -22,26 +22,29 @@ pub trait Element: Sized { self.parent_element() } - // Skips non-element nodes + /// Skips non-element nodes fn first_child_element(&self) -> Option; - // Skips non-element nodes + /// Skips non-element nodes fn last_child_element(&self) -> Option; - // Skips non-element nodes + /// Skips non-element nodes fn prev_sibling_element(&self) -> Option; - // Skips non-element nodes + /// Skips non-element nodes fn next_sibling_element(&self) -> Option; fn is_html_element_in_html_document(&self) -> bool; + fn get_local_name(&self) -> &::BorrowedLocalName; + + /// Empty string for no namespace fn get_namespace(&self) -> &::BorrowedNamespaceUrl; fn attr_matches(&self, - ns: &NamespaceConstraint, + ns: &NamespaceConstraint<&::NamespaceUrl>, local_name: &::LocalName, - operation: &AttrSelectorOperation) + operation: &AttrSelectorOperation<&::AttrValue>) -> bool; fn match_non_ts_pseudo_class(&self, @@ -56,6 +59,7 @@ pub trait Element: Sized { -> bool; fn get_id(&self) -> Option<::Identifier>; + fn has_class(&self, name: &::ClassName) -> bool; /// Returns whether this element matches `:empty`. diff --git a/components/selectors/visitor.rs b/components/selectors/visitor.rs index be335aed87b..07f121fe82f 100644 --- a/components/selectors/visitor.rs +++ b/components/selectors/visitor.rs @@ -6,8 +6,8 @@ #![deny(missing_docs)] -use parser::{AttrSelector, Combinator, Component}; -use parser::{SelectorImpl, SelectorIter}; +use attr::NamespaceConstraint; +use parser::{Combinator, Component, SelectorImpl, SelectorIter}; /// A trait to visit selector properties. /// @@ -20,7 +20,12 @@ pub trait SelectorVisitor { /// Visit an attribute selector that may match (there are other selectors /// that may never match, like those containing whitespace or the empty /// string). - fn visit_attribute_selector(&mut self, _: &AttrSelector) -> bool { + fn visit_attribute_selector( + &mut self, + _namespace: &NamespaceConstraint<&::NamespaceUrl>, + _local_name: &::LocalName, + _local_name_lower: &::LocalName, + ) -> bool { true } diff --git a/components/style/attr.rs b/components/style/attr.rs index e3df55f3d09..d1e97703e5e 100644 --- a/components/style/attr.rs +++ b/components/style/attr.rs @@ -12,7 +12,6 @@ use cssparser::{self, Color, RGBA}; use euclid::num::Zero; use num_traits::ToPrimitive; use properties::PropertyDeclarationBlock; -use selector_parser::SelectorImpl; use selectors::attr::AttrSelectorOperation; use servo_url::ServoUrl; use shared_lock::Locked; @@ -352,9 +351,10 @@ impl AttrValue { } } - pub fn eval_selector(&self, selector: &AttrSelectorOperation) -> bool { + pub fn eval_selector(&self, selector: &AttrSelectorOperation<&String>) -> bool { // FIXME(SimonSapin) this can be more efficient by matching on `(self, selector)` variants - // and doing Atom comparisons instead of string comparisons where possible. + // and doing Atom comparisons instead of string comparisons where possible, + // with SelectorImpl::AttrValue changed to Atom. selector.eval_str(self) } } diff --git a/components/style/gecko/snapshot.rs b/components/style/gecko/snapshot.rs index 33abdcc5bd9..3bc1a946c5e 100644 --- a/components/style/gecko/snapshot.rs +++ b/components/style/gecko/snapshot.rs @@ -14,10 +14,8 @@ use gecko_bindings::structs::ServoElementSnapshot; use gecko_bindings::structs::ServoElementSnapshotFlags as Flags; use gecko_bindings::structs::ServoElementSnapshotTable; use restyle_hints::ElementSnapshot; -use selector_parser::SelectorImpl; -use selectors::attr::{AttrSelectorOperation, AttrSelectorOperator, CaseSensitivity}; -use selectors::parser::NamespaceConstraint; -use string_cache::Atom; +use selectors::attr::{AttrSelectorOperation, AttrSelectorOperator, CaseSensitivity, NamespaceConstraint}; +use string_cache::{Atom, Namespace}; /// A snapshot of a Gecko element. pub type GeckoElementSnapshot = ServoElementSnapshot; @@ -59,9 +57,9 @@ impl GeckoElementSnapshot { /// selectors::Element::attr_matches pub fn attr_matches(&self, - ns: &NamespaceConstraint, + ns: &NamespaceConstraint<&Namespace>, local_name: &Atom, - operation: &AttrSelectorOperation) + operation: &AttrSelectorOperation<&Atom>) -> bool { unsafe { match *operation { diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index ac4452a0e03..5801101eacf 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -64,9 +64,8 @@ use properties::style_structs::Font; use rule_tree::CascadeLevel as ServoCascadeLevel; use selector_parser::ElementExt; use selectors::Element; -use selectors::attr::{AttrSelectorOperation, AttrSelectorOperator, CaseSensitivity}; +use selectors::attr::{AttrSelectorOperation, AttrSelectorOperator, CaseSensitivity, NamespaceConstraint}; use selectors::matching::{ElementSelectorFlags, MatchingContext, MatchingMode}; -use selectors::parser::NamespaceConstraint; use shared_lock::Locked; use sink::Push; use std::cell::RefCell; @@ -1140,9 +1139,9 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { } fn attr_matches(&self, - ns: &NamespaceConstraint, + ns: &NamespaceConstraint<&Namespace>, local_name: &Atom, - operation: &AttrSelectorOperation) + operation: &AttrSelectorOperation<&Atom>) -> bool { unsafe { match *operation { @@ -1413,11 +1412,11 @@ pub trait NamespaceConstraintHelpers { fn atom_or_null(&self) -> *mut nsIAtom; } -impl NamespaceConstraintHelpers for NamespaceConstraint { +impl<'a> NamespaceConstraintHelpers for NamespaceConstraint<&'a Namespace> { fn atom_or_null(&self) -> *mut nsIAtom { match *self { NamespaceConstraint::Any => ptr::null_mut(), - NamespaceConstraint::Specific(ref ns) => ns.url.0.as_ptr(), + NamespaceConstraint::Specific(ref ns) => ns.0.as_ptr(), } } } diff --git a/components/style/restyle_hints.rs b/components/style/restyle_hints.rs index d5e2564d3de..f1e33462c37 100644 --- a/components/style/restyle_hints.rs +++ b/components/style/restyle_hints.rs @@ -8,19 +8,19 @@ use Atom; use LocalName; +use Namespace; use dom::TElement; use element_state::*; #[cfg(feature = "gecko")] use gecko_bindings::structs::nsRestyleHint; #[cfg(feature = "servo")] use heapsize::HeapSizeOf; -use selector_parser::{NonTSPseudoClass, PseudoElement, SelectorImpl, Snapshot, SnapshotMap}; +use selector_parser::{NonTSPseudoClass, PseudoElement, SelectorImpl, Snapshot, SnapshotMap, AttrValue}; use selectors::Element; -use selectors::attr::AttrSelectorOperation; +use selectors::attr::{AttrSelectorOperation, NamespaceConstraint}; use selectors::matching::{ElementSelectorFlags, MatchingContext, MatchingMode}; use selectors::matching::matches_selector; -use selectors::parser::{Combinator, Component, Selector}; -use selectors::parser::{SelectorInner, SelectorMethods, NamespaceConstraint}; +use selectors::parser::{Combinator, Component, Selector, SelectorInner, SelectorMethods}; use selectors::visitor::SelectorVisitor; use smallvec::SmallVec; use std::borrow::Borrow; @@ -371,9 +371,9 @@ impl<'a, E> Element for ElementWrapper<'a, E> } fn attr_matches(&self, - ns: &NamespaceConstraint, + ns: &NamespaceConstraint<&Namespace>, local_name: &LocalName, - operation: &AttrSelectorOperation) + operation: &AttrSelectorOperation<&AttrValue>) -> bool { match self.snapshot() { Some(snapshot) if snapshot.has_attrs() => { @@ -437,13 +437,9 @@ fn is_attr_selector(sel: &Component) -> bool { match *sel { Component::ID(_) | Component::Class(_) | - Component::AttrExists(_) | - Component::AttrEqual(_, _, _) | - Component::AttrIncludes(_, _) | - Component::AttrDashMatch(_, _) | - Component::AttrPrefixMatch(_, _) | - Component::AttrSubstringMatch(_, _) | - Component::AttrSuffixMatch(_, _) => true, + Component::AttributeInNoNamespaceExists { .. } | + Component::AttributeInNoNamespace { .. } | + Component::AttributeOther(_) => true, _ => false, } } diff --git a/components/style/servo/selector_parser.rs b/components/style/servo/selector_parser.rs index f86a9f16469..c5eb67b6a5a 100644 --- a/components/style/servo/selector_parser.rs +++ b/components/style/servo/selector_parser.rs @@ -14,10 +14,10 @@ use element_state::ElementState; use fnv::FnvHashMap; use restyle_hints::ElementSnapshot; use selector_parser::{ElementExt, PseudoElementCascadeType, SelectorParser}; -use selectors::{Element}; -use selectors::attr::AttrSelectorOperation; +use selectors::Element; +use selectors::attr::{AttrSelectorOperation, NamespaceConstraint}; use selectors::matching::{MatchingContext, MatchingMode}; -use selectors::parser::{SelectorMethods, NamespaceConstraint}; +use selectors::parser::SelectorMethods; use selectors::visitor::SelectorVisitor; use std::borrow::Cow; use std::fmt; @@ -559,14 +559,13 @@ impl ElementSnapshot for ServoElementSnapshot { impl ServoElementSnapshot { /// selectors::Element::attr_matches pub fn attr_matches(&self, - ns: &NamespaceConstraint, + ns: &NamespaceConstraint<&Namespace>, local_name: &LocalName, - operation: &AttrSelectorOperation) + operation: &AttrSelectorOperation<&String>) -> bool { - use selectors::parser::NamespaceConstraint; match *ns { NamespaceConstraint::Specific(ref ns) => { - self.get_attr(&ns.url, local_name) + self.get_attr(ns, local_name) .map_or(false, |value| value.eval_selector(operation)) } NamespaceConstraint::Any => { diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 644552050b2..a5c103c5a55 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -6,7 +6,7 @@ #![deny(missing_docs)] -use {Atom, LocalName}; +use {Atom, LocalName, Namespace}; use bit_vec::BitVec; use context::QuirksMode; use data::ComputedStyle; @@ -26,10 +26,11 @@ use properties::PropertyDeclarationBlock; use restyle_hints::{RestyleHint, DependencySet}; use rule_tree::{CascadeLevel, RuleTree, StrongRuleNode, StyleSource}; use selector_parser::{SelectorImpl, PseudoElement, SnapshotMap}; +use selectors::attr::NamespaceConstraint; use selectors::bloom::BloomFilter; use selectors::matching::{AFFECTED_BY_STYLE_ATTRIBUTE, AFFECTED_BY_PRESENTATIONAL_HINTS}; use selectors::matching::{ElementSelectorFlags, matches_selector, MatchingContext, MatchingMode}; -use selectors::parser::{AttrSelector, Combinator, Component, Selector, SelectorInner, SelectorIter}; +use selectors::parser::{Combinator, Component, Selector, SelectorInner, SelectorIter}; use selectors::parser::{SelectorMethods, LocalName as LocalNameSelector}; use selectors::visitor::SelectorVisitor; use shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards}; @@ -502,7 +503,7 @@ impl Stylist { /// Returns whether the given attribute might appear in an attribute /// selector of some rule in the stylist. pub fn might_have_attribute_dependency(&self, - local_name: &::LocalName) + local_name: &LocalName) -> bool { #[cfg(feature = "servo")] let style_lower_name = local_name!("style"); @@ -1088,17 +1089,19 @@ struct AttributeAndStateDependencyVisitor<'a>(&'a mut Stylist); impl<'a> SelectorVisitor for AttributeAndStateDependencyVisitor<'a> { type Impl = SelectorImpl; - fn visit_attribute_selector(&mut self, selector: &AttrSelector) -> bool { + fn visit_attribute_selector(&mut self, _ns: &NamespaceConstraint<&Namespace>, + name: &LocalName, lower_name: &LocalName) + -> bool { #[cfg(feature = "servo")] let style_lower_name = local_name!("style"); #[cfg(feature = "gecko")] let style_lower_name = atom!("style"); - if selector.lower_name == style_lower_name { + if *lower_name == style_lower_name { self.0.style_attribute_dependency = true; } else { - self.0.attribute_dependencies.insert(&selector.name); - self.0.attribute_dependencies.insert(&selector.lower_name); + self.0.attribute_dependencies.insert(&name); + self.0.attribute_dependencies.insert(&lower_name); } true } @@ -1157,13 +1160,9 @@ impl SelectorVisitor for RevalidationVisitor { /// concerned. fn visit_simple_selector(&mut self, s: &Component) -> bool { match *s { - Component::AttrExists(_) | - Component::AttrEqual(_, _, _) | - Component::AttrIncludes(_, _) | - Component::AttrDashMatch(_, _) | - Component::AttrPrefixMatch(_, _) | - Component::AttrSubstringMatch(_, _) | - Component::AttrSuffixMatch(_, _) | + Component::AttributeInNoNamespaceExists { .. } | + Component::AttributeInNoNamespace { .. } | + Component::AttributeOther(_) | Component::Empty | Component::FirstChild | Component::LastChild | diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index 52030bc21ac..168268b7ba2 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -6,7 +6,7 @@ use cssparser::{self, Parser as CssParser, SourcePosition, SourceLocation}; use html5ever::{Namespace as NsAtom}; use media_queries::CSSErrorReporterTest; use parking_lot::RwLock; -use selectors::attr::CaseSensitivity; +use selectors::attr::*; use selectors::parser::*; use servo_atoms::Atom; use servo_url::ServoUrl; @@ -98,14 +98,14 @@ fn test_parse_stylesheet() { name: local_name!("input"), lower_name: local_name!("input"), }), - Component::AttrEqual(AttrSelector { - name: local_name!("type"), - lower_name: local_name!("type"), - namespace: NamespaceConstraint::Specific(Namespace { - prefix: None, - url: ns!() - }), - }, "hidden".to_owned(), CaseSensitivity::AsciiCaseInsensitive) + Component::AttributeInNoNamespace { + local_name: local_name!("type"), + local_name_lower: local_name!("type"), + operator: AttrSelectorOperator::Equal, + value: "hidden".to_owned(), + case_sensitivity: CaseSensitivity::AsciiCaseInsensitive, + never_matches: false, + } ]), (0 << 20) + (1 << 10) + (1 << 0) ), From c5e37f3d2cdcf8b53d4b5f5876d2091e5676efed Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Thu, 18 May 2017 00:54:34 +0200 Subject: [PATCH 05/12] Remove unused selectors::Element::each_class --- components/script/dom/element.rs | 12 ----------- components/script/layout_wrapper.rs | 31 ++++++++++++----------------- components/selectors/tree.rs | 6 ------ components/style/dom.rs | 3 +++ components/style/gecko/wrapper.rs | 20 ++++++++----------- components/style/restyle_hints.rs | 9 --------- 6 files changed, 24 insertions(+), 57 deletions(-) diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index bdd2b49a751..6f423f47162 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -2481,18 +2481,6 @@ impl<'a> ::selectors::Element for Root { Element::has_class(&**self, name) } - fn each_class(&self, mut callback: F) - where F: FnMut(&Atom) - { - if let Some(ref attr) = self.get_attribute(&ns!(), &local_name!("class")) { - let tokens = attr.value(); - let tokens = tokens.as_tokens(); - for token in tokens { - callback(token); - } - } - } - fn is_html_element_in_html_document(&self) -> bool { self.html_element_in_html_document() } diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index ac59921d2ea..0e9dffd28ab 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -402,6 +402,17 @@ impl<'le> TElement for ServoLayoutElement<'le> { self.get_attr(namespace, attr).map_or(false, |x| x == val) } + #[inline(always)] + fn each_class(&self, mut callback: F) where F: FnMut(&Atom) { + unsafe { + if let Some(ref classes) = self.element.get_classes_for_layout() { + for class in *classes { + callback(class) + } + } + } + } + #[inline] fn existing_style_for_restyle_damage<'a>(&'a self, current_cv: &'a ComputedValues, @@ -728,17 +739,6 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { } } - #[inline(always)] - fn each_class(&self, mut callback: F) where F: FnMut(&Atom) { - unsafe { - if let Some(ref classes) = self.element.get_classes_for_layout() { - for class in *classes { - callback(class) - } - } - } - } - fn is_html_element_in_html_document(&self) -> bool { unsafe { self.element.html_element_in_html_document_for_layout() @@ -1098,8 +1098,8 @@ impl<'le> ThreadSafeLayoutElement for ServoThreadSafeLayoutElement<'le> { /// i.e., local_name, attributes, so they can only be used for **private** /// pseudo-elements (like `::-servo-details-content`). /// -/// Probably a few more of this functions can be implemented (like `has_class`, -/// `each_class`, etc), but they have no use right now. +/// Probably a few more of this functions can be implemented (like `has_class`, etc.), +/// but they have no use right now. /// /// Note that the element implementation is needed only for selector matching, /// not for inheritance (styles are inherited appropiately). @@ -1207,11 +1207,6 @@ impl<'le> ::selectors::Element for ServoThreadSafeLayoutElement<'le> { warn!("ServoThreadSafeLayoutElement::is_root called"); false } - - fn each_class(&self, _callback: F) - where F: FnMut(&Atom) { - warn!("ServoThreadSafeLayoutElement::each_class called"); - } } impl<'le> PresentationalHintsSynthesizer for ServoThreadSafeLayoutElement<'le> { diff --git a/components/selectors/tree.rs b/components/selectors/tree.rs index 689d5c2359c..276c788d05b 100644 --- a/components/selectors/tree.rs +++ b/components/selectors/tree.rs @@ -74,10 +74,4 @@ pub trait Element: Sized { /// Note: this can be false even if `.parent_element()` is `None` /// if the parent node is a `DocumentFragment`. fn is_root(&self) -> bool; - - // Ordinarily I wouldn't use callbacks like this, but the alternative is - // really messy, since there is a `JSRef` and a `RefCell` involved. Maybe - // in the future when we have associated types and/or a more convenient - // JS GC story... --pcwalton - fn each_class(&self, callback: F) where F: FnMut(&::ClassName); } diff --git a/components/style/dom.rs b/components/style/dom.rs index 1cff5d1c519..36997de5c32 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -381,6 +381,9 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone + /// Whether an attribute value equals `value`. fn attr_equals(&self, namespace: &Namespace, attr: &LocalName, value: &Atom) -> bool; + /// Internal iterator for the classes of this element. + fn each_class(&self, callback: F) where F: FnMut(&Atom); + /// Get the pre-existing style to calculate restyle damage (change hints). /// /// This needs to be generic since it varies between Servo and Gecko. diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 5801101eacf..3d8ab270658 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -655,6 +655,14 @@ impl<'le> TElement for GeckoElement<'le> { } } + fn each_class(&self, callback: F) + where F: FnMut(&Atom) + { + snapshot_helpers::each_class(self.0, + callback, + Gecko_ClassOrClassList) + } + fn existing_style_for_restyle_damage<'a>(&'a self, _existing_values: &'a ComputedValues, pseudo: Option<&PseudoElement>) @@ -1386,18 +1394,6 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { Gecko_ClassOrClassList) } - fn each_class(&self, callback: F) - where F: FnMut(&Atom) - { - if !self.may_have_class() { - return; - } - - snapshot_helpers::each_class(self.0, - callback, - Gecko_ClassOrClassList) - } - fn is_html_element_in_html_document(&self) -> bool { let node = self.as_node(); let node_info = node.node_info(); diff --git a/components/style/restyle_hints.rs b/components/style/restyle_hints.rs index f1e33462c37..73bd39ec1b3 100644 --- a/components/style/restyle_hints.rs +++ b/components/style/restyle_hints.rs @@ -407,15 +407,6 @@ impl<'a, E> Element for ElementWrapper<'a, E> self.element.is_root() } - fn each_class(&self, callback: F) - where F: FnMut(&Atom) { - match self.snapshot() { - Some(snapshot) if snapshot.has_attrs() - => snapshot.each_class(callback), - _ => self.element.each_class(callback) - } - } - fn pseudo_element_originating_element(&self) -> Option { self.element.closest_non_native_anonymous_ancestor() .map(|e| ElementWrapper::new(e, self.snapshot_map)) From 94b4a32c1894e6e95776c350b4cbc3645837af64 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Thu, 18 May 2017 00:27:49 +0200 Subject: [PATCH 06/12] Make some attr values case-insensitive in selectors https://bugzilla.mozilla.org/show_bug.cgi?id=1363531 --- Cargo.lock | 2 + components/selectors/Cargo.toml | 5 ++ components/selectors/attr.rs | 21 ++++++++- components/selectors/build.rs | 75 ++++++++++++++++++++++++++++++ components/selectors/lib.rs | 1 + components/selectors/matching.rs | 25 +++++----- components/selectors/parser.rs | 50 +++++++++++++------- components/style/gecko/snapshot.rs | 4 ++ components/style/gecko/wrapper.rs | 4 ++ python/tidy/servo_tidy/tidy.py | 10 ++++ 10 files changed, 167 insertions(+), 30 deletions(-) create mode 100644 components/selectors/build.rs diff --git a/Cargo.lock b/Cargo.lock index b21ff19f356..f9acb8824b6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2457,6 +2457,8 @@ dependencies = [ "cssparser 0.13.5 (registry+https://github.com/rust-lang/crates.io-index)", "fnv 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)", "matches 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)", + "phf 0.7.21 (registry+https://github.com/rust-lang/crates.io-index)", + "phf_codegen 0.7.21 (registry+https://github.com/rust-lang/crates.io-index)", "precomputed-hash 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "size_of_test 0.0.1", "smallvec 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/components/selectors/Cargo.toml b/components/selectors/Cargo.toml index ceef176eae1..4ca9a240c67 100644 --- a/components/selectors/Cargo.toml +++ b/components/selectors/Cargo.toml @@ -10,6 +10,7 @@ repository = "https://github.com/servo/servo" readme = "README.md" keywords = ["css", "selectors"] license = "MPL-2.0" +build = "build.rs" [lib] name = "selectors" @@ -25,8 +26,12 @@ bitflags = "0.7" matches = "0.1" cssparser = "0.13.3" fnv = "1.0" +phf = "0.7.18" precomputed-hash = "0.1" smallvec = "0.3" [dev-dependencies] size_of_test = {path = "../size_of_test"} + +[build-dependencies] +phf_codegen = "0.7.18" diff --git a/components/selectors/attr.rs b/components/selectors/attr.rs index f18219b2204..e8ab10ce02f 100644 --- a/components/selectors/attr.rs +++ b/components/selectors/attr.rs @@ -119,13 +119,29 @@ pub static SELECTOR_WHITESPACE: &'static [char] = &[' ', '\t', '\n', '\r', '\x0C pub enum CaseSensitivity { CaseSensitive, // Selectors spec says language-defined, but HTML says sensitive. AsciiCaseInsensitive, + AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument, } impl CaseSensitivity { + pub fn to_definite(self, is_html_element_in_html_document: bool) -> Self { + if let CaseSensitivity::AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument = self { + if is_html_element_in_html_document { + CaseSensitivity::AsciiCaseInsensitive + } else { + CaseSensitivity::CaseSensitive + } + } else { + self + } + } + pub fn eq(self, a: &[u8], b: &[u8]) -> bool { match self { CaseSensitivity::CaseSensitive => a == b, - CaseSensitivity::AsciiCaseInsensitive => a.eq_ignore_ascii_case(b) + CaseSensitivity::AsciiCaseInsensitive => a.eq_ignore_ascii_case(b), + CaseSensitivity::AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument => { + unreachable!("matching.rs should have called case_sensitivity.to_definite()"); + } } } @@ -152,6 +168,9 @@ impl CaseSensitivity { true } } + CaseSensitivity::AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument => { + unreachable!("matching.rs should have called case_sensitivity.to_definite()"); + } } } } diff --git a/components/selectors/build.rs b/components/selectors/build.rs new file mode 100644 index 00000000000..0d0a40256f3 --- /dev/null +++ b/components/selectors/build.rs @@ -0,0 +1,75 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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/. */ + +extern crate phf_codegen; + +use std::env; +use std::fs::File; +use std::io::{BufWriter, Write}; +use std::path::Path; + +fn main() { + let path = Path::new(&env::var("OUT_DIR").unwrap()) + .join("ascii_case_insensitive_html_attributes.rs"); + let mut file = BufWriter::new(File::create(&path).unwrap()); + + write!(&mut file, "{{ static SET: ::phf::Set<&'static str> = ", + ).unwrap(); + let mut set = phf_codegen::Set::new(); + for name in ASCII_CASE_INSENSITIVE_HTML_ATTRIBUTES.split_whitespace() { + set.entry(name); + } + set.build(&mut file).unwrap(); + write!(&mut file, "; &SET }}").unwrap(); +} + +/// https://html.spec.whatwg.org/multipage/#selectors +static ASCII_CASE_INSENSITIVE_HTML_ATTRIBUTES: &'static str = r#" + accept + accept-charset + align + alink + axis + bgcolor + charset + checked + clear + codetype + color + compact + declare + defer + dir + direction + disabled + enctype + face + frame + hreflang + http-equiv + lang + language + link + media + method + multiple + nohref + noresize + noshade + nowrap + readonly + rel + rev + rules + scope + scrolling + selected + shape + target + text + type + valign + valuetype + vlink +"#; diff --git a/components/selectors/lib.rs b/components/selectors/lib.rs index 5e7b4c99ce8..3e413c0c08b 100644 --- a/components/selectors/lib.rs +++ b/components/selectors/lib.rs @@ -6,6 +6,7 @@ #[macro_use] extern crate cssparser; #[macro_use] extern crate matches; extern crate fnv; +extern crate phf; extern crate precomputed_hash; #[cfg(test)] #[macro_use] extern crate size_of_test; extern crate smallvec; diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs index a11708818b3..2ed6a436687 100644 --- a/components/selectors/matching.rs +++ b/components/selectors/matching.rs @@ -5,7 +5,7 @@ use attr::{AttrSelectorOperation, NamespaceConstraint}; use bloom::BloomFilter; use parser::{Combinator, ComplexSelector, Component, LocalName}; -use parser::{Selector, SelectorInner, SelectorIter, SelectorImpl}; +use parser::{Selector, SelectorInner, SelectorIter}; use std::borrow::Borrow; use tree::Element; @@ -387,7 +387,8 @@ fn matches_simple_selector( element.match_pseudo_element(pseudo, context) } Component::LocalName(LocalName { ref name, ref lower_name }) => { - element.get_local_name() == select_name(element, name, lower_name).borrow() + let is_html = element.is_html_element_in_html_document(); + element.get_local_name() == select_name(is_html, name, lower_name).borrow() } Component::ExplicitUniversalType | Component::ExplicitAnyNamespace => { @@ -410,9 +411,10 @@ fn matches_simple_selector( element.has_class(class) } Component::AttributeInNoNamespaceExists { ref local_name, ref local_name_lower } => { + let is_html = element.is_html_element_in_html_document(); element.attr_matches( &NamespaceConstraint::Specific(&::parser::namespace_empty_string::()), - select_name(element, local_name, local_name_lower), + select_name(is_html, local_name, local_name_lower), &AttrSelectorOperation::Exists ) } @@ -427,12 +429,13 @@ fn matches_simple_selector( if never_matches { false } else { + let is_html = element.is_html_element_in_html_document(); element.attr_matches( &NamespaceConstraint::Specific(&::parser::namespace_empty_string::()), - select_name(element, local_name, local_name_lower), + select_name(is_html, local_name, local_name_lower), &AttrSelectorOperation::WithValue { operator: operator, - case_sensitivity: case_sensitivity, + case_sensitivity: case_sensitivity.to_definite(is_html), expected_value: value, } ) @@ -442,9 +445,10 @@ fn matches_simple_selector( if attr_sel.never_matches { return false } else { + let is_html = element.is_html_element_in_html_document(); element.attr_matches( &attr_sel.namespace(), - select_name(element, &attr_sel.local_name, &attr_sel.local_name_lower), + select_name(is_html, &attr_sel.local_name, &attr_sel.local_name_lower), &match attr_sel.operation { AttrSelectorOperation::Exists => AttrSelectorOperation::Exists, AttrSelectorOperation::WithValue { @@ -454,7 +458,7 @@ fn matches_simple_selector( } => { AttrSelectorOperation::WithValue { operator: operator, - case_sensitivity: case_sensitivity, + case_sensitivity: case_sensitivity.to_definite(is_html), expected_value: expected_value, } } @@ -512,11 +516,8 @@ fn matches_simple_selector( } } -fn select_name<'a, E>(element: &E, local_name: &'a ::LocalName, - local_name_lower: &'a ::LocalName) - -> &'a ::LocalName -where E: Element { - if element.is_html_element_in_html_document() { +fn select_name<'a, T>(is_html: bool, local_name: &'a T, local_name_lower: &'a T) -> &'a T { + if is_html { local_name_lower } else { local_name diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index befbfd084cc..915357887fb 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -33,6 +33,16 @@ pub trait PseudoElement : Sized + ToCss { } } +fn to_ascii_lowercase(s: &str) -> Cow { + if let Some(first_uppercase) = s.bytes().position(|byte| byte >= b'A' && byte <= b'Z') { + let mut string = s.to_owned(); + string[first_uppercase..].make_ascii_lowercase(); + string.into() + } else { + s.into() + } +} + macro_rules! with_all_bounds { ( [ $( $InSelector: tt )* ] @@ -46,16 +56,6 @@ macro_rules! with_all_bounds { } } - fn from_ascii_lowercase(s: &str) -> T where T: $($FromStr)* { - if let Some(first_uppercase) = s.bytes().position(|byte| byte >= b'A' && byte <= b'Z') { - let mut string = s.to_owned(); - string[first_uppercase..].make_ascii_lowercase(); - T::from(string) - } else { - T::from(s) - } - } - /// This trait allows to define the parser implementation in regards /// of pseudo-classes/elements /// @@ -735,7 +735,8 @@ impl ToCss for Component { write!(CssStringWriter::new(dest), "{}", value)?; dest.write_char('"')?; match case_sensitivity { - CaseSensitivity::CaseSensitive => {}, + CaseSensitivity::CaseSensitive | + CaseSensitivity::AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument => {}, CaseSensitivity::AsciiCaseInsensitive => dest.write_str(" i")?, } dest.write_char(']') @@ -790,7 +791,8 @@ impl ToCss for AttrSelectorWithNamespace { write!(CssStringWriter::new(dest), "{}", expected_value)?; dest.write_char('"')?; match case_sensitivity { - CaseSensitivity::CaseSensitive => {}, + CaseSensitivity::CaseSensitive | + CaseSensitivity::AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument => {}, CaseSensitivity::AsciiCaseInsensitive => dest.write_str(" i")?, } }, @@ -1078,7 +1080,7 @@ fn parse_type_selector(parser: &P, input: &mut CssParser, sequence: &mu match local_name { Some(name) => { sequence.push(Component::LocalName(LocalName { - lower_name: from_ascii_lowercase(&name), + lower_name: from_cow_str(to_ascii_lowercase(&name)), name: from_cow_str(name), })) } @@ -1190,13 +1192,11 @@ fn parse_attribute_selector(parser: &P, input: &mut CssParser) { let namespace; let local_name; - let local_name_lower; match parse_qualified_name(parser, input, /* in_attr_selector = */ true)? { None => return Err(()), Some((_, None)) => unreachable!(), Some((ns, Some(ln))) => { - local_name_lower = from_ascii_lowercase(&ln); - local_name = from_cow_str(ln); + local_name = ln; namespace = match ns { QNamePrefix::ImplicitNoNamespace | QNamePrefix::ExplicitNoNamespace => { @@ -1222,6 +1222,8 @@ fn parse_attribute_selector(parser: &P, input: &mut CssParser) match input.next() { // [foo] Err(()) => { + let local_name_lower = from_cow_str(to_ascii_lowercase(&local_name)); + let local_name = from_cow_str(local_name); if let Some(namespace) = namespace { return Ok(Component::AttributeOther(Box::new(AttrSelectorWithNamespace { namespace: namespace, @@ -1277,8 +1279,22 @@ fn parse_attribute_selector(parser: &P, input: &mut CssParser) _ => return Err(()) } - let case_sensitivity = parse_attribute_flags(input)?; + let mut case_sensitivity = parse_attribute_flags(input)?; + let value = from_cow_str(value); + let local_name_lower; + { + let local_name_lower_cow = to_ascii_lowercase(&local_name); + if let CaseSensitivity::CaseSensitive = case_sensitivity { + if include!(concat!(env!("OUT_DIR"), "/ascii_case_insensitive_html_attributes.rs")) + .contains(&*local_name_lower_cow) + { + case_sensitivity = CaseSensitivity::AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument + } + } + local_name_lower = from_cow_str(local_name_lower_cow); + } + let local_name = from_cow_str(local_name); if let Some(namespace) = namespace { Ok(Component::AttributeOther(Box::new(AttrSelectorWithNamespace { namespace: namespace, diff --git a/components/style/gecko/snapshot.rs b/components/style/gecko/snapshot.rs index 3bc1a946c5e..3814eff5810 100644 --- a/components/style/gecko/snapshot.rs +++ b/components/style/gecko/snapshot.rs @@ -72,6 +72,10 @@ impl GeckoElementSnapshot { let ignore_case = match case_sensitivity { CaseSensitivity::CaseSensitive => false, CaseSensitivity::AsciiCaseInsensitive => true, + CaseSensitivity::AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument => { + unreachable!("selectors/matching.rs should have \ + called case_sensitivity.to_definite()"); + } }; // FIXME: case sensitivity for operators other than Equal match operator { diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 3d8ab270658..3df3381d804 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -1162,6 +1162,10 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { let ignore_case = match case_sensitivity { CaseSensitivity::CaseSensitive => false, CaseSensitivity::AsciiCaseInsensitive => true, + CaseSensitivity::AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument => { + unreachable!("selectors/matching.rs should have \ + called case_sensitivity.to_definite()"); + } }; // FIXME: case sensitivity for operators other than Equal match operator { diff --git a/python/tidy/servo_tidy/tidy.py b/python/tidy/servo_tidy/tidy.py index b8229e67d96..51e7e69f8d4 100644 --- a/python/tidy/servo_tidy/tidy.py +++ b/python/tidy/servo_tidy/tidy.py @@ -447,6 +447,7 @@ def check_rust(file_name, lines): prev_use = None prev_open_brace = False + multi_line_string = False current_indent = 0 prev_crate = {} prev_mod = {} @@ -464,6 +465,15 @@ def check_rust(file_name, lines): prev_indent = indent indent = len(original_line) - len(line) + # Hack for components/selectors/build.rs + if multi_line_string: + if line.startswith('"#'): + multi_line_string = False + else: + continue + if line.endswith('r#"'): + multi_line_string = True + is_attribute = re.search(r"#\[.*\]", line) is_comment = re.search(r"^//|^/\*|^\*", line) From 853b688d0b1e5caff55685ef0cb029b3ebe151ed Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Thu, 18 May 2017 09:45:54 +0200 Subject: [PATCH 07/12] Only non-namespaced attributes can have implicitly case-insensitive values --- components/selectors/parser.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index 915357887fb..a10b95b04e2 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -1286,7 +1286,8 @@ fn parse_attribute_selector(parser: &P, input: &mut CssParser) { let local_name_lower_cow = to_ascii_lowercase(&local_name); if let CaseSensitivity::CaseSensitive = case_sensitivity { - if include!(concat!(env!("OUT_DIR"), "/ascii_case_insensitive_html_attributes.rs")) + if namespace.is_none() && + include!(concat!(env!("OUT_DIR"), "/ascii_case_insensitive_html_attributes.rs")) .contains(&*local_name_lower_cow) { case_sensitivity = CaseSensitivity::AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument From 7149a6a29de6d1c351eedf1404369aa5f9efbd09 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Thu, 18 May 2017 10:32:26 +0200 Subject: [PATCH 08/12] =?UTF-8?q?ol[type=3D=E2=80=A6]=20and=20li[type=3D?= =?UTF-8?q?=E2=80=A6]=20preshints=20need=20to=20be=20case-sensitive?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- components/script/dom/element.rs | 5 +++ components/script/layout_wrapper.rs | 5 ++- components/style/attr.rs | 9 +++++ components/style/servo/selector_parser.rs | 33 +++++++++++++++---- resources/presentational-hints.css | 14 +++++--- .../lists/li-type-supported-xhtml.xhtml.ini | 3 -- .../lists/li-type-supported.html.ini | 3 -- 7 files changed, 53 insertions(+), 19 deletions(-) delete mode 100644 tests/wpt/metadata/html/rendering/non-replaced-elements/lists/li-type-supported-xhtml.xhtml.ini delete mode 100644 tests/wpt/metadata/html/rendering/non-replaced-elements/lists/li-type-supported.html.ini diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 6f423f47162..55175fd45c6 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -2451,6 +2451,11 @@ impl<'a> ::selectors::Element for Root { } }, + NonTSPseudoClass::ServoCaseSensitiveTypeAttr(ref expected_value) => { + self.get_attribute(&ns!(), &local_name!("type")) + .map_or(false, |attr| attr.value().eq(expected_value)) + } + // FIXME(#15746): This is wrong, we need to instead use extended filtering as per RFC4647 // https://tools.ietf.org/html/rfc4647#section-3.3.2 NonTSPseudoClass::Lang(ref lang) => extended_filtering(&*self.get_lang(), &*lang), diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index 0e9dffd28ab..62e5158f321 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -706,7 +706,10 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { _ => true, } }, - + NonTSPseudoClass::ServoCaseSensitiveTypeAttr(ref expected_value) => { + self.get_attr_enum(&ns!(), &local_name!("type")) + .map_or(false, |attr| attr == expected_value) + } NonTSPseudoClass::ReadOnly => !self.element.get_state_for_layout().contains(pseudo_class.state_flag()), diff --git a/components/style/attr.rs b/components/style/attr.rs index d1e97703e5e..535d9109151 100644 --- a/components/style/attr.rs +++ b/components/style/attr.rs @@ -379,6 +379,15 @@ impl ::std::ops::Deref for AttrValue { } } +impl PartialEq for AttrValue { + fn eq(&self, other: &Atom) -> bool { + match *self { + AttrValue::Atom(ref value) => value == other, + _ => other == &**self, + } + } +} + /// https://html.spec.whatwg.org/multipage/#rules-for-parsing-non-zero-dimension-values pub fn parse_nonzero_length(value: &str) -> LengthOrPercentageOrAuto { match parse_length(value) { diff --git a/components/style/servo/selector_parser.rs b/components/style/servo/selector_parser.rs index c5eb67b6a5a..fdc14b11e29 100644 --- a/components/style/servo/selector_parser.rs +++ b/components/style/servo/selector_parser.rs @@ -175,6 +175,7 @@ pub enum NonTSPseudoClass { ReadWrite, ReadOnly, ServoNonZeroBorder, + ServoCaseSensitiveTypeAttr(Atom), Target, Visited, } @@ -182,10 +183,18 @@ pub enum NonTSPseudoClass { impl ToCss for NonTSPseudoClass { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { use self::NonTSPseudoClass::*; - if let Lang(ref lang) = *self { - dest.write_str(":lang(")?; - serialize_identifier(lang, dest)?; - return dest.write_str(")"); + match *self { + Lang(ref lang) => { + dest.write_str(":lang(")?; + serialize_identifier(lang, dest)?; + return dest.write_str(")") + } + ServoCaseSensitiveTypeAttr(ref value) => { + dest.write_str(":-servo-case-sensitive-type-attr(")?; + serialize_identifier(value, dest)?; + return dest.write_str(")") + } + _ => {} } dest.write_str(match *self { @@ -198,7 +207,6 @@ impl ToCss for NonTSPseudoClass { Fullscreen => ":fullscreen", Hover => ":hover", Indeterminate => ":indeterminate", - Lang(_) => unreachable!(), Link => ":link", PlaceholderShown => ":placeholder-shown", ReadWrite => ":read-write", @@ -206,6 +214,8 @@ impl ToCss for NonTSPseudoClass { ServoNonZeroBorder => ":-servo-nonzero-border", Target => ":target", Visited => ":visited", + Lang(_) | + ServoCaseSensitiveTypeAttr(_) => unreachable!(), }) } } @@ -244,7 +254,8 @@ impl NonTSPseudoClass { Lang(_) | Link | Visited | - ServoNonZeroBorder => ElementState::empty(), + ServoNonZeroBorder | + ServoCaseSensitiveTypeAttr(_) => ElementState::empty(), } } @@ -313,7 +324,15 @@ impl<'a> ::selectors::Parser for SelectorParser<'a> { -> Result { use self::NonTSPseudoClass::*; let pseudo_class = match_ignore_ascii_case!{ &name, - "lang" => Lang(String::from(try!(parser.expect_ident_or_string())).into_boxed_str()), + "lang" => { + Lang(parser.expect_ident_or_string()?.into_owned().into_boxed_str()) + } + "-servo-case-sensitive-type-attr" => { + if !self.in_user_agent_stylesheet() { + return Err(()); + } + ServoCaseSensitiveTypeAttr(Atom::from(parser.expect_ident()?)) + } _ => return Err(()) }; diff --git a/resources/presentational-hints.css b/resources/presentational-hints.css index 84157dcb4bd..63f42491dd3 100644 --- a/resources/presentational-hints.css +++ b/resources/presentational-hints.css @@ -18,11 +18,15 @@ br[clear=right i] { clear: right; } br[clear=all i], br[clear=both i] { clear: both; } -ol[type=1], li[type=1] { list-style-type: decimal; } -ol[type=a], li[type=a] { list-style-type: lower-alpha; } -ol[type=A], li[type=A] { list-style-type: upper-alpha; } -ol[type=i], li[type=i] { list-style-type: lower-roman; } -ol[type=I], li[type=I] { list-style-type: upper-roman; } +ol[type="1"], li[type="1"] { list-style-type: decimal; } +ol:-servo-case-sensitive-type-attr(a), +li:-servo-case-sensitive-type-attr(a) { list-style-type: lower-alpha; } +ol:-servo-case-sensitive-type-attr(A), +li:-servo-case-sensitive-type-attr(A) { list-style-type: upper-alpha; } +ol:-servo-case-sensitive-type-attr(i), +li:-servo-case-sensitive-type-attr(i) { list-style-type: lower-roman; } +ol:-servo-case-sensitive-type-attr(I), +li:-servo-case-sensitive-type-attr(I) { list-style-type: upper-roman; } ul[type=none i], li[type=none i] { list-style-type: none; } ul[type=disc i], li[type=disc i] { list-style-type: disc; } ul[type=circle i], li[type=circle i] { list-style-type: circle; } diff --git a/tests/wpt/metadata/html/rendering/non-replaced-elements/lists/li-type-supported-xhtml.xhtml.ini b/tests/wpt/metadata/html/rendering/non-replaced-elements/lists/li-type-supported-xhtml.xhtml.ini deleted file mode 100644 index 3ac5a7343ca..00000000000 --- a/tests/wpt/metadata/html/rendering/non-replaced-elements/lists/li-type-supported-xhtml.xhtml.ini +++ /dev/null @@ -1,3 +0,0 @@ -[li-type-supported-xhtml.xhtml] - type: reftest - expected: FAIL diff --git a/tests/wpt/metadata/html/rendering/non-replaced-elements/lists/li-type-supported.html.ini b/tests/wpt/metadata/html/rendering/non-replaced-elements/lists/li-type-supported.html.ini deleted file mode 100644 index 1f631a1d1b8..00000000000 --- a/tests/wpt/metadata/html/rendering/non-replaced-elements/lists/li-type-supported.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[li-type-supported.html] - type: reftest - expected: FAIL From 76166bce585cfcbe8bea4fa2538fec2ec34ca90e Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Thu, 18 May 2017 12:28:05 +0200 Subject: [PATCH 09/12] Add more enum types to avoid unreachable!() for selector case-sensitivity --- components/selectors/attr.rs | 44 ++++++++++++++++++++---------- components/selectors/matching.rs | 10 +++---- components/selectors/parser.rs | 41 +++++++++++++++------------- components/style/gecko/snapshot.rs | 4 --- components/style/gecko/wrapper.rs | 4 --- tests/unit/style/stylesheets.rs | 2 +- 6 files changed, 57 insertions(+), 48 deletions(-) diff --git a/components/selectors/attr.rs b/components/selectors/attr.rs index e8ab10ce02f..274081ae9f7 100644 --- a/components/selectors/attr.rs +++ b/components/selectors/attr.rs @@ -12,7 +12,7 @@ pub struct AttrSelectorWithNamespace { pub namespace: NamespaceConstraint<(Impl::NamespacePrefix, Impl::NamespaceUrl)>, pub local_name: Impl::LocalName, pub local_name_lower: Impl::LocalName, - pub operation: AttrSelectorOperation, + pub operation: ParsedAttrSelectorOperation, pub never_matches: bool, } @@ -35,6 +35,16 @@ pub enum NamespaceConstraint { Specific(NamespaceUrl), } +#[derive(Eq, PartialEq, Clone)] +pub enum ParsedAttrSelectorOperation { + Exists, + WithValue { + operator: AttrSelectorOperator, + case_sensitivity: ParsedCaseSensitivity, + expected_value: AttrValue, + } +} + #[derive(Eq, PartialEq, Clone)] pub enum AttrSelectorOperation { Exists, @@ -116,32 +126,39 @@ impl AttrSelectorOperator { pub static SELECTOR_WHITESPACE: &'static [char] = &[' ', '\t', '\n', '\r', '\x0C']; #[derive(Eq, PartialEq, Clone, Copy, Debug)] -pub enum CaseSensitivity { +pub enum ParsedCaseSensitivity { CaseSensitive, // Selectors spec says language-defined, but HTML says sensitive. AsciiCaseInsensitive, AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument, } -impl CaseSensitivity { - pub fn to_definite(self, is_html_element_in_html_document: bool) -> Self { - if let CaseSensitivity::AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument = self { - if is_html_element_in_html_document { +impl ParsedCaseSensitivity { + pub fn to_unconditional(self, is_html_element_in_html_document: bool) -> CaseSensitivity { + match self { + ParsedCaseSensitivity::AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument + if is_html_element_in_html_document => { CaseSensitivity::AsciiCaseInsensitive - } else { + } + ParsedCaseSensitivity::AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument => { CaseSensitivity::CaseSensitive } - } else { - self + ParsedCaseSensitivity::CaseSensitive => CaseSensitivity::CaseSensitive, + ParsedCaseSensitivity::AsciiCaseInsensitive => CaseSensitivity::AsciiCaseInsensitive, } } +} +#[derive(Eq, PartialEq, Clone, Copy, Debug)] +pub enum CaseSensitivity { + CaseSensitive, // Selectors spec says language-defined, but HTML says sensitive. + AsciiCaseInsensitive, +} + +impl CaseSensitivity { pub fn eq(self, a: &[u8], b: &[u8]) -> bool { match self { CaseSensitivity::CaseSensitive => a == b, CaseSensitivity::AsciiCaseInsensitive => a.eq_ignore_ascii_case(b), - CaseSensitivity::AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument => { - unreachable!("matching.rs should have called case_sensitivity.to_definite()"); - } } } @@ -168,9 +185,6 @@ impl CaseSensitivity { true } } - CaseSensitivity::AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument => { - unreachable!("matching.rs should have called case_sensitivity.to_definite()"); - } } } } diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs index 2ed6a436687..e5e130aeaeb 100644 --- a/components/selectors/matching.rs +++ b/components/selectors/matching.rs @@ -2,7 +2,7 @@ * 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/. */ -use attr::{AttrSelectorOperation, NamespaceConstraint}; +use attr::{ParsedAttrSelectorOperation, AttrSelectorOperation, NamespaceConstraint}; use bloom::BloomFilter; use parser::{Combinator, ComplexSelector, Component, LocalName}; use parser::{Selector, SelectorInner, SelectorIter}; @@ -435,7 +435,7 @@ fn matches_simple_selector( select_name(is_html, local_name, local_name_lower), &AttrSelectorOperation::WithValue { operator: operator, - case_sensitivity: case_sensitivity.to_definite(is_html), + case_sensitivity: case_sensitivity.to_unconditional(is_html), expected_value: value, } ) @@ -450,15 +450,15 @@ fn matches_simple_selector( &attr_sel.namespace(), select_name(is_html, &attr_sel.local_name, &attr_sel.local_name_lower), &match attr_sel.operation { - AttrSelectorOperation::Exists => AttrSelectorOperation::Exists, - AttrSelectorOperation::WithValue { + ParsedAttrSelectorOperation::Exists => AttrSelectorOperation::Exists, + ParsedAttrSelectorOperation::WithValue { operator, case_sensitivity, ref expected_value, } => { AttrSelectorOperation::WithValue { operator: operator, - case_sensitivity: case_sensitivity.to_definite(is_html), + case_sensitivity: case_sensitivity.to_unconditional(is_html), expected_value: expected_value, } } diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index a10b95b04e2..3fa3f135f9e 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -3,8 +3,8 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use arcslice::ArcSlice; -use attr::{AttrSelectorWithNamespace, AttrSelectorOperation, AttrSelectorOperator}; -use attr::{CaseSensitivity, SELECTOR_WHITESPACE, NamespaceConstraint}; +use attr::{AttrSelectorWithNamespace, ParsedAttrSelectorOperation, AttrSelectorOperator}; +use attr::{ParsedCaseSensitivity, SELECTOR_WHITESPACE, NamespaceConstraint}; use cssparser::{Token, Parser as CssParser, parse_nth, ToCss, serialize_identifier, CssStringWriter}; use precomputed_hash::PrecomputedHash; use smallvec::SmallVec; @@ -548,7 +548,7 @@ pub enum Component { local_name_lower: Impl::LocalName, operator: AttrSelectorOperator, value: Impl::AttrValue, - case_sensitivity: CaseSensitivity, + case_sensitivity: ParsedCaseSensitivity, never_matches: bool, }, // Use a Box in the less common cases with more data to keep size_of::() small. @@ -735,9 +735,9 @@ impl ToCss for Component { write!(CssStringWriter::new(dest), "{}", value)?; dest.write_char('"')?; match case_sensitivity { - CaseSensitivity::CaseSensitive | - CaseSensitivity::AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument => {}, - CaseSensitivity::AsciiCaseInsensitive => dest.write_str(" i")?, + ParsedCaseSensitivity::CaseSensitive | + ParsedCaseSensitivity::AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument => {}, + ParsedCaseSensitivity::AsciiCaseInsensitive => dest.write_str(" i")?, } dest.write_char(']') } @@ -784,16 +784,18 @@ impl ToCss for AttrSelectorWithNamespace { } display_to_css_identifier(&self.local_name, dest)?; match self.operation { - AttrSelectorOperation::Exists => {}, - AttrSelectorOperation::WithValue { operator, case_sensitivity, ref expected_value } => { + ParsedAttrSelectorOperation::Exists => {}, + ParsedAttrSelectorOperation::WithValue { + operator, case_sensitivity, ref expected_value + } => { operator.to_css(dest)?; dest.write_char('"')?; write!(CssStringWriter::new(dest), "{}", expected_value)?; dest.write_char('"')?; match case_sensitivity { - CaseSensitivity::CaseSensitive | - CaseSensitivity::AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument => {}, - CaseSensitivity::AsciiCaseInsensitive => dest.write_str(" i")?, + ParsedCaseSensitivity::CaseSensitive | + ParsedCaseSensitivity::AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument => {}, + ParsedCaseSensitivity::AsciiCaseInsensitive => dest.write_str(" i")?, } }, } @@ -1229,7 +1231,7 @@ fn parse_attribute_selector(parser: &P, input: &mut CssParser) namespace: namespace, local_name: local_name, local_name_lower: local_name_lower, - operation: AttrSelectorOperation::Exists, + operation: ParsedAttrSelectorOperation::Exists, never_matches: false, }))) } else { @@ -1285,12 +1287,13 @@ fn parse_attribute_selector(parser: &P, input: &mut CssParser) let local_name_lower; { let local_name_lower_cow = to_ascii_lowercase(&local_name); - if let CaseSensitivity::CaseSensitive = case_sensitivity { + if let ParsedCaseSensitivity::CaseSensitive = case_sensitivity { if namespace.is_none() && include!(concat!(env!("OUT_DIR"), "/ascii_case_insensitive_html_attributes.rs")) .contains(&*local_name_lower_cow) { - case_sensitivity = CaseSensitivity::AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument + case_sensitivity = + ParsedCaseSensitivity::AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument } } local_name_lower = from_cow_str(local_name_lower_cow); @@ -1302,7 +1305,7 @@ fn parse_attribute_selector(parser: &P, input: &mut CssParser) local_name: local_name, local_name_lower: local_name_lower, never_matches: never_matches, - operation: AttrSelectorOperation::WithValue { + operation: ParsedAttrSelectorOperation::WithValue { operator: operator, case_sensitivity: case_sensitivity, expected_value: value, @@ -1321,11 +1324,11 @@ fn parse_attribute_selector(parser: &P, input: &mut CssParser) } -fn parse_attribute_flags(input: &mut CssParser) -> Result { +fn parse_attribute_flags(input: &mut CssParser) -> Result { match input.next() { - Err(()) => Ok(CaseSensitivity::CaseSensitive), + Err(()) => Ok(ParsedCaseSensitivity::CaseSensitive), Ok(Token::Ident(ref value)) if value.eq_ignore_ascii_case("i") => { - Ok(CaseSensitivity::AsciiCaseInsensitive) + Ok(ParsedCaseSensitivity::AsciiCaseInsensitive) } _ => Err(()) } @@ -1971,7 +1974,7 @@ pub mod tests { operator: AttrSelectorOperator::DashMatch, value: DummyAtom::from("foo"), never_matches: false, - case_sensitivity: CaseSensitivity::CaseSensitive, + case_sensitivity: ParsedCaseSensitivity::CaseSensitive, } ]), specificity_and_flags: specificity(0, 1, 0), diff --git a/components/style/gecko/snapshot.rs b/components/style/gecko/snapshot.rs index 3814eff5810..3bc1a946c5e 100644 --- a/components/style/gecko/snapshot.rs +++ b/components/style/gecko/snapshot.rs @@ -72,10 +72,6 @@ impl GeckoElementSnapshot { let ignore_case = match case_sensitivity { CaseSensitivity::CaseSensitive => false, CaseSensitivity::AsciiCaseInsensitive => true, - CaseSensitivity::AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument => { - unreachable!("selectors/matching.rs should have \ - called case_sensitivity.to_definite()"); - } }; // FIXME: case sensitivity for operators other than Equal match operator { diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 3df3381d804..3d8ab270658 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -1162,10 +1162,6 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { let ignore_case = match case_sensitivity { CaseSensitivity::CaseSensitive => false, CaseSensitivity::AsciiCaseInsensitive => true, - CaseSensitivity::AsciiCaseInsensitiveIfInHtmlElementInHtmlDocument => { - unreachable!("selectors/matching.rs should have \ - called case_sensitivity.to_definite()"); - } }; // FIXME: case sensitivity for operators other than Equal match operator { diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index 168268b7ba2..44c02f6c1f2 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -103,7 +103,7 @@ fn test_parse_stylesheet() { local_name_lower: local_name!("type"), operator: AttrSelectorOperator::Equal, value: "hidden".to_owned(), - case_sensitivity: CaseSensitivity::AsciiCaseInsensitive, + case_sensitivity: ParsedCaseSensitivity::AsciiCaseInsensitive, never_matches: false, } ]), From c06e574c186b17b9c49cf78ac6cd0daef679a3e3 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Thu, 18 May 2017 12:32:10 +0200 Subject: [PATCH 10/12] Short-circuit and unindent --- components/selectors/matching.rs | 60 +++++++++++++++----------------- 1 file changed, 29 insertions(+), 31 deletions(-) diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs index e5e130aeaeb..50b686f2558 100644 --- a/components/selectors/matching.rs +++ b/components/selectors/matching.rs @@ -427,44 +427,42 @@ fn matches_simple_selector( never_matches, } => { if never_matches { - false - } else { - let is_html = element.is_html_element_in_html_document(); - element.attr_matches( - &NamespaceConstraint::Specific(&::parser::namespace_empty_string::()), - select_name(is_html, local_name, local_name_lower), - &AttrSelectorOperation::WithValue { - operator: operator, - case_sensitivity: case_sensitivity.to_unconditional(is_html), - expected_value: value, - } - ) + return false } + let is_html = element.is_html_element_in_html_document(); + element.attr_matches( + &NamespaceConstraint::Specific(&::parser::namespace_empty_string::()), + select_name(is_html, local_name, local_name_lower), + &AttrSelectorOperation::WithValue { + operator: operator, + case_sensitivity: case_sensitivity.to_unconditional(is_html), + expected_value: value, + } + ) } Component::AttributeOther(ref attr_sel) => { if attr_sel.never_matches { return false - } else { - let is_html = element.is_html_element_in_html_document(); - element.attr_matches( - &attr_sel.namespace(), - select_name(is_html, &attr_sel.local_name, &attr_sel.local_name_lower), - &match attr_sel.operation { - ParsedAttrSelectorOperation::Exists => AttrSelectorOperation::Exists, - ParsedAttrSelectorOperation::WithValue { - operator, - case_sensitivity, - ref expected_value, - } => { - AttrSelectorOperation::WithValue { - operator: operator, - case_sensitivity: case_sensitivity.to_unconditional(is_html), - expected_value: expected_value, - } + } + let is_html = element.is_html_element_in_html_document(); + element.attr_matches( + &attr_sel.namespace(), + select_name(is_html, &attr_sel.local_name, &attr_sel.local_name_lower), + &match attr_sel.operation { + ParsedAttrSelectorOperation::Exists => AttrSelectorOperation::Exists, + ParsedAttrSelectorOperation::WithValue { + operator, + case_sensitivity, + ref expected_value, + } => { + AttrSelectorOperation::WithValue { + operator: operator, + case_sensitivity: case_sensitivity.to_unconditional(is_html), + expected_value: expected_value, } } - ) - } + } + ) } Component::NonTSPseudoClass(ref pc) => { element.match_non_ts_pseudo_class(pc, context, flags_setter) From c968842653533fe54761239ea2dae04369664580 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Thu, 18 May 2017 13:31:04 +0200 Subject: [PATCH 11/12] =?UTF-8?q?Don=E2=80=99t=20visit=20attribute=20selec?= =?UTF-8?q?tors=20that=20never=20match.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- components/selectors/parser.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index 3fa3f135f9e..197ce4f2629 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -316,7 +316,6 @@ impl SelectorMethods for Component { } } - AttributeInNoNamespace { ref local_name, ref local_name_lower, .. } | AttributeInNoNamespaceExists { ref local_name, ref local_name_lower } => { if !visitor.visit_attribute_selector( &NamespaceConstraint::Specific(&namespace_empty_string::()), @@ -326,7 +325,17 @@ impl SelectorMethods for Component { return false; } } - AttributeOther(ref attr_selector) => { + AttributeInNoNamespace { ref local_name, ref local_name_lower, never_matches, .. } + if !never_matches => { + if !visitor.visit_attribute_selector( + &NamespaceConstraint::Specific(&namespace_empty_string::()), + local_name, + local_name_lower, + ) { + return false; + } + } + AttributeOther(ref attr_selector) if !attr_selector.never_matches => { if !visitor.visit_attribute_selector( &attr_selector.namespace(), &attr_selector.local_name, From b359e3fcb40c03b1cb801caa7943eb72881509cf Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 19 May 2017 00:20:08 +0200 Subject: [PATCH 12/12] =?UTF-8?q?More=20test=20expectations=20changes?= =?UTF-8?q?=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../selectors-3_dev/html/css3-selectors-lang-024.htm.ini | 5 ----- .../selectors-3_dev/html/css3-selectors-lang-035.htm.ini | 5 ----- .../selectors-3_dev/html/css3-selectors-lang-044.htm.ini | 5 ----- .../selectors-3_dev/html/css3-selectors-lang-055.htm.ini | 5 ----- 4 files changed, 20 deletions(-) delete mode 100644 tests/wpt/metadata-css/selectors-3_dev/html/css3-selectors-lang-024.htm.ini delete mode 100644 tests/wpt/metadata-css/selectors-3_dev/html/css3-selectors-lang-035.htm.ini delete mode 100644 tests/wpt/metadata-css/selectors-3_dev/html/css3-selectors-lang-044.htm.ini delete mode 100644 tests/wpt/metadata-css/selectors-3_dev/html/css3-selectors-lang-055.htm.ini diff --git a/tests/wpt/metadata-css/selectors-3_dev/html/css3-selectors-lang-024.htm.ini b/tests/wpt/metadata-css/selectors-3_dev/html/css3-selectors-lang-024.htm.ini deleted file mode 100644 index 8d25a735ed8..00000000000 --- a/tests/wpt/metadata-css/selectors-3_dev/html/css3-selectors-lang-024.htm.ini +++ /dev/null @@ -1,5 +0,0 @@ -[css3-selectors-lang-024.htm] - type: testharness - [A lang|= value will match a lang attribute value regardless of case differences.] - expected: FAIL - diff --git a/tests/wpt/metadata-css/selectors-3_dev/html/css3-selectors-lang-035.htm.ini b/tests/wpt/metadata-css/selectors-3_dev/html/css3-selectors-lang-035.htm.ini deleted file mode 100644 index f1eb302573d..00000000000 --- a/tests/wpt/metadata-css/selectors-3_dev/html/css3-selectors-lang-035.htm.ini +++ /dev/null @@ -1,5 +0,0 @@ -[css3-selectors-lang-035.htm] - type: testharness - [A lang|= value will match a lang attribute value regardless of case differences in the script tag.] - expected: FAIL - diff --git a/tests/wpt/metadata-css/selectors-3_dev/html/css3-selectors-lang-044.htm.ini b/tests/wpt/metadata-css/selectors-3_dev/html/css3-selectors-lang-044.htm.ini deleted file mode 100644 index 0a343c73563..00000000000 --- a/tests/wpt/metadata-css/selectors-3_dev/html/css3-selectors-lang-044.htm.ini +++ /dev/null @@ -1,5 +0,0 @@ -[css3-selectors-lang-044.htm] - type: testharness - [A lang= value will match a lang attribute value regardless of case differences.] - expected: FAIL - diff --git a/tests/wpt/metadata-css/selectors-3_dev/html/css3-selectors-lang-055.htm.ini b/tests/wpt/metadata-css/selectors-3_dev/html/css3-selectors-lang-055.htm.ini deleted file mode 100644 index f8ff6b5f1f9..00000000000 --- a/tests/wpt/metadata-css/selectors-3_dev/html/css3-selectors-lang-055.htm.ini +++ /dev/null @@ -1,5 +0,0 @@ -[css3-selectors-lang-055.htm] - type: testharness - [A lang= value will match a lang attribute value regardless of case differences in the script tag.] - expected: FAIL -