From 8c5005fc44a61326579745dbfe094ea573581446 Mon Sep 17 00:00:00 2001 From: Connor Brewster Date: Wed, 19 Jul 2017 12:58:15 -0600 Subject: [PATCH 01/10] Add construction stack --- components/script/dom/bindings/interface.rs | 47 +++++++++++++------ .../script/dom/customelementregistry.rs | 9 ++++ 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/components/script/dom/bindings/interface.rs b/components/script/dom/bindings/interface.rs index 887a62f11eb..0df46f40241 100644 --- a/components/script/dom/bindings/interface.rs +++ b/components/script/dom/bindings/interface.rs @@ -80,6 +80,7 @@ use dom::bindings::guard::Guard; use dom::bindings::js::Root; use dom::bindings::utils::{DOM_PROTOTYPE_SLOT, ProtoOrIfaceArray, get_proto_or_iface_array}; use dom::create::create_native_html_element; +use dom::customelementregistry::ConstructionStackEntry; use dom::element::{Element, ElementCreator}; use dom::htmlelement::HTMLElement; use dom::window::Window; @@ -281,24 +282,42 @@ pub unsafe fn html_constructor(window: &Window, call_args: &CallArgs) -> Fall } } - // Step 8.1 - let name = QualName::new(None, ns!(html), definition.local_name.clone()); - let element = if definition.is_autonomous() { - Root::upcast(HTMLElement::new(name.local, None, &*document)) - } else { - create_native_html_element(name, None, &*document, ElementCreator::ScriptCreated) - }; + let entry = definition.construction_stack.borrow().last().cloned(); + match entry { + // Step 8 + None => { + // Step 8.1 + let name = QualName::new(None, ns!(html), definition.local_name.clone()); + let element = if definition.is_autonomous() { + Root::upcast(HTMLElement::new(name.local, None, &*document)) + } else { + create_native_html_element(name, None, &*document, ElementCreator::ScriptCreated) + }; - // Step 8.2 is performed in the generated caller code. + // Step 8.2 is performed in the generated caller code. - // TODO: Step 8.3 - 8.4 - // Set the element's custom element state and definition. + // TODO: Step 8.3 - 8.4 + // Set the element's custom element state and definition. + element.set_custom_element_definition(definition.clone()); - // Step 8.5 - Root::downcast(element).ok_or(Error::InvalidState) + // Step 8.5 + Root::downcast(element).ok_or(Error::InvalidState) + }, + // Step 9 + Some(ConstructionStackEntry::Element(element)) => { + // Step 11 is performed in the generated caller code. - // TODO: Steps 9-13 - // Custom element upgrades are not implemented yet, so these steps are unnecessary. + // Step 12 + let mut construction_stack = definition.construction_stack.borrow_mut(); + construction_stack.pop(); + construction_stack.push(ConstructionStackEntry::AlreadyConstructedMarker); + + // Step 13 + Root::downcast(element).ok_or(Error::InvalidState) + }, + // Step 10 + Some(ConstructionStackEntry::AlreadyConstructedMarker) => Err(Error::InvalidState), + } } pub fn push_new_element_queue() { diff --git a/components/script/dom/customelementregistry.rs b/components/script/dom/customelementregistry.rs index 89296884080..42ffb6c5eea 100644 --- a/components/script/dom/customelementregistry.rs +++ b/components/script/dom/customelementregistry.rs @@ -366,6 +366,12 @@ pub struct LifecycleCallbacks { attribute_changed_callback: Option>, } +#[derive(HeapSizeOf, JSTraceable, Clone)] +pub enum ConstructionStackEntry { + Element(Root), + AlreadyConstructedMarker, +} + /// https://html.spec.whatwg.org/multipage/#custom-element-definition #[derive(HeapSizeOf, JSTraceable, Clone)] pub struct CustomElementDefinition { @@ -379,6 +385,8 @@ pub struct CustomElementDefinition { pub observed_attributes: Vec, pub callbacks: LifecycleCallbacks, + + pub construction_stack: DOMRefCell>, } impl CustomElementDefinition { @@ -394,6 +402,7 @@ impl CustomElementDefinition { constructor: constructor, observed_attributes: observed_attributes, callbacks: callbacks, + construction_stack: Default::default(), } } From ec528e944a922ec4786b935be5a25e365174d1a1 Mon Sep 17 00:00:00 2001 From: Connor Brewster Date: Wed, 19 Jul 2017 12:58:37 -0600 Subject: [PATCH 02/10] Implement element upgrade algorithm --- .../script/dom/customelementregistry.rs | 70 ++++++++++++++++++- 1 file changed, 68 insertions(+), 2 deletions(-) diff --git a/components/script/dom/customelementregistry.rs b/components/script/dom/customelementregistry.rs index 42ffb6c5eea..b2798267a86 100644 --- a/components/script/dom/customelementregistry.rs +++ b/components/script/dom/customelementregistry.rs @@ -20,14 +20,14 @@ use dom::domexception::{DOMErrorName, DOMException}; use dom::element::Element; use dom::globalscope::GlobalScope; use dom::htmlelement::HTMLElement; -use dom::node::Node; +use dom::node::{document_from_node, Node, window_from_node}; use dom::promise::Promise; use dom::window::Window; use dom_struct::dom_struct; use html5ever::{LocalName, Namespace, Prefix}; use js::conversions::ToJSValConvertible; use js::jsapi::{Construct1, IsCallable, IsConstructor, HandleValueArray, HandleObject, MutableHandleValue}; -use js::jsapi::{Heap, JS_GetProperty, JSAutoCompartment, JSContext}; +use js::jsapi::{Heap, JS_GetProperty, JS_SameValue, JSAutoCompartment, JSContext}; use js::jsval::{JSVal, NullValue, ObjectValue, UndefinedValue}; use microtask::Microtask; use script_thread::ScriptThread; @@ -462,6 +462,72 @@ impl CustomElementDefinition { } } +/// https://html.spec.whatwg.org/multipage/#concept-upgrade-an-element +#[allow(unsafe_code)] +fn upgrade_element(definition: Rc, element: &Element) { + // TODO: Steps 1-2 + // Track custom element state + + // Step 3 + for attr in element.attrs().iter() { + let local_name = attr.local_name().clone(); + let value = DOMString::from(&**attr.value()); + let namespace = attr.namespace().clone(); + ScriptThread::enqueue_callback_reaction(element, + CallbackReaction::AttributeChanged(local_name, None, Some(value), namespace)); + } + + // Step 4 + if element.is_connected() { + ScriptThread::enqueue_callback_reaction(element, CallbackReaction::Connected); + } + + // Step 5 + definition.construction_stack.borrow_mut().push(ConstructionStackEntry::Element(Root::from_ref(element))); + + // Step 6 + let window = window_from_node(element); + let cx = window.get_cx(); + rooted!(in(cx) let constructor = ObjectValue(definition.constructor.callback())); + rooted!(in(cx) let mut element_val = UndefinedValue()); + unsafe { element.to_jsval(cx, element_val.handle_mut()); } + rooted!(in(cx) let mut construct_result = ptr::null_mut()); + { + // Go into the constructor's compartment + let _ac = JSAutoCompartment::new(cx, definition.constructor.callback()); + let args = HandleValueArray::new(); + // Step 7 + if unsafe { !Construct1(cx, constructor.handle(), &args, construct_result.handle_mut()) } { + // TODO: Catch exceptions + return; + } + // Step 8 + let mut same = false; + rooted!(in(cx) let construct_result_val = ObjectValue(construct_result.get())); + if unsafe { !JS_SameValue(cx, construct_result_val.handle(), element_val.handle(), &mut same) } { + // TODO: Catch exceptions + return; + } + if !same { + // TODO: Throw InvalidStateError + } + } + definition.construction_stack.borrow_mut().pop(); + // TODO: Handle Exceptions + + element.set_custom_element_definition(definition); +} + +/// https://html.spec.whatwg.org/multipage/#concept-try-upgrade +pub fn try_upgrade_element(element: &Element) { + // Step 1 + let document = document_from_node(element); + if let Some(definition) = document.lookup_custom_element_definition(element.local_name().clone(), element.get_is()) { + // Step 2 + ScriptThread::enqueue_upgrade_reaction(element, definition); + } +} + #[derive(HeapSizeOf, JSTraceable)] #[must_root] pub enum CustomElementReaction { From e83a0045f9016b7d03631389107c1748a27fdb0c Mon Sep 17 00:00:00 2001 From: Connor Brewster Date: Wed, 19 Jul 2017 13:10:54 -0600 Subject: [PATCH 03/10] Add upgrade reaction --- components/script/dom/customelementregistry.rs | 14 +++++++++++++- components/script/dom/element.rs | 4 ++++ components/script/script_thread.rs | 13 +++++++++++-- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/components/script/dom/customelementregistry.rs b/components/script/dom/customelementregistry.rs index b2798267a86..fdf197d7d97 100644 --- a/components/script/dom/customelementregistry.rs +++ b/components/script/dom/customelementregistry.rs @@ -531,7 +531,10 @@ pub fn try_upgrade_element(element: &Element) { #[derive(HeapSizeOf, JSTraceable)] #[must_root] pub enum CustomElementReaction { - // TODO: Support upgrade reactions + Upgrade( + #[ignore_heap_size_of = "Rc"] + Rc + ), Callback( #[ignore_heap_size_of = "Rc"] Rc, @@ -545,6 +548,7 @@ impl CustomElementReaction { pub fn invoke(&self, element: &Element) { // Step 2.1 match *self { + CustomElementReaction::Upgrade(ref definition) => upgrade_element(definition.clone(), element), CustomElementReaction::Callback(ref callback, ref arguments) => { let arguments = arguments.iter().map(|arg| arg.handle()).collect(); let _ = callback.Call_(&*element, arguments, ExceptionHandling::Report); @@ -703,6 +707,14 @@ impl CustomElementReactionStack { // Step 6 self.enqueue_element(element); } + + /// https://html.spec.whatwg.org/multipage/#enqueue-a-custom-element-upgrade-reaction + pub fn enqueue_upgrade_reaction(&self, element: &Element, definition: Rc) { + // Step 1 + element.push_upgrade_reaction(definition); + // Step 2 + self.enqueue_element(element); + } } /// https://html.spec.whatwg.org/multipage/#element-queue diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 328dd76e822..b44757d3aa1 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -300,6 +300,10 @@ impl Element { self.custom_element_reaction_queue.borrow_mut().push(CustomElementReaction::Callback(function, args)); } + pub fn push_upgrade_reaction(&self, definition: Rc) { + self.custom_element_reaction_queue.borrow_mut().push(CustomElementReaction::Upgrade(definition)); + } + pub fn invoke_reactions(&self) { let mut reaction_queue = self.custom_element_reaction_queue.borrow_mut(); for reaction in reaction_queue.iter() { diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index e87938bfa1e..49c09787ecc 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -39,7 +39,7 @@ use dom::bindings::str::DOMString; use dom::bindings::structuredclone::StructuredCloneData; use dom::bindings::trace::JSTraceable; use dom::bindings::utils::WRAP_CALLBACKS; -use dom::customelementregistry::{CallbackReaction, CustomElementReactionStack}; +use dom::customelementregistry::{CallbackReaction, CustomElementDefinition, CustomElementReactionStack}; use dom::document::{Document, DocumentSource, FocusType, HasBrowsingContext, IsHTMLDocument, TouchEventResult}; use dom::element::Element; use dom::event::{Event, EventBubbles, EventCancelable}; @@ -774,7 +774,7 @@ impl ScriptThread { }) } - pub fn enqueue_callback_reaction(element:&Element, reaction: CallbackReaction) { + pub fn enqueue_callback_reaction(element: &Element, reaction: CallbackReaction) { SCRIPT_THREAD_ROOT.with(|root| { if let Some(script_thread) = root.get() { let script_thread = unsafe { &*script_thread }; @@ -783,6 +783,15 @@ impl ScriptThread { }) } + pub fn enqueue_upgrade_reaction(element: &Element, definition: Rc) { + SCRIPT_THREAD_ROOT.with(|root| { + if let Some(script_thread) = root.get() { + let script_thread = unsafe { &*script_thread }; + script_thread.custom_element_reaction_stack.enqueue_upgrade_reaction(element, definition); + } + }) + } + pub fn invoke_backup_element_queue() { SCRIPT_THREAD_ROOT.with(|root| { if let Some(script_thread) = root.get() { From 37cbc857276578617f9771bb984700a00f132a3f Mon Sep 17 00:00:00 2001 From: Connor Brewster Date: Thu, 20 Jul 2017 16:35:01 -0600 Subject: [PATCH 04/10] Use HTMLElement interface for possible CEs --- components/script/dom/create.rs | 5 ++++- components/script/dom/customelementregistry.rs | 2 +- tests/wpt/metadata/html/semantics/interfaces.html.ini | 6 ------ 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/components/script/dom/create.rs b/components/script/dom/create.rs index 5f03a3adb98..fa05f4e00f1 100644 --- a/components/script/dom/create.rs +++ b/components/script/dom/create.rs @@ -5,6 +5,7 @@ use dom::bindings::error::{report_pending_exception, throw_dom_exception}; use dom::bindings::js::Root; use dom::bindings::reflector::DomObject; +use dom::customelementregistry::is_valid_custom_element_name; use dom::document::Document; use dom::element::{CustomElementCreationMode, Element, ElementCreator}; use dom::globalscope::GlobalScope; @@ -184,6 +185,7 @@ pub fn create_native_html_element(name: QualName, // This is a big match, and the IDs for inline-interned atoms are not very structured. // Perhaps we should build a perfect hash from those IDs instead. + // https://html.spec.whatwg.org/multipage/#elements-in-the-dom match name.local { local_name!("a") => make!(HTMLAnchorElement), local_name!("abbr") => make!(HTMLElement), @@ -326,7 +328,8 @@ pub fn create_native_html_element(name: QualName, local_name!("video") => make!(HTMLVideoElement), local_name!("wbr") => make!(HTMLElement), local_name!("xmp") => make!(HTMLPreElement), - _ => make!(HTMLUnknownElement), + _ if is_valid_custom_element_name(&*name.local) => make!(HTMLElement), + _ => make!(HTMLUnknownElement), } } diff --git a/components/script/dom/customelementregistry.rs b/components/script/dom/customelementregistry.rs index fdf197d7d97..e3fbe6ef2f5 100644 --- a/components/script/dom/customelementregistry.rs +++ b/components/script/dom/customelementregistry.rs @@ -750,7 +750,7 @@ impl ElementQueue { } /// https://html.spec.whatwg.org/multipage/#valid-custom-element-name -fn is_valid_custom_element_name(name: &str) -> bool { +pub fn is_valid_custom_element_name(name: &str) -> bool { // Custom elment names must match: // PotentialCustomElementName ::= [a-z] (PCENChar)* '-' (PCENChar)* diff --git a/tests/wpt/metadata/html/semantics/interfaces.html.ini b/tests/wpt/metadata/html/semantics/interfaces.html.ini index a2fb2c08ab4..2ebff96f036 100644 --- a/tests/wpt/metadata/html/semantics/interfaces.html.ini +++ b/tests/wpt/metadata/html/semantics/interfaces.html.ini @@ -42,12 +42,6 @@ [Interfaces for BASEFONT] expected: FAIL - [Interfaces for foo-bar] - expected: FAIL - - [Interfaces for FOO-BAR] - expected: FAIL - [Interfaces for menuitem] expected: FAIL From 1a9f4cad0871bcde9420f633c527b933ac288057 Mon Sep 17 00:00:00 2001 From: Connor Brewster Date: Thu, 20 Jul 2017 16:35:46 -0600 Subject: [PATCH 05/10] Fix compartment mismatch issue --- components/script/dom/bindings/codegen/CodegenRust.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index 482b713e10c..7f97868a378 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -5377,7 +5377,12 @@ let result = match result { }, }; -JS_SetPrototype(cx, result.reflector().get_jsobject(), prototype.handle()); +rooted!(in(cx) let mut element = result.reflector().get_jsobject().get()); +if !JS_WrapObject(cx, element.handle_mut()) { + return false; +} + +JS_SetPrototype(cx, element.handle(), prototype.handle()); (result).to_jsval(cx, args.rval()); return true; From 41371208a51661e82d65617e4f6ac221463a9ee7 Mon Sep 17 00:00:00 2001 From: Connor Brewster Date: Fri, 21 Jul 2017 14:08:03 -0600 Subject: [PATCH 06/10] Check namespace during ce def lookup --- components/script/dom/create.rs | 2 +- components/script/dom/customelementregistry.rs | 13 ++++++++----- components/script/dom/document.rs | 12 +++++++++--- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/components/script/dom/create.rs b/components/script/dom/create.rs index fa05f4e00f1..f1895e8844e 100644 --- a/components/script/dom/create.rs +++ b/components/script/dom/create.rs @@ -122,7 +122,7 @@ fn create_html_element(name: QualName, assert!(name.ns == ns!(html)); // Step 4 - let definition = document.lookup_custom_element_definition(name.local.clone(), is); + let definition = document.lookup_custom_element_definition(&name.ns, &name.local, is.as_ref()); if let Some(definition) = definition { if definition.is_autonomous() { diff --git a/components/script/dom/customelementregistry.rs b/components/script/dom/customelementregistry.rs index e3fbe6ef2f5..45482d29c82 100644 --- a/components/script/dom/customelementregistry.rs +++ b/components/script/dom/customelementregistry.rs @@ -79,13 +79,13 @@ impl CustomElementRegistry { /// https://html.spec.whatwg.org/multipage/#look-up-a-custom-element-definition pub fn lookup_definition(&self, - local_name: LocalName, - is: Option) + local_name: &LocalName, + is: Option<&LocalName>) -> Option> { self.definitions.borrow().values().find(|definition| { // Step 4-5 - definition.local_name == local_name && - (definition.name == local_name || Some(&definition.name) == is.as_ref()) + definition.local_name == *local_name && + (definition.name == *local_name || Some(&definition.name) == is) }).cloned() } @@ -522,7 +522,10 @@ fn upgrade_element(definition: Rc, element: &Element) { pub fn try_upgrade_element(element: &Element) { // Step 1 let document = document_from_node(element); - if let Some(definition) = document.lookup_custom_element_definition(element.local_name().clone(), element.get_is()) { + let namespace = element.namespace(); + let local_name = element.local_name(); + let is = element.get_is(); + if let Some(definition) = document.lookup_custom_element_definition(namespace, local_name, is.as_ref()) { // Step 2 ScriptThread::enqueue_upgrade_reaction(element, definition); } diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index c63295a0674..4f594f96357 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -95,7 +95,7 @@ use dom_struct::dom_struct; use encoding::EncodingRef; use encoding::all::UTF_8; use euclid::{Point2D, Vector2D}; -use html5ever::{LocalName, QualName}; +use html5ever::{LocalName, Namespace, QualName}; use hyper::header::{Header, SetCookie}; use hyper_serde::Serde; use ipc_channel::ipc::{self, IpcSender}; @@ -2001,13 +2001,19 @@ impl Document { /// https://html.spec.whatwg.org/multipage/#look-up-a-custom-element-definition pub fn lookup_custom_element_definition(&self, - local_name: LocalName, - is: Option) + namespace: &Namespace, + local_name: &LocalName, + is: Option<&LocalName>) -> Option> { if !PREFS.get("dom.customelements.enabled").as_boolean().unwrap_or(false) { return None; } + // Step 1 + if *namespace != ns!(html) { + return None; + } + // Step 2 if !self.has_browsing_context { return None; From 6d9d4add617e4e5ad08a65d563e9842b3cde0c7b Mon Sep 17 00:00:00 2001 From: Connor Brewster Date: Wed, 19 Jul 2017 13:34:33 -0600 Subject: [PATCH 07/10] Enqueue upgrades --- components/script/dom/create.rs | 20 ++++++-- .../script/dom/customelementregistry.rs | 30 +++++++---- components/script/dom/node.rs | 9 ++-- ...r-uses-registry-of-owner-document.html.ini | 3 -- .../custom-elements/reaction-timing.html.ini | 5 -- .../reactions/Document.html.ini | 3 -- .../custom-elements/reactions/Node.html.ini | 3 -- .../custom-elements/reactions/Range.html.ini | 3 -- .../custom-elements/upgrading.html.ini | 50 ------------------- .../upgrading/Node-cloneNode.html.ini | 12 ----- .../upgrading-enqueue-reactions.html.ini | 3 -- .../upgrading-parser-created-element.html.ini | 3 -- 12 files changed, 42 insertions(+), 102 deletions(-) delete mode 100644 tests/wpt/metadata/custom-elements/reaction-timing.html.ini delete mode 100644 tests/wpt/metadata/custom-elements/upgrading.html.ini diff --git a/components/script/dom/create.rs b/components/script/dom/create.rs index f1895e8844e..e70c547e3d3 100644 --- a/components/script/dom/create.rs +++ b/components/script/dom/create.rs @@ -5,7 +5,7 @@ use dom::bindings::error::{report_pending_exception, throw_dom_exception}; use dom::bindings::js::Root; use dom::bindings::reflector::DomObject; -use dom::customelementregistry::is_valid_custom_element_name; +use dom::customelementregistry::{is_valid_custom_element_name, upgrade_element}; use dom::document::Document; use dom::element::{CustomElementCreationMode, Element, ElementCreator}; use dom::globalscope::GlobalScope; @@ -81,6 +81,7 @@ use dom::htmlvideoelement::HTMLVideoElement; use dom::svgsvgelement::SVGSVGElement; use html5ever::{LocalName, Prefix, QualName}; use js::jsapi::JSAutoCompartment; +use script_thread::ScriptThread; use servo_config::prefs::PREFS; fn create_svg_element(name: QualName, @@ -127,8 +128,11 @@ fn create_html_element(name: QualName, if let Some(definition) = definition { if definition.is_autonomous() { match mode { - // TODO: Handle asynchronous CE creation. Relies on CE upgrades. - CustomElementCreationMode::Asynchronous => {}, + CustomElementCreationMode::Asynchronous => { + let result = Root::upcast(HTMLElement::new(name.local.clone(), prefix.clone(), document)); + ScriptThread::enqueue_upgrade_reaction(&*result, definition); + return result; + }, CustomElementCreationMode::Synchronous => { let local_name = name.local.clone(); return match definition.create_element(document, prefix.clone()) { @@ -155,9 +159,17 @@ fn create_html_element(name: QualName, }, } } else { + // Steps 5.1-5.2 let element = create_native_html_element(name, prefix, document, creator); element.set_is(definition.name.clone()); - // TODO: Enqueue custom element upgrade + match mode { + // Step 5.3 + CustomElementCreationMode::Synchronous => + upgrade_element(definition, &*element), + // Step 5.4 + CustomElementCreationMode::Asynchronous => + ScriptThread::enqueue_upgrade_reaction(&*element, definition), + } return element; } } diff --git a/components/script/dom/customelementregistry.rs b/components/script/dom/customelementregistry.rs index 45482d29c82..3f41f5da1cc 100644 --- a/components/script/dom/customelementregistry.rs +++ b/components/script/dom/customelementregistry.rs @@ -9,6 +9,7 @@ use dom::bindings::codegen::Bindings::CustomElementRegistryBinding::CustomElemen use dom::bindings::codegen::Bindings::CustomElementRegistryBinding::ElementDefinitionOptions; use dom::bindings::codegen::Bindings::ElementBinding::ElementMethods; use dom::bindings::codegen::Bindings::FunctionBinding::Function; +use dom::bindings::codegen::Bindings::WindowBinding::WindowBinding::WindowMethods; use dom::bindings::conversions::{ConversionResult, FromJSValConvertible, StringificationBehavior}; use dom::bindings::error::{Error, ErrorResult, Fallible}; use dom::bindings::inheritance::Castable; @@ -281,17 +282,28 @@ impl CustomElementRegistryMethods for CustomElementRegistry { self.element_definition_is_running.set(false); // Step 11 - let definition = CustomElementDefinition::new(name.clone(), - local_name, - constructor_, - observed_attributes, - callbacks); + let definition = Rc::new(CustomElementDefinition::new(name.clone(), + local_name.clone(), + constructor_, + observed_attributes, + callbacks)); // Step 12 - self.definitions.borrow_mut().insert(name.clone(), Rc::new(definition)); + self.definitions.borrow_mut().insert(name.clone(), definition.clone()); - // TODO: Step 13, 14, 15 - // Handle custom element upgrades + // Step 13 + let document = self.window.Document(); + + // Steps 14-15 + for candidate in document.upcast::().traverse_preorder().filter_map(Root::downcast::) { + let is = candidate.get_is(); + if *candidate.local_name() == local_name && + *candidate.namespace() == ns!(html) && + (extends.is_none() || is.as_ref() == Some(&name)) + { + ScriptThread::enqueue_upgrade_reaction(&*candidate, definition.clone()); + } + } // Step 16, 16.3 if let Some(promise) = self.when_defined.borrow_mut().remove(&name) { @@ -464,7 +476,7 @@ impl CustomElementDefinition { /// https://html.spec.whatwg.org/multipage/#concept-upgrade-an-element #[allow(unsafe_code)] -fn upgrade_element(definition: Rc, element: &Element) { +pub fn upgrade_element(definition: Rc, element: &Element) { // TODO: Steps 1-2 // Track custom element state diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index d6420b0a6f1..3a978008e99 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -30,7 +30,7 @@ use dom::bindings::str::{DOMString, USVString}; use dom::bindings::xmlname::namespace_from_domstring; use dom::characterdata::{CharacterData, LayoutCharacterDataHelpers}; use dom::cssstylesheet::CSSStyleSheet; -use dom::customelementregistry::CallbackReaction; +use dom::customelementregistry::{CallbackReaction, try_upgrade_element}; use dom::document::{Document, DocumentSource, HasBrowsingContext, IsHTMLDocument}; use dom::documentfragment::DocumentFragment; use dom::documenttype::DocumentType; @@ -1639,12 +1639,13 @@ impl Node { for descendant in kid.traverse_preorder().filter_map(Root::downcast::) { // Step 7.7.2. if descendant.is_connected() { - // Step 7.7.2.1. if descendant.get_custom_element_definition().is_some() { + // Step 7.7.2.1. ScriptThread::enqueue_callback_reaction(&*descendant, CallbackReaction::Connected); + } else { + // Step 7.7.2.2. + try_upgrade_element(&*descendant); } - // TODO: Step 7.7.2.2. - // Try to upgrade descendant. } } } diff --git a/tests/wpt/metadata/custom-elements/parser/parser-uses-registry-of-owner-document.html.ini b/tests/wpt/metadata/custom-elements/parser/parser-uses-registry-of-owner-document.html.ini index 4106ee6ebb9..aea72a43f87 100644 --- a/tests/wpt/metadata/custom-elements/parser/parser-uses-registry-of-owner-document.html.ini +++ b/tests/wpt/metadata/custom-elements/parser/parser-uses-registry-of-owner-document.html.ini @@ -3,9 +3,6 @@ [HTML parser must not instantiate custom elements inside template elements] expected: FAIL - [HTML parser must use the registry of the content document inside an iframe] - expected: FAIL - [HTML parser must use the registry of window.document in a document created by document.implementation.createHTMLDocument()] expected: FAIL diff --git a/tests/wpt/metadata/custom-elements/reaction-timing.html.ini b/tests/wpt/metadata/custom-elements/reaction-timing.html.ini deleted file mode 100644 index 0697f701403..00000000000 --- a/tests/wpt/metadata/custom-elements/reaction-timing.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[reaction-timing.html] - type: testharness - [Calling Node.prototype.cloneNode(false) must push a new element queue to the processing stack] - expected: FAIL - diff --git a/tests/wpt/metadata/custom-elements/reactions/Document.html.ini b/tests/wpt/metadata/custom-elements/reactions/Document.html.ini index c9319bdc330..0076f82da66 100644 --- a/tests/wpt/metadata/custom-elements/reactions/Document.html.ini +++ b/tests/wpt/metadata/custom-elements/reactions/Document.html.ini @@ -1,8 +1,5 @@ [Document.html] type: testharness - [importNode on Document must construct a new custom element when importing a custom element from a template] - expected: FAIL - [execCommand on Document must enqueue a disconnected reaction when deleting a custom element from a contenteditable element] expected: FAIL diff --git a/tests/wpt/metadata/custom-elements/reactions/Node.html.ini b/tests/wpt/metadata/custom-elements/reactions/Node.html.ini index fc8e80a9dcb..e3396bed69f 100644 --- a/tests/wpt/metadata/custom-elements/reactions/Node.html.ini +++ b/tests/wpt/metadata/custom-elements/reactions/Node.html.ini @@ -3,9 +3,6 @@ [cloneNode on Node must enqueue an attributeChanged reaction when cloning an element with an observed attribute] expected: FAIL - [cloneNode on Node must not enqueue an attributeChanged reaction when cloning an element with an unobserved attribute] - expected: FAIL - [cloneNode on Node must enqueue an attributeChanged reaction when cloning an element only for observed attributes] expected: FAIL diff --git a/tests/wpt/metadata/custom-elements/reactions/Range.html.ini b/tests/wpt/metadata/custom-elements/reactions/Range.html.ini index 678971e6eaf..37eaa10d28b 100644 --- a/tests/wpt/metadata/custom-elements/reactions/Range.html.ini +++ b/tests/wpt/metadata/custom-elements/reactions/Range.html.ini @@ -3,9 +3,6 @@ [cloneContents on Range must enqueue an attributeChanged reaction when cloning an element with an observed attribute] expected: FAIL - [cloneContents on Range must not enqueue an attributeChanged reaction when cloning an element with an unobserved attribute] - expected: FAIL - [cloneContents on Range must enqueue an attributeChanged reaction when cloning an element only for observed attributes] expected: FAIL diff --git a/tests/wpt/metadata/custom-elements/upgrading.html.ini b/tests/wpt/metadata/custom-elements/upgrading.html.ini deleted file mode 100644 index a9bcfe9fca6..00000000000 --- a/tests/wpt/metadata/custom-elements/upgrading.html.ini +++ /dev/null @@ -1,50 +0,0 @@ -[upgrading.html] - type: testharness - [Creating an element in the document of the template elements and inserting into the document must not enqueue a custom element upgrade reaction] - expected: FAIL - - [Creating an element in the document of the template elements and adopting back to a document with browsing context must enqueue a custom element upgrade reaction] - expected: FAIL - - [Creating an element in a new document and inserting into the document must not enqueue a custom element upgrade reaction] - expected: FAIL - - [Creating an element in a new document and adopting back to a document with browsing context must enqueue a custom element upgrade reaction] - expected: FAIL - - [Creating an element in a cloned document and inserting into the document must not enqueue a custom element upgrade reaction] - expected: FAIL - - [Creating an element in a cloned document and adopting back to a document with browsing context must enqueue a custom element upgrade reaction] - expected: FAIL - - [Creating an element in a document created by createHTMLDocument and inserting into the document must not enqueue a custom element upgrade reaction] - expected: FAIL - - [Creating an element in a document created by createHTMLDocument and adopting back to a document with browsing context must enqueue a custom element upgrade reaction] - expected: FAIL - - [Creating an element in an HTML document created by createDocument and inserting into the document must not enqueue a custom element upgrade reaction] - expected: FAIL - - [Creating an element in an HTML document created by createDocument and adopting back to a document with browsing context must enqueue a custom element upgrade reaction] - expected: FAIL - - [Creating an element in an HTML document fetched by XHR and inserting into the document must not enqueue a custom element upgrade reaction] - expected: FAIL - - [Creating an element in an HTML document fetched by XHR and adopting back to a document with browsing context must enqueue a custom element upgrade reaction] - expected: FAIL - - ["define" in the document of an iframe must not enqueue a custom element upgrade reaction on a disconnected unresolved custom element] - expected: FAIL - - [Inserting an unresolved custom element into the document of an iframe must enqueue a custom element upgrade reaction] - expected: FAIL - - ["define" in the document of an iframe must enqueue a custom element upgrade reaction on a connected unresolved custom element] - expected: FAIL - - [Adopting and inserting an unresolved custom element into the document of an iframe must enqueue a custom element upgrade reaction] - expected: FAIL - diff --git a/tests/wpt/metadata/custom-elements/upgrading/Node-cloneNode.html.ini b/tests/wpt/metadata/custom-elements/upgrading/Node-cloneNode.html.ini index 3214c6fd3b0..2e78908601d 100644 --- a/tests/wpt/metadata/custom-elements/upgrading/Node-cloneNode.html.ini +++ b/tests/wpt/metadata/custom-elements/upgrading/Node-cloneNode.html.ini @@ -1,17 +1,5 @@ [Node-cloneNode.html] type: testharness - [Node.prototype.cloneNode(false) must be able to clone a custom element] - expected: FAIL - - [Node.prototype.cloneNode(false) must be able to clone a custom element inside an iframe] - expected: FAIL - - [Node.prototype.cloneNode(true) must be able to clone a descendent custom element] - expected: FAIL - - [Node.prototype.cloneNode(true) must set parentNode, previousSibling, and nextSibling before upgrading custom elements] - expected: FAIL - [HTMLElement constructor must throw an InvalidStateError when the top of the construction stack is marked AlreadyConstructed due to a custom element constructor constructing itself after super() call] expected: FAIL diff --git a/tests/wpt/metadata/custom-elements/upgrading/upgrading-enqueue-reactions.html.ini b/tests/wpt/metadata/custom-elements/upgrading/upgrading-enqueue-reactions.html.ini index 6b6638f0b86..c45292cb290 100644 --- a/tests/wpt/metadata/custom-elements/upgrading/upgrading-enqueue-reactions.html.ini +++ b/tests/wpt/metadata/custom-elements/upgrading/upgrading-enqueue-reactions.html.ini @@ -12,6 +12,3 @@ [Upgrading a custom element must enqueue attributeChangedCallback before connectedCallback] expected: FAIL - [Upgrading a custom element must not invoke attributeChangedCallback and connectedCallback when the element failed to upgrade] - expected: FAIL - diff --git a/tests/wpt/metadata/custom-elements/upgrading/upgrading-parser-created-element.html.ini b/tests/wpt/metadata/custom-elements/upgrading/upgrading-parser-created-element.html.ini index d252ad96aa1..bf6273a3516 100644 --- a/tests/wpt/metadata/custom-elements/upgrading/upgrading-parser-created-element.html.ini +++ b/tests/wpt/metadata/custom-elements/upgrading/upgrading-parser-created-element.html.ini @@ -1,8 +1,5 @@ [upgrading-parser-created-element.html] type: testharness - [Element.prototype.createElement must add an unresolved custom element to the upgrade candidates map] - expected: FAIL - [HTMLElement constructor must throw an InvalidStateError when the top of the construction stack is marked AlreadyConstructed due to a custom element constructor constructing itself after super() call] expected: FAIL From e700006fb218f04cdfdfab0d6c4e07d6901ab94d Mon Sep 17 00:00:00 2001 From: Connor Brewster Date: Fri, 21 Jul 2017 14:38:58 -0600 Subject: [PATCH 08/10] Handle exceptions during upgrades --- components/script/dom/attr.rs | 2 +- .../script/dom/customelementregistry.rs | 67 +++++++++++++------ components/script/dom/element.rs | 22 ++++-- components/script/dom/node.rs | 6 +- components/script/script_thread.rs | 6 +- .../custom-element-reaction-queue.html.ini | 8 --- .../reactions/Document.html.ini | 12 ---- .../reactions/Element.html.ini | 12 ---- .../reactions/HTMLAnchorElement.html.ini | 5 -- .../reactions/HTMLOptionElement.html.ini | 5 -- .../reactions/HTMLTableElement.html.ini | 21 ------ .../reactions/HTMLTableRowElement.html.ini | 5 -- .../HTMLTableSectionElement.html.ini | 8 --- .../custom-elements/reactions/Node.html.ini | 8 --- .../custom-elements/reactions/Range.html.ini | 6 -- .../upgrading/Node-cloneNode.html.ini | 9 --- .../upgrading-enqueue-reactions.html.ini | 14 ---- .../upgrading-parser-created-element.html.ini | 11 --- 18 files changed, 71 insertions(+), 156 deletions(-) delete mode 100644 tests/wpt/metadata/custom-elements/custom-element-reaction-queue.html.ini delete mode 100644 tests/wpt/metadata/custom-elements/reactions/HTMLAnchorElement.html.ini delete mode 100644 tests/wpt/metadata/custom-elements/reactions/HTMLOptionElement.html.ini delete mode 100644 tests/wpt/metadata/custom-elements/reactions/HTMLTableRowElement.html.ini delete mode 100644 tests/wpt/metadata/custom-elements/reactions/HTMLTableSectionElement.html.ini delete mode 100644 tests/wpt/metadata/custom-elements/reactions/Node.html.ini delete mode 100644 tests/wpt/metadata/custom-elements/upgrading/upgrading-enqueue-reactions.html.ini delete mode 100644 tests/wpt/metadata/custom-elements/upgrading/upgrading-parser-created-element.html.ini diff --git a/components/script/dom/attr.rs b/components/script/dom/attr.rs index bf2ecd6e910..3ce627b4677 100644 --- a/components/script/dom/attr.rs +++ b/components/script/dom/attr.rs @@ -188,7 +188,7 @@ impl Attr { if owner.get_custom_element_definition().is_some() { let reaction = CallbackReaction::AttributeChanged(name, Some(old_value), Some(new_value), namespace); - ScriptThread::enqueue_callback_reaction(owner, reaction); + ScriptThread::enqueue_callback_reaction(owner, reaction, None); } assert!(Some(owner) == self.owner().r()); diff --git a/components/script/dom/customelementregistry.rs b/components/script/dom/customelementregistry.rs index 3f41f5da1cc..100aae7cae3 100644 --- a/components/script/dom/customelementregistry.rs +++ b/components/script/dom/customelementregistry.rs @@ -11,7 +11,7 @@ use dom::bindings::codegen::Bindings::ElementBinding::ElementMethods; use dom::bindings::codegen::Bindings::FunctionBinding::Function; use dom::bindings::codegen::Bindings::WindowBinding::WindowBinding::WindowMethods; use dom::bindings::conversions::{ConversionResult, FromJSValConvertible, StringificationBehavior}; -use dom::bindings::error::{Error, ErrorResult, Fallible}; +use dom::bindings::error::{Error, ErrorResult, Fallible, report_pending_exception, throw_dom_exception}; use dom::bindings::inheritance::Castable; use dom::bindings::js::{JS, Root}; use dom::bindings::reflector::{DomObject, Reflector, reflect_dom_object}; @@ -486,48 +486,72 @@ pub fn upgrade_element(definition: Rc, element: &Elemen let value = DOMString::from(&**attr.value()); let namespace = attr.namespace().clone(); ScriptThread::enqueue_callback_reaction(element, - CallbackReaction::AttributeChanged(local_name, None, Some(value), namespace)); + CallbackReaction::AttributeChanged(local_name, None, Some(value), namespace), Some(definition.clone())); } // Step 4 if element.is_connected() { - ScriptThread::enqueue_callback_reaction(element, CallbackReaction::Connected); + ScriptThread::enqueue_callback_reaction(element, CallbackReaction::Connected, Some(definition.clone())); } // Step 5 definition.construction_stack.borrow_mut().push(ConstructionStackEntry::Element(Root::from_ref(element))); - // Step 6 + // Step 7 + let result = run_upgrade_constructor(&definition.constructor, element); + + definition.construction_stack.borrow_mut().pop(); + + // Step 7 exception handling + if let Err(error) = result { + // TODO: Step 7.1 + // Track custom element state + + // Step 7.2 + element.clear_reaction_queue(); + + // Step 7.3 + let global = GlobalScope::current().expect("No current global"); + let cx = global.get_cx(); + unsafe { + throw_dom_exception(cx, &global, error); + report_pending_exception(cx, true); + } + return; + } + + element.set_custom_element_definition(definition); +} + +/// https://html.spec.whatwg.org/multipage/#concept-upgrade-an-element +/// Steps 7.1-7.2 +#[allow(unsafe_code)] +fn run_upgrade_constructor(constructor: &Rc, element: &Element) -> ErrorResult { let window = window_from_node(element); let cx = window.get_cx(); - rooted!(in(cx) let constructor = ObjectValue(definition.constructor.callback())); + rooted!(in(cx) let constructor_val = ObjectValue(constructor.callback())); rooted!(in(cx) let mut element_val = UndefinedValue()); unsafe { element.to_jsval(cx, element_val.handle_mut()); } rooted!(in(cx) let mut construct_result = ptr::null_mut()); { // Go into the constructor's compartment - let _ac = JSAutoCompartment::new(cx, definition.constructor.callback()); + let _ac = JSAutoCompartment::new(cx, constructor.callback()); let args = HandleValueArray::new(); - // Step 7 - if unsafe { !Construct1(cx, constructor.handle(), &args, construct_result.handle_mut()) } { - // TODO: Catch exceptions - return; + // Step 7.1 + if unsafe { !Construct1(cx, constructor_val.handle(), &args, construct_result.handle_mut()) } { + return Err(Error::JSFailed); } - // Step 8 + // Step 7.2 let mut same = false; rooted!(in(cx) let construct_result_val = ObjectValue(construct_result.get())); if unsafe { !JS_SameValue(cx, construct_result_val.handle(), element_val.handle(), &mut same) } { - // TODO: Catch exceptions - return; + return Err(Error::JSFailed); } if !same { - // TODO: Throw InvalidStateError + return Err(Error::InvalidState); } } - definition.construction_stack.borrow_mut().pop(); - // TODO: Handle Exceptions - - element.set_custom_element_definition(definition); + Ok(()) } /// https://html.spec.whatwg.org/multipage/#concept-try-upgrade @@ -655,9 +679,12 @@ impl CustomElementReactionStack { /// https://html.spec.whatwg.org/multipage/#enqueue-a-custom-element-callback-reaction #[allow(unsafe_code)] - pub fn enqueue_callback_reaction(&self, element: &Element, reaction: CallbackReaction) { + pub fn enqueue_callback_reaction(&self, + element: &Element, + reaction: CallbackReaction, + definition: Option>) { // Step 1 - let definition = match element.get_custom_element_definition() { + let definition = match definition.or_else(|| element.get_custom_element_definition()) { Some(definition) => definition, None => return, }; diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index b44757d3aa1..270d7d7e020 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -101,6 +101,7 @@ use std::cell::{Cell, Ref}; use std::convert::TryFrom; use std::default::Default; use std::fmt; +use std::mem; use std::rc::Rc; use style::CaseSensitivityExt; use style::applicable_declarations::ApplicableDeclarationBlock; @@ -304,12 +305,21 @@ impl Element { self.custom_element_reaction_queue.borrow_mut().push(CustomElementReaction::Upgrade(definition)); } + pub fn clear_reaction_queue(&self) { + self.custom_element_reaction_queue.borrow_mut().clear(); + } + pub fn invoke_reactions(&self) { - let mut reaction_queue = self.custom_element_reaction_queue.borrow_mut(); - for reaction in reaction_queue.iter() { - reaction.invoke(self); + // TODO: This is not spec compliant, as this will allow some reactions to be processed + // after clear_reaction_queue has been called. + rooted_vec!(let mut reactions); + while !self.custom_element_reaction_queue.borrow().is_empty() { + mem::swap(&mut *reactions, &mut *self.custom_element_reaction_queue.borrow_mut()); + for reaction in reactions.iter() { + reaction.invoke(self); + } + reactions.clear(); } - reaction_queue.clear(); } // https://drafts.csswg.org/cssom-view/#css-layout-box @@ -1081,7 +1091,7 @@ impl Element { if self.get_custom_element_definition().is_some() { let reaction = CallbackReaction::AttributeChanged(name, None, Some(value), namespace); - ScriptThread::enqueue_callback_reaction(self, reaction); + ScriptThread::enqueue_callback_reaction(self, reaction, None); } assert!(attr.GetOwnerElement().r() == Some(self)); @@ -1224,7 +1234,7 @@ impl Element { MutationObserver::queue_a_mutation_record(&self.node, mutation); let reaction = CallbackReaction::AttributeChanged(name, Some(old_value), None, namespace); - ScriptThread::enqueue_callback_reaction(self, reaction); + ScriptThread::enqueue_callback_reaction(self, reaction, None); self.attrs.borrow_mut().remove(idx); attr.set_owner(None); diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index 3a978008e99..7503618884b 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -317,7 +317,7 @@ impl Node { node.style_and_layout_data.get().map(|d| node.dispose(d)); // https://dom.spec.whatwg.org/#concept-node-remove step 14 if let Some(element) = node.as_custom_element() { - ScriptThread::enqueue_callback_reaction(&*element, CallbackReaction::Disconnected); + ScriptThread::enqueue_callback_reaction(&*element, CallbackReaction::Disconnected, None); } } @@ -1425,7 +1425,7 @@ impl Node { for descendant in node.traverse_preorder().filter_map(|d| d.as_custom_element()) { // Step 3.2. ScriptThread::enqueue_callback_reaction(&*descendant, - CallbackReaction::Adopted(old_doc.clone(), Root::from_ref(document))); + CallbackReaction::Adopted(old_doc.clone(), Root::from_ref(document)), None); } for descendant in node.traverse_preorder() { // Step 3.3. @@ -1641,7 +1641,7 @@ impl Node { if descendant.is_connected() { if descendant.get_custom_element_definition().is_some() { // Step 7.7.2.1. - ScriptThread::enqueue_callback_reaction(&*descendant, CallbackReaction::Connected); + ScriptThread::enqueue_callback_reaction(&*descendant, CallbackReaction::Connected, None); } else { // Step 7.7.2.2. try_upgrade_element(&*descendant); diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 49c09787ecc..4a4657e28a5 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -774,11 +774,13 @@ impl ScriptThread { }) } - pub fn enqueue_callback_reaction(element: &Element, reaction: CallbackReaction) { + pub fn enqueue_callback_reaction(element: &Element, + reaction: CallbackReaction, + definition: Option>) { SCRIPT_THREAD_ROOT.with(|root| { if let Some(script_thread) = root.get() { let script_thread = unsafe { &*script_thread }; - script_thread.custom_element_reaction_stack.enqueue_callback_reaction(element, reaction); + script_thread.custom_element_reaction_stack.enqueue_callback_reaction(element, reaction, definition); } }) } diff --git a/tests/wpt/metadata/custom-elements/custom-element-reaction-queue.html.ini b/tests/wpt/metadata/custom-elements/custom-element-reaction-queue.html.ini deleted file mode 100644 index b36672713c3..00000000000 --- a/tests/wpt/metadata/custom-elements/custom-element-reaction-queue.html.ini +++ /dev/null @@ -1,8 +0,0 @@ -[custom-element-reaction-queue.html] - type: testharness - [Upgrading a custom element must invoke attributeChangedCallback and connectedCallback before start upgrading another element] - expected: FAIL - - [Mutating a undefined custom element while upgrading a custom element must not enqueue or invoke reactions on the mutated element] - expected: FAIL - diff --git a/tests/wpt/metadata/custom-elements/reactions/Document.html.ini b/tests/wpt/metadata/custom-elements/reactions/Document.html.ini index 0076f82da66..cf4ac38d747 100644 --- a/tests/wpt/metadata/custom-elements/reactions/Document.html.ini +++ b/tests/wpt/metadata/custom-elements/reactions/Document.html.ini @@ -3,15 +3,3 @@ [execCommand on Document must enqueue a disconnected reaction when deleting a custom element from a contenteditable element] expected: FAIL - [body on Document must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - - [open on Document must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - - [write on Document must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - - [writeln on Document must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - diff --git a/tests/wpt/metadata/custom-elements/reactions/Element.html.ini b/tests/wpt/metadata/custom-elements/reactions/Element.html.ini index 988eddc141a..8661d431874 100644 --- a/tests/wpt/metadata/custom-elements/reactions/Element.html.ini +++ b/tests/wpt/metadata/custom-elements/reactions/Element.html.ini @@ -12,18 +12,6 @@ [setAttributeNodeNS on Element must enqueue an attributeChanged reaction when replacing an existing attribute] expected: FAIL - [innerHTML on Element must enqueue a connected reaction for a newly constructed custom element] - expected: FAIL - - [innerHTML on Element must enqueue a attributeChanged reaction for a newly constructed custom element] - expected: FAIL - - [outerHTML on Element must enqueue a connected reaction for a newly constructed custom element] - expected: FAIL - - [outerHTML on Element must enqueue a attributeChanged reaction for a newly constructed custom element] - expected: FAIL - [insertAdjacentHTML on Element must enqueue a connected reaction for a newly constructed custom element] expected: FAIL diff --git a/tests/wpt/metadata/custom-elements/reactions/HTMLAnchorElement.html.ini b/tests/wpt/metadata/custom-elements/reactions/HTMLAnchorElement.html.ini deleted file mode 100644 index 4f045309030..00000000000 --- a/tests/wpt/metadata/custom-elements/reactions/HTMLAnchorElement.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[HTMLAnchorElement.html] - type: testharness - [text on HTMLAnchorElement must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - diff --git a/tests/wpt/metadata/custom-elements/reactions/HTMLOptionElement.html.ini b/tests/wpt/metadata/custom-elements/reactions/HTMLOptionElement.html.ini deleted file mode 100644 index fe5243ef3f3..00000000000 --- a/tests/wpt/metadata/custom-elements/reactions/HTMLOptionElement.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[HTMLOptionElement.html] - type: testharness - [text on HTMLOptionElement must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - diff --git a/tests/wpt/metadata/custom-elements/reactions/HTMLTableElement.html.ini b/tests/wpt/metadata/custom-elements/reactions/HTMLTableElement.html.ini index 2349ffff761..6a698e89781 100644 --- a/tests/wpt/metadata/custom-elements/reactions/HTMLTableElement.html.ini +++ b/tests/wpt/metadata/custom-elements/reactions/HTMLTableElement.html.ini @@ -3,30 +3,9 @@ [caption on HTMLTableElement must enqueue connectedCallback when inserting a custom element] expected: FAIL - [caption on HTMLTableElement must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - - [deleteCaption() on HTMLTableElement must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - [tHead on HTMLTableElement must enqueue connectedCallback when inserting a custom element] expected: FAIL - [tHead on HTMLTableElement must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - - [deleteTHead() on HTMLTableElement must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - [tFoot on HTMLTableElement must enqueue connectedCallback when inserting a custom element] expected: FAIL - [tFoot on HTMLTableElement must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - - [deleteTFoot() on HTMLTableElement must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - - [deleteRow() on HTMLTableElement must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - diff --git a/tests/wpt/metadata/custom-elements/reactions/HTMLTableRowElement.html.ini b/tests/wpt/metadata/custom-elements/reactions/HTMLTableRowElement.html.ini deleted file mode 100644 index 697e2e8a17d..00000000000 --- a/tests/wpt/metadata/custom-elements/reactions/HTMLTableRowElement.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[HTMLTableRowElement.html] - type: testharness - [deleteCell() on HTMLTableRowElement must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - diff --git a/tests/wpt/metadata/custom-elements/reactions/HTMLTableSectionElement.html.ini b/tests/wpt/metadata/custom-elements/reactions/HTMLTableSectionElement.html.ini deleted file mode 100644 index 38ece7c0697..00000000000 --- a/tests/wpt/metadata/custom-elements/reactions/HTMLTableSectionElement.html.ini +++ /dev/null @@ -1,8 +0,0 @@ -[HTMLTableSectionElement.html] - type: testharness - [deleteRow() on HTMLTableSectionElement on thead must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - - [deleteRow() on HTMLTableSectionElement on tfoot must enqueue disconnectedCallback when removing a custom element] - expected: FAIL - diff --git a/tests/wpt/metadata/custom-elements/reactions/Node.html.ini b/tests/wpt/metadata/custom-elements/reactions/Node.html.ini deleted file mode 100644 index e3396bed69f..00000000000 --- a/tests/wpt/metadata/custom-elements/reactions/Node.html.ini +++ /dev/null @@ -1,8 +0,0 @@ -[Node.html] - type: testharness - [cloneNode on Node must enqueue an attributeChanged reaction when cloning an element with an observed attribute] - expected: FAIL - - [cloneNode on Node must enqueue an attributeChanged reaction when cloning an element only for observed attributes] - expected: FAIL - diff --git a/tests/wpt/metadata/custom-elements/reactions/Range.html.ini b/tests/wpt/metadata/custom-elements/reactions/Range.html.ini index 37eaa10d28b..a5696b0e0fe 100644 --- a/tests/wpt/metadata/custom-elements/reactions/Range.html.ini +++ b/tests/wpt/metadata/custom-elements/reactions/Range.html.ini @@ -1,11 +1,5 @@ [Range.html] type: testharness - [cloneContents on Range must enqueue an attributeChanged reaction when cloning an element with an observed attribute] - expected: FAIL - - [cloneContents on Range must enqueue an attributeChanged reaction when cloning an element only for observed attributes] - expected: FAIL - [createContextualFragment on Range must construct a custom element] expected: FAIL diff --git a/tests/wpt/metadata/custom-elements/upgrading/Node-cloneNode.html.ini b/tests/wpt/metadata/custom-elements/upgrading/Node-cloneNode.html.ini index 2e78908601d..10ac904ac52 100644 --- a/tests/wpt/metadata/custom-elements/upgrading/Node-cloneNode.html.ini +++ b/tests/wpt/metadata/custom-elements/upgrading/Node-cloneNode.html.ini @@ -1,14 +1,5 @@ [Node-cloneNode.html] type: testharness - [HTMLElement constructor must throw an InvalidStateError when the top of the construction stack is marked AlreadyConstructed due to a custom element constructor constructing itself after super() call] - expected: FAIL - - [HTMLElement constructor must throw an InvalidStateError when the top of the construction stack is marked AlreadyConstructed due to a custom element constructor constructing itself before super() call] - expected: FAIL - - [Upgrading a custom element must throw InvalidStateError when the custom element's constructor returns another element] - expected: FAIL - [Inserting an element must not try to upgrade a custom element when it had already failed to upgrade once] expected: FAIL diff --git a/tests/wpt/metadata/custom-elements/upgrading/upgrading-enqueue-reactions.html.ini b/tests/wpt/metadata/custom-elements/upgrading/upgrading-enqueue-reactions.html.ini deleted file mode 100644 index c45292cb290..00000000000 --- a/tests/wpt/metadata/custom-elements/upgrading/upgrading-enqueue-reactions.html.ini +++ /dev/null @@ -1,14 +0,0 @@ -[upgrading-enqueue-reactions.html] - type: testharness - [Upgrading a custom element must enqueue attributeChangedCallback on each attribute] - expected: FAIL - - [Upgrading a custom element not must enqueue attributeChangedCallback on unobserved attributes] - expected: FAIL - - [Upgrading a custom element must enqueue connectedCallback if the element in the document] - expected: FAIL - - [Upgrading a custom element must enqueue attributeChangedCallback before connectedCallback] - expected: FAIL - diff --git a/tests/wpt/metadata/custom-elements/upgrading/upgrading-parser-created-element.html.ini b/tests/wpt/metadata/custom-elements/upgrading/upgrading-parser-created-element.html.ini deleted file mode 100644 index bf6273a3516..00000000000 --- a/tests/wpt/metadata/custom-elements/upgrading/upgrading-parser-created-element.html.ini +++ /dev/null @@ -1,11 +0,0 @@ -[upgrading-parser-created-element.html] - type: testharness - [HTMLElement constructor must throw an InvalidStateError when the top of the construction stack is marked AlreadyConstructed due to a custom element constructor constructing itself after super() call] - expected: FAIL - - [HTMLElement constructor must throw an InvalidStateError when the top of the construction stack is marked AlreadyConstructed due to a custom element constructor constructing itself before super() call] - expected: FAIL - - [Upgrading a custom element must throw an InvalidStateError when the returned element is not SameValue as the upgraded element] - expected: FAIL - From 9f51c7df21b9a03c40235cbccd1b7930528f23a2 Mon Sep 17 00:00:00 2001 From: Connor Brewster Date: Tue, 1 Aug 2017 12:20:31 -0600 Subject: [PATCH 09/10] Track custom element state --- components/script/dom/bindings/interface.rs | 8 ++++--- components/script/dom/create.rs | 22 +++++++++++++++---- .../script/dom/customelementregistry.rs | 17 +++++++++----- components/script/dom/element.rs | 20 +++++++++++++++++ tests/unit/script/size_of.rs | 8 +++---- .../upgrading/Node-cloneNode.html.ini | 5 ----- 6 files changed, 59 insertions(+), 21 deletions(-) delete mode 100644 tests/wpt/metadata/custom-elements/upgrading/Node-cloneNode.html.ini diff --git a/components/script/dom/bindings/interface.rs b/components/script/dom/bindings/interface.rs index 0df46f40241..63658b3f0f9 100644 --- a/components/script/dom/bindings/interface.rs +++ b/components/script/dom/bindings/interface.rs @@ -81,7 +81,7 @@ use dom::bindings::js::Root; use dom::bindings::utils::{DOM_PROTOTYPE_SLOT, ProtoOrIfaceArray, get_proto_or_iface_array}; use dom::create::create_native_html_element; use dom::customelementregistry::ConstructionStackEntry; -use dom::element::{Element, ElementCreator}; +use dom::element::{CustomElementState, Element, ElementCreator}; use dom::htmlelement::HTMLElement; use dom::window::Window; use html5ever::LocalName; @@ -296,8 +296,10 @@ pub unsafe fn html_constructor(window: &Window, call_args: &CallArgs) -> Fall // Step 8.2 is performed in the generated caller code. - // TODO: Step 8.3 - 8.4 - // Set the element's custom element state and definition. + // Step 8.3 + element.set_custom_element_state(CustomElementState::Custom); + + // Step 8.4 element.set_custom_element_definition(definition.clone()); // Step 8.5 diff --git a/components/script/dom/create.rs b/components/script/dom/create.rs index e70c547e3d3..922757593f3 100644 --- a/components/script/dom/create.rs +++ b/components/script/dom/create.rs @@ -7,7 +7,7 @@ use dom::bindings::js::Root; use dom::bindings::reflector::DomObject; use dom::customelementregistry::{is_valid_custom_element_name, upgrade_element}; use dom::document::Document; -use dom::element::{CustomElementCreationMode, Element, ElementCreator}; +use dom::element::{CustomElementCreationMode, CustomElementState, Element, ElementCreator}; use dom::globalscope::GlobalScope; use dom::htmlanchorelement::HTMLAnchorElement; use dom::htmlappletelement::HTMLAppletElement; @@ -129,7 +129,9 @@ fn create_html_element(name: QualName, if definition.is_autonomous() { match mode { CustomElementCreationMode::Asynchronous => { - let result = Root::upcast(HTMLElement::new(name.local.clone(), prefix.clone(), document)); + let result = Root::upcast::( + HTMLElement::new(name.local.clone(), prefix.clone(), document)); + result.set_custom_element_state(CustomElementState::Undefined); ScriptThread::enqueue_upgrade_reaction(&*result, definition); return result; }, @@ -153,7 +155,10 @@ fn create_html_element(name: QualName, } // Step 6.1.2 - Root::upcast(HTMLUnknownElement::new(local_name, prefix, document)) + let element = Root::upcast::( + HTMLUnknownElement::new(local_name, prefix, document)); + element.set_custom_element_state(CustomElementState::Failed); + element }, }; }, @@ -162,6 +167,7 @@ fn create_html_element(name: QualName, // Steps 5.1-5.2 let element = create_native_html_element(name, prefix, document, creator); element.set_is(definition.name.clone()); + element.set_custom_element_state(CustomElementState::Undefined); match mode { // Step 5.3 CustomElementCreationMode::Synchronous => @@ -174,7 +180,15 @@ fn create_html_element(name: QualName, } } - create_native_html_element(name, prefix, document, creator) + // Steps 7.1-7.2 + let result = create_native_html_element(name.clone(), prefix, document, creator); + + // Step 7.3 + if is_valid_custom_element_name(&*name.local) || is.is_some() { + result.set_custom_element_state(CustomElementState::Undefined); + } + + result } pub fn create_native_html_element(name: QualName, diff --git a/components/script/dom/customelementregistry.rs b/components/script/dom/customelementregistry.rs index 100aae7cae3..a625d464129 100644 --- a/components/script/dom/customelementregistry.rs +++ b/components/script/dom/customelementregistry.rs @@ -18,7 +18,7 @@ use dom::bindings::reflector::{DomObject, Reflector, reflect_dom_object}; use dom::bindings::str::DOMString; use dom::document::Document; use dom::domexception::{DOMErrorName, DOMException}; -use dom::element::Element; +use dom::element::{CustomElementState, Element}; use dom::globalscope::GlobalScope; use dom::htmlelement::HTMLElement; use dom::node::{document_from_node, Node, window_from_node}; @@ -477,8 +477,11 @@ impl CustomElementDefinition { /// https://html.spec.whatwg.org/multipage/#concept-upgrade-an-element #[allow(unsafe_code)] pub fn upgrade_element(definition: Rc, element: &Element) { - // TODO: Steps 1-2 - // Track custom element state + // Steps 1-2 + let state = element.get_custom_element_state(); + if state == CustomElementState::Custom || state == CustomElementState::Failed { + return; + } // Step 3 for attr in element.attrs().iter() { @@ -504,8 +507,8 @@ pub fn upgrade_element(definition: Rc, element: &Elemen // Step 7 exception handling if let Err(error) = result { - // TODO: Step 7.1 - // Track custom element state + // Step 7.1 + element.set_custom_element_state(CustomElementState::Failed); // Step 7.2 element.clear_reaction_queue(); @@ -520,6 +523,10 @@ pub fn upgrade_element(definition: Rc, element: &Elemen return; } + // Step 8 + element.set_custom_element_state(CustomElementState::Custom); + + // Step 9 element.set_custom_element_definition(definition); } diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 270d7d7e020..6aeef3ce986 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -150,6 +150,8 @@ pub struct Element { /// https://dom.spec.whatwg.org/#concept-element-custom-element-definition #[ignore_heap_size_of = "Rc"] custom_element_definition: DOMRefCell>>, + /// https://dom.spec.whatwg.org/#concept-element-custom-element-state + custom_element_state: Cell, } impl fmt::Debug for Element { @@ -179,6 +181,15 @@ pub enum CustomElementCreationMode { Asynchronous, } +/// https://dom.spec.whatwg.org/#concept-element-custom-element-state +#[derive(Clone, Copy, PartialEq, Eq, HeapSizeOf, JSTraceable)] +pub enum CustomElementState { + Undefined, + Failed, + Uncustomized, + Custom, +} + impl ElementCreator { pub fn is_parser_created(&self) -> bool { match *self { @@ -255,6 +266,7 @@ impl Element { selector_flags: Cell::new(ElementSelectorFlags::empty()), custom_element_reaction_queue: Default::default(), custom_element_definition: Default::default(), + custom_element_state: Cell::new(CustomElementState::Uncustomized), } } @@ -289,6 +301,14 @@ impl Element { self.is.borrow().clone() } + pub fn set_custom_element_state(&self, state: CustomElementState) { + self.custom_element_state.set(state); + } + + pub fn get_custom_element_state(&self) -> CustomElementState { + self.custom_element_state.get() + } + pub fn set_custom_element_definition(&self, definition: Rc) { *self.custom_element_definition.borrow_mut() = Some(definition); } diff --git a/tests/unit/script/size_of.rs b/tests/unit/script/size_of.rs index d6c4a6ad7f8..903ffc5ddc3 100644 --- a/tests/unit/script/size_of.rs +++ b/tests/unit/script/size_of.rs @@ -31,9 +31,9 @@ macro_rules! sizeof_checker ( // Update the sizes here sizeof_checker!(size_event_target, EventTarget, 40); sizeof_checker!(size_node, Node, 184); -sizeof_checker!(size_element, Element, 424); -sizeof_checker!(size_htmlelement, HTMLElement, 440); -sizeof_checker!(size_div, HTMLDivElement, 440); -sizeof_checker!(size_span, HTMLSpanElement, 440); +sizeof_checker!(size_element, Element, 432); +sizeof_checker!(size_htmlelement, HTMLElement, 448); +sizeof_checker!(size_div, HTMLDivElement, 448); +sizeof_checker!(size_span, HTMLSpanElement, 448); sizeof_checker!(size_text, Text, 216); sizeof_checker!(size_characterdata, CharacterData, 216); diff --git a/tests/wpt/metadata/custom-elements/upgrading/Node-cloneNode.html.ini b/tests/wpt/metadata/custom-elements/upgrading/Node-cloneNode.html.ini deleted file mode 100644 index 10ac904ac52..00000000000 --- a/tests/wpt/metadata/custom-elements/upgrading/Node-cloneNode.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[Node-cloneNode.html] - type: testharness - [Inserting an element must not try to upgrade a custom element when it had already failed to upgrade once] - expected: FAIL - From aa9b9c10f315179df5d9fd6dcbf8a36129092b19 Mon Sep 17 00:00:00 2001 From: Connor Brewster Date: Wed, 9 Aug 2017 14:32:37 -0600 Subject: [PATCH 10/10] =?UTF-8?q?Change=20foo-=C3=A1=20to=20foo=C3=A1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These tests assumed that foo-á would create a HTMLUnknownElement; however, foo-á is a valid custom element name; therefore, according to the spec, a HTMLElement instace is created instead. I changed the name to fooá so a HTMLUnknownElement is created as the test expects. --- tests/wpt/mozilla/meta/MANIFEST.json | 4 ++-- tests/wpt/mozilla/tests/mozilla/collections.html | 4 ++-- tests/wpt/mozilla/tests/mozilla/prototypes.html | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index 155f9a504d4..9af1b5e0218 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -26534,7 +26534,7 @@ "testharness" ], "mozilla/collections.html": [ - "f1029d519aa7017a1a3d18a891a0774b9a39f847", + "aa060d1e5d1364224f2c2151e5f8b9580ac4427e", "testharness" ], "mozilla/createEvent-storageevent.html": [ @@ -27534,7 +27534,7 @@ "testharness" ], "mozilla/prototypes.html": [ - "bb4824ae242ed89485a272f6ef74631d6fea62be", + "e19d75682a83f4d2de1cbdf053aadfda181d6725", "testharness" ], "mozilla/proxy_setter.html": [ diff --git a/tests/wpt/mozilla/tests/mozilla/collections.html b/tests/wpt/mozilla/tests/mozilla/collections.html index 2f33466cfa8..d10ec90360d 100644 --- a/tests/wpt/mozilla/tests/mozilla/collections.html +++ b/tests/wpt/mozilla/tests/mozilla/collections.html @@ -63,7 +63,7 @@ -hi +hi diff --git a/tests/wpt/mozilla/tests/mozilla/prototypes.html b/tests/wpt/mozilla/tests/mozilla/prototypes.html index b509b46192e..478b89a6fb8 100644 --- a/tests/wpt/mozilla/tests/mozilla/prototypes.html +++ b/tests/wpt/mozilla/tests/mozilla/prototypes.html @@ -5,7 +5,7 @@ - foo + foo