Auto merge of #25654 - pshaughn:domtokennull, r=jdm

fix DOMTokenList

I fixed the bug described in 25128, and also the other cases in the same test (order of elements after replace).

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25128

<!-- Either: -->
- [X] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
This commit is contained in:
bors-servo 2020-01-31 04:08:55 -05:00 committed by GitHub
commit 7ac4f9eec6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 33 additions and 90 deletions

View file

@ -52,6 +52,17 @@ impl DOMTokenList {
slice => Ok(Atom::from(slice)),
}
}
// https://dom.spec.whatwg.org/#concept-dtl-update
fn perform_update_steps(&self, atoms: Vec<Atom>) {
// Step 1
if !self.element.has_attribute(&self.local_name) && atoms.len() == 0 {
return;
}
// step 2
self.element
.set_atomic_tokenlist_attribute(&self.local_name, atoms)
}
}
// https://dom.spec.whatwg.org/#domtokenlist
@ -93,8 +104,7 @@ impl DOMTokenListMethods for DOMTokenList {
atoms.push(token);
}
}
self.element
.set_atomic_tokenlist_attribute(&self.local_name, atoms);
self.perform_update_steps(atoms);
Ok(())
}
@ -108,8 +118,7 @@ impl DOMTokenListMethods for DOMTokenList {
.position(|atom| *atom == token)
.map(|index| atoms.remove(index));
}
self.element
.set_atomic_tokenlist_attribute(&self.local_name, atoms);
self.perform_update_steps(atoms);
Ok(())
}
@ -122,8 +131,7 @@ impl DOMTokenListMethods for DOMTokenList {
Some(true) => Ok(true),
_ => {
atoms.remove(index);
self.element
.set_atomic_tokenlist_attribute(&self.local_name, atoms);
self.perform_update_steps(atoms);
Ok(false)
},
},
@ -131,8 +139,7 @@ impl DOMTokenListMethods for DOMTokenList {
Some(false) => Ok(false),
_ => {
atoms.push(token);
self.element
.set_atomic_tokenlist_attribute(&self.local_name, atoms);
self.perform_update_steps(atoms);
Ok(true)
},
},
@ -166,14 +173,27 @@ impl DOMTokenListMethods for DOMTokenList {
let mut atoms = self.element.get_tokenlist_attribute(&self.local_name);
let mut result = false;
if let Some(pos) = atoms.iter().position(|atom| *atom == token) {
if !atoms.contains(&new_token) {
atoms[pos] = new_token;
if let Some(redundant_pos) = atoms.iter().position(|atom| *atom == new_token) {
if redundant_pos > pos {
// The replacement is already in the list, later,
// so we perform the replacement and remove the
// later copy.
atoms[pos] = new_token;
atoms.remove(redundant_pos);
} else if redundant_pos < pos {
// The replacement is already in the list, earlier,
// so we remove the index where we'd be putting the
// later copy.
atoms.remove(pos);
}
// else we are replacing the token with itself, nothing to change
} else {
atoms.remove(pos);
// The replacement is not in the list already
atoms[pos] = new_token;
}
// Step 5.
self.element
.set_atomic_tokenlist_attribute(&self.local_name, atoms);
self.perform_update_steps(atoms);
result = true;
}
Ok(result)

View file

@ -1,77 +0,0 @@
[Element-classlist.html]
type: testharness
[classList.remove("a") with attribute value null (HTML node)]
expected: FAIL
[classList.remove("a", "b") with attribute value null (HTML node)]
expected: FAIL
[classList.replace("a", "a") with attribute value "a" (HTML node)]
expected: FAIL
[classList.replace("a", "a") with attribute value "a a a b" (HTML node)]
expected: FAIL
[classList.replace("c", "a") with attribute value "c b a" (HTML node)]
expected: FAIL
[classList.remove("a") with attribute value null (XHTML node)]
expected: FAIL
[classList.remove("a", "b") with attribute value null (XHTML node)]
expected: FAIL
[classList.replace("a", "a") with attribute value "a" (XHTML node)]
expected: FAIL
[classList.replace("a", "a") with attribute value "a a a b" (XHTML node)]
expected: FAIL
[classList.replace("c", "a") with attribute value "c b a" (XHTML node)]
expected: FAIL
[classList.remove("a") with attribute value null (MathML node)]
expected: FAIL
[classList.remove("a", "b") with attribute value null (MathML node)]
expected: FAIL
[classList.replace("a", "a") with attribute value "a" (MathML node)]
expected: FAIL
[classList.replace("a", "a") with attribute value "a a a b" (MathML node)]
expected: FAIL
[classList.replace("c", "a") with attribute value "c b a" (MathML node)]
expected: FAIL
[classList.remove("a") with attribute value null (XML node with null namespace)]
expected: FAIL
[classList.remove("a", "b") with attribute value null (XML node with null namespace)]
expected: FAIL
[classList.replace("a", "a") with attribute value "a" (XML node with null namespace)]
expected: FAIL
[classList.replace("a", "a") with attribute value "a a a b" (XML node with null namespace)]
expected: FAIL
[classList.replace("c", "a") with attribute value "c b a" (XML node with null namespace)]
expected: FAIL
[classList.remove("a") with attribute value null (foo node)]
expected: FAIL
[classList.remove("a", "b") with attribute value null (foo node)]
expected: FAIL
[classList.replace("a", "a") with attribute value "a" (foo node)]
expected: FAIL
[classList.replace("a", "a") with attribute value "a a a b" (foo node)]
expected: FAIL
[classList.replace("c", "a") with attribute value "c b a" (foo node)]
expected: FAIL