servo: Merge #18358 - Properly change the kind of the given attribute in SetAttributeNode (from servo:change-attr-kind); r=emilio
authorAnthony Ramine <n.oxyde@gmail.com>
Sun, 03 Sep 2017 13:05:00 -0500
changeset 427992 c44410214970dd92d780b401dade5b2316b90735
parent 427991 3ba65c513e8cc2db39f2c0d6cde27aa26b5b0134
child 427993 d225dbb6b59f6886d93665fcb4aa008aa001db63
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #18358 - Properly change the kind of the given attribute in SetAttributeNode (from servo:change-attr-kind); r=emilio Fixes #18357. Source-Repo: https://github.com/servo/servo Source-Revision: 096912101b2ea8702a0337aae092241b930d6b5d
servo/components/script/dom/element.rs
--- a/servo/components/script/dom/element.rs
+++ b/servo/components/script/dom/element.rs
@@ -1644,27 +1644,32 @@ impl ElementMethods for Element {
         self.set_first_matching_attribute(
             local_name.clone(), value, qualified_name, namespace.clone(), prefix,
             |attr| *attr.local_name() == local_name && *attr.namespace() == namespace);
         Ok(())
     }
 
     // https://dom.spec.whatwg.org/#dom-element-setattributenode
     fn SetAttributeNode(&self, attr: &Attr) -> Fallible<Option<Root<Attr>>> {
-        // Workaround for https://github.com/servo/servo/issues/17366
-        // This ensures that if this is an "id" attr, its value is an Atom
-        attr.swap_value(&mut self.parse_plain_attribute(attr.local_name(), attr.Value()));
-
         // Step 1.
         if let Some(owner) = attr.GetOwnerElement() {
             if &*owner != self {
                 return Err(Error::InUseAttribute);
             }
         }
 
+        let vtable = vtable_for(self.upcast());
+
+        // This ensures that the attribute is of the expected kind for this
+        // specific element. This is inefficient and should probably be done
+        // differently.
+        attr.swap_value(
+            &mut vtable.parse_plain_attribute(attr.local_name(), attr.Value()),
+        );
+
         // Step 2.
         let position = self.attrs.borrow().iter().position(|old_attr| {
             attr.namespace() == old_attr.namespace() && attr.local_name() == old_attr.local_name()
         });
 
         if let Some(position) = position {
             let old_attr = Root::from_ref(&*self.attrs.borrow()[position]);
 
@@ -1683,17 +1688,17 @@ impl ElementMethods for Element {
                     Some(new_value), namespace);
                 ScriptThread::enqueue_callback_reaction(self, reaction, None);
             }
             self.will_mutate_attr(attr);
             attr.set_owner(Some(self));
             self.attrs.borrow_mut()[position] = JS::from_ref(attr);
             old_attr.set_owner(None);
             if attr.namespace() == &ns!() {
-                vtable_for(self.upcast()).attribute_mutated(
+                vtable.attribute_mutated(
                     &attr, AttributeMutation::Set(Some(&old_attr.value())));
             }
 
             // Step 6.
             Ok(Some(old_attr))
         } else {
             // Step 5.
             attr.set_owner(Some(self));