From 14e7574746b7a97e386083a5520d57c576ff5acb Mon Sep 17 00:00:00 2001
From: David Shin <dshin@mozilla.com>
Date: Fri, 27 Oct 2023 18:44:15 +0000
Subject: [PATCH] Bug 1861704: Fix `:has()` side-effect causing DOM mutations.
 r=emilio,layout-reviewers

Previously, if an element was blocking a :has next-sibling match gets deleted,
even if the sibling next to the deleted element would now match, we'd discard
that possibility.

Differential Revision: https://phabricator.services.mozilla.com/D192089
---
 .../invalidation/element/relative_selector.rs | 31 ++++++++++++-------
 .../has-sibling-insertion-removal.html.ini    |  6 ----
 2 files changed, 19 insertions(+), 18 deletions(-)
 delete mode 100644 testing/web-platform/meta/css/selectors/invalidation/has-sibling-insertion-removal.html.ini

diff --git a/servo/components/style/invalidation/element/relative_selector.rs b/servo/components/style/invalidation/element/relative_selector.rs
index a1a47897617a..c3e9f1b6f397 100644
--- a/servo/components/style/invalidation/element/relative_selector.rs
+++ b/servo/components/style/invalidation/element/relative_selector.rs
@@ -50,10 +50,20 @@ pub enum DomMutationOperation {
 }
 
 impl DomMutationOperation {
-    fn accept(&self, d: &Dependency) -> bool {
+    fn accept<E: TElement>(&self, d: &Dependency, e: E) -> bool {
         match self {
-            Self::Insert | Self::Append | Self::Remove => true,
-            Self::SideEffectPrevSibling => d.right_combinator_is_next_sibling(),
+            Self::Insert | Self::Append | Self::Remove => {
+                e.relative_selector_search_direction().is_some()
+            },
+            // `:has(+ .a + .b)` with `.anchor + .a + .remove + .b` - `.a` would be present
+            // in the search path.
+            Self::SideEffectPrevSibling => {
+                e.relative_selector_search_direction().is_some() &&
+                    d.right_combinator_is_next_sibling()
+            },
+            // If an element is being removed and would cause next-sibling match to happen,
+            // e.g. `:has(+ .a)` with `.anchor + .remove + .a`, `.a` isn't yet searched
+            // for relative selector matching.
             Self::SideEffectNextSibling => d.dependency_is_relative_with_single_next_sibling(),
         }
     }
@@ -356,9 +366,6 @@ where
                     dependency.parent.is_some(),
                     "Orphaned inner relative selector?"
                 );
-                if element.relative_selector_search_direction().is_none() {
-                    return;
-                }
                 if element != self.top &&
                     matches!(
                         kind,
@@ -453,7 +460,7 @@ where
             .map(|v| match map.map.id_to_selector.get(v, quirks_mode) {
                 Some(v) => {
                     for dependency in v {
-                        if !operation.accept(dependency) {
+                        if !operation.accept(dependency, element) {
                             continue;
                         }
                         self.add_dependency(dependency, element, scope);
@@ -464,7 +471,7 @@ where
         element.each_class(|v| match map.map.class_to_selector.get(v, quirks_mode) {
             Some(v) => {
                 for dependency in v {
-                    if !operation.accept(dependency) {
+                    if !operation.accept(dependency, element) {
                         continue;
                     }
                     self.add_dependency(dependency, element, scope);
@@ -476,7 +483,7 @@ where
             |v| match map.map.other_attribute_affecting_selectors.get(v) {
                 Some(v) => {
                     for dependency in v {
-                        if !operation.accept(dependency) {
+                        if !operation.accept(dependency, element) {
                             continue;
                         }
                         self.add_dependency(dependency, element, scope);
@@ -496,7 +503,7 @@ where
                 if !dependency.state.intersects(state) {
                     return true;
                 }
-                if !operation.accept(&dependency.dep) {
+                if !operation.accept(&dependency.dep, element) {
                     return true;
                 }
                 self.add_dependency(&dependency.dep, element, scope);
@@ -506,7 +513,7 @@ where
 
         if let Some(v) = map.type_to_selector.get(element.local_name()) {
             for dependency in v {
-                if !operation.accept(dependency) {
+                if !operation.accept(dependency, element) {
                     continue;
                 }
                 self.add_dependency(dependency, element, scope);
@@ -514,7 +521,7 @@ where
         }
 
         for dependency in &map.any_to_selector {
-            if !operation.accept(dependency) {
+            if !operation.accept(dependency, element) {
                 continue;
             }
             self.add_dependency(dependency, element, scope);
diff --git a/testing/web-platform/meta/css/selectors/invalidation/has-sibling-insertion-removal.html.ini b/testing/web-platform/meta/css/selectors/invalidation/has-sibling-insertion-removal.html.ini
deleted file mode 100644
index 50e9ab94e656..000000000000
--- a/testing/web-platform/meta/css/selectors/invalidation/has-sibling-insertion-removal.html.ini
+++ /dev/null
@@ -1,6 +0,0 @@
-[has-sibling-insertion-removal.html]
-  [subject2: color after #sibling2_1 removed should be rgb(0, 128, 0)]
-    expected: FAIL
-
-  [subject4: color after #sibling4_1 removed should be rgb(255, 255, 0)]
-    expected: FAIL