Auto merge of #19027 - emilio:svg-use-bug, r=upsuper

selectors: Be consistent about how we get a parent element for selector matching

This fixes bug 1412011.

<!-- 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/19027)
<!-- Reviewable:end -->

1 files changed, 93 insertions, 71 deletions

diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs
index 7b505779315..d55fdae9d90 100644
--- a/components/selectors/matching.rs
+++ b/components/selectors/matching.rs
@@ -486,6 +486,32 @@ enum Rightmost {
     No,
 }
 
+#[inline(always)]
+fn next_element_for_combinator<E>(
+    element: &E,
+    combinator: Combinator,
+) -> Option<E>
+where
+    E: Element,
+{
+    match combinator {
+        Combinator::NextSibling |
+        Combinator::LaterSibling => {
+            element.prev_sibling_element()
+        }
+        Combinator::Child |
+        Combinator::Descendant => {
+            if element.blocks_ancestor_combinators() {
+                return None;
+            }
+            element.parent_element()
+        }
+        Combinator::PseudoElement => {
+            element.pseudo_element_originating_element()
+        }
+    }
+}
+
 fn matches_complex_selector_internal<E, F>(
     mut selector_iter: SelectorIter<E::Impl>,
     element: &E,
@@ -524,8 +550,7 @@ where
     };
 
     let combinator = selector_iter.next_sequence();
-    let siblings = combinator.map_or(false, |c| c.is_sibling());
-    if siblings {
+    if combinator.map_or(false, |c| c.is_sibling()) {
         flags_setter(element, HAS_SLOW_SELECTOR_LATER_SIBLINGS);
     }
 
@@ -533,78 +558,75 @@ where
         return SelectorMatchingResult::NotMatchedAndRestartFromClosestLaterSibling;
     }
 
-    match combinator {
-        None => SelectorMatchingResult::Matched,
-        Some(c) => {
-            let (mut next_element, candidate_not_found) = match c {
-                Combinator::NextSibling | Combinator::LaterSibling => {
-                    // Only ancestor combinators are allowed while looking for
-                    // relevant links, so switch to not looking.
-                    *relevant_link = RelevantLinkStatus::NotLooking;
-                    (element.prev_sibling_element(),
-                     SelectorMatchingResult::NotMatchedAndRestartFromClosestDescendant)
-                }
-                Combinator::Child | Combinator::Descendant => {
-                    if element.blocks_ancestor_combinators() {
-                        (None, SelectorMatchingResult::NotMatchedGlobally)
-                    } else {
-                        (element.parent_element(),
-                         SelectorMatchingResult::NotMatchedGlobally)
-                    }
-                }
-                Combinator::PseudoElement => {
-                    (element.pseudo_element_originating_element(),
-                     SelectorMatchingResult::NotMatchedGlobally)
-                }
-            };
+    let combinator = match combinator {
+        None => return SelectorMatchingResult::Matched,
+        Some(c) => c,
+    };
 
-            loop {
-                let element = match next_element {
-                    None => return candidate_not_found,
-                    Some(next_element) => next_element,
-                };
-                let result = matches_complex_selector_internal(
-                    selector_iter.clone(),
-                    &element,
-                    context,
-                    relevant_link,
-                    flags_setter,
-                    Rightmost::No,
-                );
-                match (result, c) {
-                    // Return the status immediately.
-                    (SelectorMatchingResult::Matched, _) => return result,
-                    (SelectorMatchingResult::NotMatchedGlobally, _) => return result,
-
-                    // Upgrade the failure status to
-                    // NotMatchedAndRestartFromClosestDescendant.
-                    (_, Combinator::PseudoElement) |
-                    (_, Combinator::Child) => return SelectorMatchingResult::NotMatchedAndRestartFromClosestDescendant,
-
-                    // Return the status directly.
-                    (_, Combinator::NextSibling) => return result,
-
-                    // If the failure status is NotMatchedAndRestartFromClosestDescendant
-                    // and combinator is Combinator::LaterSibling, give up this Combinator::LaterSibling matching
-                    // and restart from the closest descendant combinator.
-                    (SelectorMatchingResult::NotMatchedAndRestartFromClosestDescendant, Combinator::LaterSibling)
-                        => return result,
-
-                    // The Combinator::Descendant combinator and the status is
-                    // NotMatchedAndRestartFromClosestLaterSibling or
-                    // NotMatchedAndRestartFromClosestDescendant,
-                    // or the Combinator::LaterSibling combinator and the status is
-                    // NotMatchedAndRestartFromClosestDescendant
-                    // can continue to matching on the next candidate element.
-                    _ => {},
-                }
-                next_element = if siblings {
-                    element.prev_sibling_element()
-                } else {
-                    element.parent_element()
-                };
+    let candidate_not_found = match combinator {
+        Combinator::NextSibling |
+        Combinator::LaterSibling => {
+            // Only ancestor combinators are allowed while looking for
+            // relevant links, so switch to not looking.
+            *relevant_link = RelevantLinkStatus::NotLooking;
+             SelectorMatchingResult::NotMatchedAndRestartFromClosestDescendant
+        }
+        Combinator::Child |
+        Combinator::Descendant |
+        Combinator::PseudoElement => {
+             SelectorMatchingResult::NotMatchedGlobally
+        }
+    };
+
+    let mut next_element = next_element_for_combinator(element, combinator);
+
+    loop {
+        let element = match next_element {
+            None => return candidate_not_found,
+            Some(next_element) => next_element,
+        };
+        let result = matches_complex_selector_internal(
+            selector_iter.clone(),
+            &element,
+            context,
+            relevant_link,
+            flags_setter,
+            Rightmost::No,
+        );
+
+        match (result, combinator) {
+            // Return the status immediately.
+            (SelectorMatchingResult::Matched, _) |
+            (SelectorMatchingResult::NotMatchedGlobally, _) |
+            (_, Combinator::NextSibling) => {
+                return result;
+            }
+
+            // Upgrade the failure status to
+            // NotMatchedAndRestartFromClosestDescendant.
+            (_, Combinator::PseudoElement) |
+            (_, Combinator::Child) => {
+                return SelectorMatchingResult::NotMatchedAndRestartFromClosestDescendant;
             }
+
+            // If the failure status is
+            // NotMatchedAndRestartFromClosestDescendant and combinator is
+            // Combinator::LaterSibling, give up this Combinator::LaterSibling
+            // matching and restart from the closest descendant combinator.
+            (SelectorMatchingResult::NotMatchedAndRestartFromClosestDescendant, Combinator::LaterSibling) => {
+                return result;
+            }
+
+            // The Combinator::Descendant combinator and the status is
+            // NotMatchedAndRestartFromClosestLaterSibling or
+            // NotMatchedAndRestartFromClosestDescendant, or the
+            // Combinator::LaterSibling combinator and the status is
+            // NotMatchedAndRestartFromClosestDescendant, we can continue to
+            // matching on the next candidate element.
+            _ => {},
         }
+
+        next_element = next_element_for_combinator(&element, combinator);
     }
 }