Skip to content
This repository has been archived by the owner on Mar 2, 2021. It is now read-only.

Commit

Permalink
Fix a bug where "volatile" attributes weren't getting properly updated
Browse files Browse the repository at this point in the history
  • Loading branch information
fitzgen committed Feb 12, 2019
1 parent 25dcb1c commit 5837d53
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 10 deletions.
30 changes: 28 additions & 2 deletions js/change-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,20 @@ const OP_TABLE = [
const pointer2 = mem32[i++];
const length2 = mem32[i++];
const value = string(mem8, pointer2, length2);
top(changeList.stack).setAttribute(name, value);
const node = top(changeList.stack);
node.setAttribute(name, value);

// Some attributes are "volatile" and don't work through `setAttribute`.
if (name === "value") {
node.value = value;
}
if (name === "checked") {
node.checked = true;
}
if (name === "selected") {
node.selected = true;
}

return i;
},

Expand All @@ -58,7 +71,20 @@ const OP_TABLE = [
const pointer = mem32[i++];
const length = mem32[i++];
const name = string(mem8, pointer, length);
top(changeList.stack).removeAttribute(name);
const node = top(changeList.stack);
node.removeAttribute(name);

// Some attributes are "volatile" and don't work through `removeAttribute`.
if (name === "value") {
node.value = null;
}
if (name === "checked") {
node.checked = false;
}
if (name === "selected") {
node.selected = false;
}

return i;
},

Expand Down
14 changes: 14 additions & 0 deletions src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,20 @@ impl fmt::Debug for Listener<'_> {
}
}

impl<'a> Attribute<'a> {
/// Certain attributes are considered "volatile" and can change via user
/// input that we can't see when diffing against the old virtual DOM. For
/// these attributes, we want to always re-set the attribute on the physical
/// DOM node, even if the old and new virtual DOM nodes have the same value.
#[inline]
pub(crate) fn is_volatile(&self) -> bool {
match self.name {
"value" | "checked" | "selected" => true,
_ => false,
}
}
}

impl<'a> Node<'a> {
/// Construct a new element node with the given tag name and children.
#[inline]
Expand Down
21 changes: 13 additions & 8 deletions src/vdom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,17 +364,22 @@ impl VdomInnerExclusive {
// Do O(n^2) passes to add/update and remove attributes, since
// there are almost always very few attributes.
'outer: for new_attr in new {
for old_attr in old {
if old_attr.name == new_attr.name {
if old_attr.value != new_attr.value {
self.change_list
.emit_set_attribute(new_attr.name, new_attr.value);
if new_attr.is_volatile() {
self.change_list
.emit_set_attribute(new_attr.name, new_attr.value);
} else {
for old_attr in old {
if old_attr.name == new_attr.name {
if old_attr.value != new_attr.value {
self.change_list
.emit_set_attribute(new_attr.name, new_attr.value);
}
continue 'outer;
}
continue 'outer;
}
self.change_list
.emit_set_attribute(new_attr.name, new_attr.value);
}
self.change_list
.emit_set_attribute(new_attr.name, new_attr.value);
}

'outer2: for old_attr in old {
Expand Down

1 comment on commit 5837d53

@alexcrichton
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wut

Please sign in to comment.