Closed
Bug 1370502
Opened 7 years ago
Closed 7 years ago
stylo: Implement ServoStyleRule::SelectorMatchesElement
Categories
(Core :: CSS Parsing and Computation, enhancement, P1)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: ferjm, Assigned: ferjm)
References
Details
Attachments
(1 file)
No description provided.
Updated•7 years ago
|
Priority: -- → P2
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → ferjmoreno
Blocks: 1375150
Raising to P1, this seems to be blocking more and more bits of DevTools.
Priority: P2 → P1
Comment hidden (mozreview-request) |
Comment 3•7 years ago
|
||
mozreview-review |
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 hidden (mozreview-request) |
Comment 5•7 years ago
|
||
mozreview-review |
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+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Pushed by ferjmoreno@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/936d01cd7b99
stylo: Implement ServoStyleRule::SelectorMatchesElement. r=emilio
Comment 9•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•