Closed Bug 1370502 Opened 7 years ago Closed 7 years ago

stylo: Implement ServoStyleRule::SelectorMatchesElement

Categories

(Core :: CSS Parsing and Computation, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: ferjm, Assigned: ferjm)

References

Details

Attachments

(1 file)

No description provided.
Priority: -- → P2
Assignee: nobody → ferjmoreno
Raising to P1, this seems to be blocking more and more bits of DevTools.
Priority: P2 → P1
Comment on attachment 8882634 [details] Bug 1370502 - stylo: Implement ServoStyleRule::SelectorMatchesElement. https://reviewboard.mozilla.org/r/153720/#review158928 ::: servo/ports/geckolib/glue.rs:1283 (Diff revision 1) > + // matches the selector pseudo element type before proceeding. > + if let Some(ep) = selector_and_hashes.selector.pseudo_element() { > + if *ep != p { > + return false; > + } > + } We also want to return false here, in case the selector doesn't have a pseudo-element, right? ::: servo/ports/geckolib/glue.rs:1294 (Diff revision 1) > + return false; > + } > + }, > + }; > + > + let mut ctx = MatchingContext::new(MatchingMode::Normal, None, QuirksMode::NoQuirks); I'm reasonably sure this should use `MatchingMode::ForStatelessPseudoElement` in the case there's an actual pseudo-element (but not otherwise, since we assert). Also, the quirks mode should be gotten from `aElement->OwnerDoc()` to match gecko.
Attachment #8882634 - Flags: review?(emilio+bugs)
Comment on attachment 8882634 [details] Bug 1370502 - stylo: Implement ServoStyleRule::SelectorMatchesElement. https://reviewboard.mozilla.org/r/153720/#review159084 With those fixed, r=me ::: layout/style/ServoStyleRule.cpp:282 (Diff revision 2) > ServoStyleRule::SelectorMatchesElement(Element* aElement, > uint32_t aSelectorIndex, > const nsAString& aPseudo, > bool* aMatches) > { > - // TODO Bug 1370502 > + nsCOMPtr<nsIAtom> pseudoElt = NS_Atomize(aPseudo); Please verify this does the correct thing for the empty string, or special-case as appropriate. ::: servo/components/style/gecko/wrapper.rs:699 (Diff revision 2) > let node = self.as_node(); > unsafe { Gecko_GetDocumentLWTheme(node.owner_doc()) } > } > + > + /// Quirks mode getter. > + pub fn get_quirks_mode(&self) -> QuirksMode { Let's call this `owner_document_quirks_mode` instead. ::: servo/ports/geckolib/glue.rs:1281 (Diff revision 2) > + match PseudoElement::from_pseudo_type(pseudo_type) { > + Some(p) => { > + // We need to make sure that the requested pseudo element type > + // matches the selector pseudo element type before proceeding. > + match selector_and_hashes.selector.pseudo_element() { > + Some(ep) => if *ep == p { This doesn't return false if `*ep != p`, right? This should probably be: ``` Some(ep) if *ep == p => { /* .. */ } _ => return false, ``` Also, perhaps `p` and `ep` aren't particularly good names, maybe `pseudo` and `selector_pseudo` or something like that?
Attachment #8882634 - Flags: review?(emilio+bugs) → review+
Pushed by ferjmoreno@gmail.com: https://hg.mozilla.org/integration/autoland/rev/936d01cd7b99 stylo: Implement ServoStyleRule::SelectorMatchesElement. r=emilio
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: