From e5880136bb515740a3badc99c11e08272494b02e Mon Sep 17 00:00:00 2001
From: Dietmar Maurer <dietmar@proxmox.com>
Date: Thu, 5 May 2022 08:20:05 +0200
Subject: [PATCH] Attributes: Fix apply_diff_index_maps (#2653)

* Attributes: Fix apply_diff_index_maps

The old algorithm simply stops when key name/order changes, which
is simply wrong.

Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>

* add test case for test for bug #2653
---
 .../yew/src/dom_bundle/btag/attributes.rs     | 50 +++++--------------
 packages/yew/src/dom_bundle/btag/mod.rs       | 42 ++++++++++++++++
 2 files changed, 54 insertions(+), 38 deletions(-)

diff --git a/packages/yew/src/dom_bundle/btag/attributes.rs b/packages/yew/src/dom_bundle/btag/attributes.rs
index 601da687899..05bbf507286 100644
--- a/packages/yew/src/dom_bundle/btag/attributes.rs
+++ b/packages/yew/src/dom_bundle/btag/attributes.rs
@@ -1,5 +1,4 @@
 use std::collections::HashMap;
-use std::iter;
 use std::ops::Deref;
 
 use indexmap::IndexMap;
@@ -91,45 +90,20 @@ impl Attributes {
         new: &IndexMap<AttrValue, AttrValue>,
         old: &IndexMap<AttrValue, AttrValue>,
     ) {
-        let mut old_iter = old.iter();
-        let mut new_iter = new.iter();
-        loop {
-            match (new_iter.next(), old_iter.next()) {
-                (Some((new_key, new_value)), Some((old_key, old_value))) => {
-                    if new_key != old_key {
-                        break;
+        for (key, value) in new.iter() {
+            match old.get(key) {
+                Some(old_value) => {
+                    if value != old_value {
+                        Self::set_attribute(el, key, value);
                     }
-                    if new_value != old_value {
-                        Self::set_attribute(el, new_key, new_value);
-                    }
-                }
-                // new attributes
-                (Some(attr), None) => {
-                    for (key, value) in iter::once(attr).chain(new_iter) {
-                        match old.get(key) {
-                            Some(old_value) => {
-                                if value != old_value {
-                                    Self::set_attribute(el, key, value);
-                                }
-                            }
-                            None => {
-                                Self::set_attribute(el, key, value);
-                            }
-                        }
-                    }
-                    break;
                 }
-                // removed attributes
-                (None, Some(attr)) => {
-                    for (key, _) in iter::once(attr).chain(old_iter) {
-                        let key = key;
-                        if !new.contains_key(key) {
-                            Self::remove_attribute(el, key);
-                        }
-                    }
-                    break;
-                }
-                (None, None) => break,
+                None => Self::set_attribute(el, key, value),
+            }
+        }
+
+        for (key, _value) in old.iter() {
+            if !new.contains_key(key) {
+                Self::remove_attribute(el, key);
             }
         }
     }
diff --git a/packages/yew/src/dom_bundle/btag/mod.rs b/packages/yew/src/dom_bundle/btag/mod.rs
index aeaa19a519e..9645123029f 100644
--- a/packages/yew/src/dom_bundle/btag/mod.rs
+++ b/packages/yew/src/dom_bundle/btag/mod.rs
@@ -931,6 +931,48 @@ mod tests {
             "<div id=\"after\"></div>"
         );
     }
+
+    // test for bug: https://github.com/yewstack/yew/pull/2653
+    #[test]
+    fn test_index_map_attribute_diff() {
+        let (root, scope, parent) = setup_parent();
+
+        let test_ref = NodeRef::default();
+
+        // We want to test appy_diff with Attributes::IndexMap, so we
+        // need to create the VTag manually
+
+        // Create <div disabled="disabled" tabindex="0">
+        let mut vtag = VTag::new("div");
+        vtag.node_ref = test_ref.clone();
+        vtag.add_attribute("disabled", "disabled");
+        vtag.add_attribute("tabindex", "0");
+
+        let elem = VNode::VTag(Box::new(vtag));
+
+        let (_, mut elem) = elem.attach(&root, &scope, &parent, NodeRef::default());
+
+        // Create <div tabindex="0"> (removed first attribute "disabled")
+        let mut vtag = VTag::new("div");
+        vtag.node_ref = test_ref.clone();
+        vtag.add_attribute("tabindex", "0");
+        let next_elem = VNode::VTag(Box::new(vtag));
+        let elem_vtag = assert_vtag(next_elem);
+
+        // Sync happens here
+        // this should remove the the "disabled" attribute
+        elem_vtag.reconcile_node(&root, &scope, &parent, NodeRef::default(), &mut elem);
+
+        assert_eq!(
+            test_ref
+                .get()
+                .unwrap()
+                .dyn_ref::<web_sys::Element>()
+                .unwrap()
+                .outer_html(),
+            "<div tabindex=\"0\"></div>"
+        )
+    }
 }
 
 #[cfg(all(test, feature = "wasm_test"))]