Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shrink selectors::Component, implement attr selector (in)case-sensitivity #16915

Merged
merged 12 commits into from May 19, 2017

Add more enum types to avoid unreachable!() for selector case-sensiti…

…vity
  • Loading branch information
SimonSapin committed May 18, 2017
commit 76166bce585cfcbe8bea4fa2538fec2ec34ca90e
@@ -12,7 +12,7 @@ pub struct AttrSelectorWithNamespace<Impl: SelectorImpl> {
pub namespace: NamespaceConstraint<(Impl::NamespacePrefix, Impl::NamespaceUrl)>,
pub local_name: Impl::LocalName,
pub local_name_lower: Impl::LocalName,
pub operation: AttrSelectorOperation<Impl::AttrValue>,
pub operation: ParsedAttrSelectorOperation<Impl::AttrValue>,
pub never_matches: bool,
}

@@ -35,6 +35,16 @@ pub enum NamespaceConstraint<NamespaceUrl> {
Specific(NamespaceUrl),
}

#[derive(Eq, PartialEq, Clone)]
pub enum ParsedAttrSelectorOperation<AttrValue> {
Exists,
WithValue {
operator: AttrSelectorOperator,
case_sensitivity: ParsedCaseSensitivity,
expected_value: AttrValue,
}
}

#[derive(Eq, PartialEq, Clone)]
pub enum AttrSelectorOperation<AttrValue> {
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()");
}
}
}
}
@@ -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<E, F>(
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<E, F>(
&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,
}
}
@@ -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<Impl: SelectorImpl> {
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::<Component>() small.
@@ -735,9 +735,9 @@ impl<Impl: SelectorImpl> ToCss for Component<Impl> {
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<Impl: SelectorImpl> ToCss for AttrSelectorWithNamespace<Impl> {
}
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<P, Impl>(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<P, Impl>(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<P, Impl>(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<P, Impl>(parser: &P, input: &mut CssParser)
}


fn parse_attribute_flags(input: &mut CssParser) -> Result<CaseSensitivity, ()> {
fn parse_attribute_flags(input: &mut CssParser) -> Result<ParsedCaseSensitivity, ()> {
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),
@@ -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 {
@@ -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 {
@@ -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,
}
]),
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.