Repository: servo/servo
File: components/script/dom/html/htmldetailselement.rs (Rust)

Code before:
            };

            let this = Trusted::new(self);
            self.owner_global()
                .task_manager()
                .dom_manipulation_task_source()
                .queue(task!(details_notification_task_steps: move || {
                    let this = this.root();
                    if counter == this.toggle_counter.get() {
                        let event = ToggleEvent::new(
                            this.global().as_window(),
                            atom!("toggle"),
                            EventBubbles::DoesNotBubble,
                            EventCancelable::NotCancelable,
                            DOMString::from(old_state),
                            DOMString::from(new_state),
                            None,
                            CanGc::note(),
                        );
                        let event = event.upcast::<Event>();
                        event.fire(this.upcast::<EventTarget>(), CanGc::note());
                    }
                }));
            self.upcast::<Node>().dirty(NodeDamage::Other);

            if self.Open() {
                self.ensure_details_exclusivity(
                    ExclusivityConflictResolution::CloseExistingOpenElement,
                );
            }
        }
    }

    fn children_changed(&self, mutation: &ChildrenMutation, can_gc: CanGc) {
        self.super_type()
            .unwrap()
            .children_changed(mutation, can_gc);

        self.update_shadow_tree_contents(can_gc);
    }

    fn bind_to_tree(&self, context: &BindContext, can_gc: CanGc) {
        self.super_type().unwrap().bind_to_tree(context, can_gc);

        self.update_shadow_tree_contents(can_gc);
        self.update_shadow_tree_styles(can_gc);

        if context.tree_is_in_a_document_tree {
            // If this is true then we can't have been in a document tree previously, so
            // we register ourselves.
            self.owner_document()

Review comment (suggestion):
```suggestion
            // Step 3.2. If oldValue is null and value is not null,
            // then ensure details exclusivity by closing other elements if needed given element.
            if self.Open() {
```
Seems like we are not handling the case where `oldValue` is not null? We only check if `value` is null.

Code after:
            self.toggle_counter.set(counter);
            let (old_state, new_state) = if self.Open() {
                ("closed", "open")
            } else {
                ("open", "closed")
            };

            let this = Trusted::new(self);
            self.owner_global()
                .task_manager()
                .dom_manipulation_task_source()
                .queue(task!(details_notification_task_steps: move || {
                    let this = this.root();
                    if counter == this.toggle_counter.get() {
                        let event = ToggleEvent::new(
                            this.global().as_window(),
                            atom!("toggle"),
                            EventBubbles::DoesNotBubble,
                            EventCancelable::NotCancelable,
                            DOMString::from(old_state),
                            DOMString::from(new_state),
                            None,
                            CanGc::note(),
                        );
                        let event = event.upcast::<Event>();
                        event.fire(this.upcast::<EventTarget>(), CanGc::note());
                    }
                }));
            self.upcast::<Node>().dirty(NodeDamage::Other);

            // Step 3.2. If oldValue is null and value is not null, then ensure details exclusivity
            // by closing other elements if needed given element.
            let was_previously_closed = match mutation {
                AttributeMutation::Set(old) => old.is_none(),
                AttributeMutation::Removed => false,
            };
            if was_previously_closed && self.Open() {
                self.ensure_details_exclusivity(
                    ExclusivityConflictResolution::CloseExistingOpenElement,
                );
            }
        }
    }

    fn children_changed(&self, mutation: &ChildrenMutation, can_gc: CanGc) {
        self.super_type()
            .unwrap()
            .children_changed(mutation, can_gc);

        self.update_shadow_tree_contents(can_gc);
    }