Skip to content

Commit

Permalink
Auto merge of #17806 - emilio:quirks-mode-bloom, r=bholley
Browse files Browse the repository at this point in the history
selectors: Don't track class and id ancestor hashes in quirks mode.

It's incorrect to track classes and id selectors in a quirks-mode document,
since they should match case-insensitively.

Bug: 1382812
Reviewed-by: bholley
MozReview-Commit-ID: 4uvrfYsWb1v

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17806)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Jul 21, 2017
2 parents 3c89486 + f879ebc commit 1e6999b
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 15 deletions.
33 changes: 20 additions & 13 deletions components/selectors/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use attr::{AttrSelectorWithNamespace, ParsedAttrSelectorOperation, AttrSelectorO
use attr::{ParsedCaseSensitivity, SELECTOR_WHITESPACE, NamespaceConstraint};
use bloom::BLOOM_HASH_MASK;
use builder::{SelectorBuilder, SpecificityAndFlags};
use context::QuirksMode;
use cssparser::{ParseError, BasicParseError, CompactCowStr};
use cssparser::{Token, Parser as CssParser, parse_nth, ToCss, serialize_identifier, CssStringWriter};
use precomputed_hash::PrecomputedHash;
Expand Down Expand Up @@ -218,17 +219,21 @@ pub struct AncestorHashes {
}

impl AncestorHashes {
pub fn new<Impl: SelectorImpl>(s: &Selector<Impl>) -> Self {
Self::from_iter(s.iter())
pub fn new<Impl: SelectorImpl>(
selector: &Selector<Impl>,
quirks_mode: QuirksMode,
) -> Self {
Self::from_iter(selector.iter(), quirks_mode)
}

pub fn from_iter<Impl: SelectorImpl>(iter: SelectorIter<Impl>) -> Self {
fn from_iter<Impl: SelectorImpl>(
iter: SelectorIter<Impl>,
quirks_mode: QuirksMode,
) -> Self {
// Compute ancestor hashes for the bloom filter.
let mut hashes = [0u32; 4];
let mut hash_iter = AncestorIter::new(iter)
.map(|x| x.ancestor_hash())
.filter(|x| x.is_some())
.map(|x| x.unwrap());
.filter_map(|x| x.ancestor_hash(quirks_mode));
for i in 0..4 {
hashes[i] = match hash_iter.next() {
Some(x) => x & BLOOM_HASH_MASK,
Expand Down Expand Up @@ -532,7 +537,7 @@ impl<'a, Impl: SelectorImpl> fmt::Debug for SelectorIter<'a, Impl> {
}

/// An iterator over all simple selectors belonging to ancestors.
pub struct AncestorIter<'a, Impl: 'a + SelectorImpl>(SelectorIter<'a, Impl>);
struct AncestorIter<'a, Impl: 'a + SelectorImpl>(SelectorIter<'a, Impl>);
impl<'a, Impl: 'a + SelectorImpl> AncestorIter<'a, Impl> {
/// Creates an AncestorIter. The passed-in iterator is assumed to point to
/// the beginning of the child sequence, which will be skipped.
Expand Down Expand Up @@ -672,12 +677,12 @@ pub enum Component<Impl: SelectorImpl> {

impl<Impl: SelectorImpl> Component<Impl> {
/// Compute the ancestor hash to check against the bloom filter.
pub fn ancestor_hash(&self) -> Option<u32> {
fn ancestor_hash(&self, quirks_mode: QuirksMode) -> Option<u32> {
match *self {
Component::LocalName(LocalName { ref name, ref lower_name }) => {
// Only insert the local-name into the filter if it's all lowercase.
// Otherwise we would need to test both hashes, and our data structures
// aren't really set up for that.
// Only insert the local-name into the filter if it's all
// lowercase. Otherwise we would need to test both hashes, and
// our data structures aren't really set up for that.
if name == lower_name {
Some(name.precomputed_hash())
} else {
Expand All @@ -688,10 +693,12 @@ impl<Impl: SelectorImpl> Component<Impl> {
Component::Namespace(_, ref url) => {
Some(url.precomputed_hash())
},
Component::ID(ref id) => {
// In quirks mode, class and id selectors should match
// case-insensitively, so just avoid inserting them into the filter.
Component::ID(ref id) if quirks_mode != QuirksMode::Quirks => {
Some(id.precomputed_hash())
},
Component::Class(ref class) => {
Component::Class(ref class) if quirks_mode != QuirksMode::Quirks => {
Some(class.precomputed_hash())
},
_ => None,
Expand Down
4 changes: 3 additions & 1 deletion components/style/stylist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,9 @@ impl Stylist {
self.element_map.borrow_for_origin(&origin)
};

let hashes = AncestorHashes::new(&selector);
let hashes =
AncestorHashes::new(&selector, self.quirks_mode);

map.insert(
Rule::new(selector.clone(),
hashes.clone(),
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/style/stylist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ fn get_mock_rules(css_selectors: &[&str]) -> (Vec<Vec<Rule>>, SharedRwLock) {
let guard = shared_lock.read();
let rule = locked.read_with(&guard);
rule.selectors.0.iter().map(|s| {
Rule::new(s.clone(), AncestorHashes::new(s), locked.clone(), i as u32)
Rule::new(s.clone(), AncestorHashes::new(s, QuirksMode::NoQuirks), locked.clone(), i as u32)
}).collect()
}).collect(), shared_lock)
}
Expand Down

0 comments on commit 1e6999b

Please sign in to comment.