diff --git a/moli-benchmark/wpt-cross-current/failed-cases.txt b/moli-benchmark/wpt-cross-current/failed-cases.txt index 9614f4816f..a600ab1187 100644 --- a/moli-benchmark/wpt-cross-current/failed-cases.txt +++ b/moli-benchmark/wpt-cross-current/failed-cases.txt @@ -3137,7 +3137,6 @@ html/semantics/popovers/popover-shadow-dom.html html/semantics/popovers/popover-stacking.html html/semantics/popovers/popover-target-element-disabled.html html/semantics/popovers/popover-types-with-hints.html -html/semantics/popovers/popovertarget-reflection.html html/semantics/scripting-1/the-script-element/css-module/content-type-checking.html html/semantics/scripting-1/the-script-element/css-module/cors-crossorigin-requests.html html/semantics/scripting-1/the-script-element/execution-timing/083.html diff --git a/moli-benchmark/wpt-cross-current/passed-cases.txt b/moli-benchmark/wpt-cross-current/passed-cases.txt index 26d583b3bb..c0e7c47b6d 100644 --- a/moli-benchmark/wpt-cross-current/passed-cases.txt +++ b/moli-benchmark/wpt-cross-current/passed-cases.txt @@ -6578,6 +6578,7 @@ html/semantics/popovers/popover-toggle-source.html html/semantics/popovers/popover-types.html html/semantics/popovers/popover-undefined-remove-crash.html html/semantics/popovers/popovertarget-disabled-fieldset.html +html/semantics/popovers/popovertarget-reflection.html html/semantics/popovers/togglePopover.html html/semantics/popovers/toggleevent-interface.html html/semantics/rellist-feature-detection.html diff --git a/moli-dom/src/native/element/control_state.rs b/moli-dom/src/native/element/control_state.rs index 3dc7b210f9..9ee46680e6 100644 --- a/moli-dom/src/native/element/control_state.rs +++ b/moli-dom/src/native/element/control_state.rs @@ -126,7 +126,7 @@ impl ScriptElementState { } #[derive(Debug, Clone, Default)] -struct ExplicitAriaElementReferenceState { +struct ExplicitElementReferenceState { references_by_attribute: HashMap>, } @@ -171,36 +171,36 @@ pub struct ElementControlState { dialog_modal: bool, dialog_return_value: String, custom_states: IndexSet, - explicit_aria_element_references: Option>, + explicit_element_references: Option>, } impl ElementControlState { - pub fn explicit_aria_element_references(&self, attribute: &str) -> Option<&[NativeNodeId]> { - self.explicit_aria_element_references + pub fn explicit_element_references(&self, attribute: &str) -> Option<&[NativeNodeId]> { + self.explicit_element_references .as_deref()? .references_by_attribute .get(attribute) .map(Vec::as_slice) } - pub fn set_explicit_aria_element_references( + pub fn set_explicit_element_references( &mut self, attribute: &str, references: Vec, ) { - self.explicit_aria_element_references + self.explicit_element_references .get_or_insert_with(Default::default) .references_by_attribute .insert(attribute.to_owned(), references); } - pub fn clear_explicit_aria_element_references(&mut self, attribute: &str) { - let Some(references) = self.explicit_aria_element_references.as_mut() else { + pub fn clear_explicit_element_references(&mut self, attribute: &str) { + let Some(references) = self.explicit_element_references.as_mut() else { return; }; references.references_by_attribute.remove(attribute); if references.references_by_attribute.is_empty() { - self.explicit_aria_element_references = None; + self.explicit_element_references = None; } } diff --git a/moli-dom/src/native/element/mod.rs b/moli-dom/src/native/element/mod.rs index 69c5339f72..5f6ad8db55 100644 --- a/moli-dom/src/native/element/mod.rs +++ b/moli-dom/src/native/element/mod.rs @@ -59,7 +59,7 @@ fn initial_custom_element_state_for_identity( } } -fn is_aria_element_reference_attribute(name: &str) -> bool { +fn is_element_reference_attribute(name: &str) -> bool { matches!( name, "aria-activedescendant" @@ -70,6 +70,9 @@ fn is_aria_element_reference_attribute(name: &str) -> bool { | "aria-flowto" | "aria-labelledby" | "aria-owns" + | "commandfor" + | "interestfor" + | "popovertarget" ) } @@ -181,24 +184,22 @@ impl Element { self.prefix.as_ref().map(AsRef::as_ref) } - pub fn explicit_aria_element_references(&self, attribute: &str) -> Option<&[NativeNodeId]> { - self.control_state() - .explicit_aria_element_references(attribute) + pub fn explicit_element_references(&self, attribute: &str) -> Option<&[NativeNodeId]> { + self.control_state().explicit_element_references(attribute) } - pub fn set_explicit_aria_element_references( + pub fn set_explicit_element_references( &mut self, attribute: &str, references: Vec, ) { self.control_state_mut() - .set_explicit_aria_element_references(attribute, references); + .set_explicit_element_references(attribute, references); } - fn synchronize_aria_element_reference_attribute(&mut self, namespace: &str, local_name: &str) { - if namespace.is_empty() && is_aria_element_reference_attribute(local_name) { - self.rare_data - .clear_explicit_aria_element_references(local_name); + fn synchronize_element_reference_attribute(&mut self, namespace: &str, local_name: &str) { + if namespace.is_empty() && is_element_reference_attribute(local_name) { + self.rare_data.clear_explicit_element_references(local_name); } } @@ -1240,7 +1241,7 @@ impl Element { prefix: Option, value: String, ) -> bool { - self.synchronize_aria_element_reference_attribute(&namespace, &local_name); + self.synchronize_element_reference_attribute(&namespace, &local_name); let next_value = value.clone(); if let Some(index) = self .attributes @@ -1290,7 +1291,7 @@ impl Element { value: String, units: Vec, ) -> bool { - self.synchronize_aria_element_reference_attribute(&namespace, &local_name); + self.synchronize_element_reference_attribute(&namespace, &local_name); let next_value = value.clone(); let value_utf16_units = utf16_units_contain_unpaired_surrogate(&units).then(|| units.into_boxed_slice()); @@ -1342,7 +1343,7 @@ impl Element { prefix: Option, value: String, ) -> bool { - self.synchronize_aria_element_reference_attribute(&namespace, &local_name); + self.synchronize_element_reference_attribute(&namespace, &local_name); let next_value = value.clone(); if let Some(index) = self.attributes.iter().position(|attribute| { attribute.local_name() == local_name && attribute.namespace() == namespace @@ -1375,7 +1376,7 @@ impl Element { } pub fn remove_attribute(&mut self, name: &str) -> bool { - self.synchronize_aria_element_reference_attribute("", name); + self.synchronize_element_reference_attribute("", name); let Some(index) = self .attributes .iter() @@ -1392,7 +1393,7 @@ impl Element { } pub fn remove_attribute_ns(&mut self, namespace: &str, local_name: &str) -> bool { - self.synchronize_aria_element_reference_attribute(namespace, local_name); + self.synchronize_element_reference_attribute(namespace, local_name); let Some(index) = self.attributes.iter().position(|attribute| { attribute.namespace() == namespace && attribute.local_name() == local_name }) else { diff --git a/moli-dom/src/native/element/rare_data.rs b/moli-dom/src/native/element/rare_data.rs index bda1c1ec18..89cf53440e 100644 --- a/moli-dom/src/native/element/rare_data.rs +++ b/moli-dom/src/native/element/rare_data.rs @@ -118,13 +118,13 @@ impl ElementRareData { .get_or_insert_with(ElementControlState::default) } - pub(super) fn clear_explicit_aria_element_references(&mut self, attribute: &str) { + pub(super) fn clear_explicit_element_references(&mut self, attribute: &str) { if let Some(control_state) = self .payload .as_deref_mut() .and_then(|payload| payload.control_state.as_mut()) { - control_state.clear_explicit_aria_element_references(attribute); + control_state.clear_explicit_element_references(attribute); } } diff --git a/moli-dom/src/native/element/tests.rs b/moli-dom/src/native/element/tests.rs index 270f383a04..ac9b3fc5a5 100644 --- a/moli-dom/src/native/element/tests.rs +++ b/moli-dom/src/native/element/tests.rs @@ -155,7 +155,7 @@ fn template_contents_uses_and_releases_rare_data() { } #[test] -fn aria_element_reference_state_is_owned_by_the_content_attribute() { +fn element_reference_state_is_owned_by_the_content_attribute() { let mut element = Element::new_html("div"); assert!(element.set_attribute( "aria-controls".to_owned(), @@ -163,7 +163,7 @@ fn aria_element_reference_state_is_owned_by_the_content_attribute() { None, String::new(), )); - element.set_explicit_aria_element_references("aria-controls", vec![NativeNodeId::new(7)]); + element.set_explicit_element_references("aria-controls", vec![NativeNodeId::new(7)]); assert!(!element.set_attribute( "aria-controls".to_owned(), @@ -171,12 +171,9 @@ fn aria_element_reference_state_is_owned_by_the_content_attribute() { None, String::new(), )); - assert_eq!( - element.explicit_aria_element_references("aria-controls"), - None - ); + assert_eq!(element.explicit_element_references("aria-controls"), None); - element.set_explicit_aria_element_references("aria-controls", vec![NativeNodeId::new(8)]); + element.set_explicit_element_references("aria-controls", vec![NativeNodeId::new(8)]); assert!(element.set_attribute_ns( "aria-controls".to_owned(), "urn:example".to_owned(), @@ -184,15 +181,27 @@ fn aria_element_reference_state_is_owned_by_the_content_attribute() { "foreign".to_owned(), )); assert_eq!( - element.explicit_aria_element_references("aria-controls"), + element.explicit_element_references("aria-controls"), Some([NativeNodeId::new(8)].as_slice()) ); assert!(element.remove_attribute("aria-controls")); - assert_eq!( - element.explicit_aria_element_references("aria-controls"), - None - ); + assert_eq!(element.explicit_element_references("aria-controls"), None); + + assert!(element.set_attribute( + "popovertarget".to_owned(), + String::new(), + None, + String::new(), + )); + element.set_explicit_element_references("popovertarget", vec![NativeNodeId::new(9)]); + assert!(!element.set_attribute( + "popovertarget".to_owned(), + String::new(), + None, + String::new(), + )); + assert_eq!(element.explicit_element_references("popovertarget"), None); } #[test] diff --git a/moli-dom/src/native/host/mutation/attributes.rs b/moli-dom/src/native/host/mutation/attributes.rs index 52e03989e3..2978c38551 100644 --- a/moli-dom/src/native/host/mutation/attributes.rs +++ b/moli-dom/src/native/host/mutation/attributes.rs @@ -3,18 +3,18 @@ use std::sync::Arc; use super::*; impl DomHost { - pub fn explicit_aria_element_references( + pub fn explicit_element_references( &self, handle: DomHandle, attribute: &str, ) -> Option> { self.node(handle)? .as_element()? - .explicit_aria_element_references(attribute) + .explicit_element_references(attribute) .map(<[DomHandle]>::to_vec) } - pub fn set_explicit_aria_element_references( + pub fn set_explicit_element_references( &mut self, handle: DomHandle, attribute: &str, @@ -26,7 +26,7 @@ impl DomHost { else { return false; }; - element.set_explicit_aria_element_references(attribute, references); + element.set_explicit_element_references(attribute, references); true } diff --git a/moli-renderer-v8/src/native_bridge/context_host/core.rs b/moli-renderer-v8/src/native_bridge/context_host/core.rs index 60572034e9..e0f1019800 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/core.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/core.rs @@ -383,7 +383,6 @@ impl JsContextHost { child_web_storage_opaque_context_nonces: HashMap::new(), broadcast_channel_wrappers: HashMap::new(), form_past_named_items: HashMap::new(), - button_element_targets: HashMap::new(), constructing_form_data_forms: Vec::new(), active_form_submission_forms: Vec::new(), active_dialog_request_closes: HashSet::new(), @@ -1413,36 +1412,6 @@ impl JsContextHost { .then_some(item_handle) } - pub(in crate::native_bridge) fn remember_button_element_target( - &mut self, - source_handle: DomHandle, - slot: &str, - target_handle: DomHandle, - ) { - self.button_element_targets - .insert((source_handle, slot.to_owned()), target_handle); - } - - pub(in crate::native_bridge) fn clear_button_element_target( - &mut self, - source_handle: DomHandle, - slot: &str, - ) { - self.button_element_targets - .remove(&(source_handle, slot.to_owned())); - } - - pub(in crate::native_bridge) fn button_element_target( - &self, - source_handle: DomHandle, - slot: &str, - ) -> Option { - self.button_element_targets - .get(&(source_handle, slot.to_owned())) - .copied() - .filter(|handle| self.dom_host().node(*handle).is_some()) - } - pub(in crate::native_bridge) fn replace_active_image_submitter_coordinate( &mut self, coordinate: Option<(DomHandle, u32, u32)>, diff --git a/moli-renderer-v8/src/native_bridge/context_host/mod.rs b/moli-renderer-v8/src/native_bridge/context_host/mod.rs index 933807e5b6..8541bb9e55 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/mod.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/mod.rs @@ -989,7 +989,6 @@ pub(crate) struct JsContextHost { HashMap, broadcast_channel_wrappers: HashMap, form_past_named_items: HashMap<(DomHandle, String), DomHandle>, - button_element_targets: HashMap<(DomHandle, String), DomHandle>, constructing_form_data_forms: Vec, active_form_submission_forms: Vec, active_dialog_request_closes: HashSet, diff --git a/moli-renderer-v8/src/native_bridge/element.rs b/moli-renderer-v8/src/native_bridge/element.rs index 253e3d9621..b77419f58b 100644 --- a/moli-renderer-v8/src/native_bridge/element.rs +++ b/moli-renderer-v8/src/native_bridge/element.rs @@ -7659,7 +7659,7 @@ fn aria_element_reference_handle_for_owner( .or_else(|| (reference.runtime_ptr == owner_runtime_ptr).then_some(reference.handle)) } -fn aria_element_reference_is_in_valid_scope( +fn element_reference_is_in_valid_scope( runtime: &JsContextHost, owner: DomHandle, candidate: DomHandle, @@ -7690,7 +7690,7 @@ fn aria_element_reference_is_in_valid_scope( } } -fn aria_element_by_id_including_disconnected( +fn element_by_id_including_disconnected( runtime: &JsContextHost, owner: DomHandle, id: &str, @@ -7714,6 +7714,39 @@ fn aria_element_by_id_including_disconnected( None } +pub(in crate::native_bridge::element) fn reflected_element_attribute_handle( + runtime: &JsContextHost, + owner: DomHandle, + attribute: &str, +) -> Option { + let candidate = match runtime + .dom_host() + .explicit_element_references(owner, attribute) + { + Some(references) => references.into_iter().next()?, + None => { + let id = runtime.dom_host().get_attribute(owner, attribute)?; + element_by_id_including_disconnected(runtime, owner, &id)? + } + }; + if !element_reference_is_in_valid_scope(runtime, owner, candidate) { + return None; + } + runtime + .dom_host() + .resolve_reference_target_chain(candidate) + .map(|_| candidate) +} + +pub(in crate::native_bridge::element) fn resolved_reflected_element_attribute_handle( + runtime: &JsContextHost, + owner: DomHandle, + attribute: &str, +) -> Option { + reflected_element_attribute_handle(runtime, owner, attribute) + .and_then(|candidate| runtime.dom_host().resolve_reference_target_chain(candidate)) +} + fn aria_element_reference_content_handles( runtime: &JsContextHost, owner: DomHandle, @@ -7722,7 +7755,7 @@ fn aria_element_reference_content_handles( let value = runtime.dom_host().get_attribute(owner, attribute)?; if aria_element_reference_is_singular(attribute) { return Some( - aria_element_by_id_including_disconnected(runtime, owner, &value) + element_by_id_including_disconnected(runtime, owner, &value) .into_iter() .collect(), ); @@ -7731,7 +7764,7 @@ fn aria_element_reference_content_handles( value .split([' ', '\t', '\n', '\r', '\u{000c}']) .filter(|token| !token.is_empty()) - .filter_map(|token| aria_element_by_id_including_disconnected(runtime, owner, token)) + .filter_map(|token| element_by_id_including_disconnected(runtime, owner, token)) .collect(), ) } @@ -7743,14 +7776,12 @@ fn aria_element_reference_handles( ) -> Option> { match runtime .dom_host() - .explicit_aria_element_references(owner, attribute) + .explicit_element_references(owner, attribute) { Some(references) => Some( references .into_iter() - .filter(|candidate| { - aria_element_reference_is_in_valid_scope(runtime, owner, *candidate) - }) + .filter(|candidate| element_reference_is_in_valid_scope(runtime, owner, *candidate)) .collect(), ), None => aria_element_reference_content_handles(runtime, owner, attribute), @@ -7871,7 +7902,7 @@ fn set_explicit_aria_element_references( ); let _ = unsafe { &mut *runtime_ptr } .dom_host_mut() - .set_explicit_aria_element_references(owner, attribute, handles); + .set_explicit_element_references(owner, attribute, handles); }); } diff --git a/moli-renderer-v8/src/native_bridge/element/activation/default_action.rs b/moli-renderer-v8/src/native_bridge/element/activation/default_action.rs index 710f584b14..461f9d38dc 100644 --- a/moli-renderer-v8/src/native_bridge/element/activation/default_action.rs +++ b/moli-renderer-v8/src/native_bridge/element/activation/default_action.rs @@ -7,8 +7,8 @@ use crate::dom::{ native::{Element, Node, SelectedFile}, }; use crate::util::{ - call_object_method, get_private_value, node_wrapper_from_handle, object_bool_property, - object_number_property, object_property_as_object, utf16_len, v8_string, v8str, + call_object_method, node_wrapper_from_handle, object_bool_property, object_number_property, + object_property_as_object, utf16_len, v8_string, v8str, }; use crate::{ RendererInputDispatchOutcome, RendererPendingDownloadActivation, @@ -30,7 +30,8 @@ use super::super::{ is_disabled_form_control, is_focusable, is_valid_submit_button, label_activation_control_handle, observable_bounding_client_rect, perform_popover_invoker_default_action, perform_summary_click_default_action, - replace_text_control_selection, resolve_url_like_attribute, scroll_node_into_view_at_start, + replace_text_control_selection, resolve_url_like_attribute, + resolved_reflected_element_attribute_handle, scroll_node_into_view_at_start, submit_form_with_submit_event, update_focus, }; use super::targets::{ @@ -39,9 +40,6 @@ use super::targets::{ navigate_target_browsing_context, }; -const BUTTON_COMMAND_FOR_ELEMENT_SLOT: &str = "__moliButtonCommandForElement"; -const BUTTON_POPOVER_TARGET_ELEMENT_SLOT: &str = "__moliButtonPopoverTargetElement"; - fn array_like_length(scope: &mut v8::PinScope<'_, '_>, object: v8::Local<'_, v8::Object>) -> u32 { object .get(scope, v8str(scope, "length").into()) @@ -1543,7 +1541,7 @@ fn dispatch_button_command_event_if_needed( else { return; }; - let Some(target) = command_for_element_target(scope, runtime_ptr, handle) else { + let Some(target) = command_for_element_target(runtime_ptr, handle) else { return; }; let Some(source) = node_wrapper_from_handle(scope, handle) else { @@ -1568,7 +1566,7 @@ fn dispatch_button_popover_toggle_events_if_needed( if !runtime.dom_host().is_html_element_named(handle, "button") { return false; } - let Some(target) = popover_target_element_target(scope, runtime_ptr, handle) else { + let Some(target) = popover_target_element_target(runtime_ptr, handle) else { return false; }; dispatch_popover_toggle_events(scope, runtime_ptr, target, handle); @@ -1576,97 +1574,17 @@ fn dispatch_button_popover_toggle_events_if_needed( } fn popover_target_element_target( - scope: &mut v8::PinScope<'_, '_>, runtime_ptr: *mut JsContextHost, handle: DomHandle, ) -> Option { - if let Some(target) = - unsafe { &*runtime_ptr }.button_element_target(handle, BUTTON_POPOVER_TARGET_ELEMENT_SLOT) - { - return unsafe { &*runtime_ptr } - .dom_host() - .resolve_reference_target_chain(target); - } - if let Some(wrapper) = node_wrapper_from_handle(scope, handle) - && let Some(value) = get_private_value(scope, wrapper, BUTTON_POPOVER_TARGET_ELEMENT_SLOT) - && !value.is_null_or_undefined() - { - if let Ok(big) = v8::Local::::try_from(value) { - let (index, lossless) = big.u64_value(); - if lossless { - let target = DomHandle::new(index as usize); - if unsafe { &*runtime_ptr }.dom_host().node(target).is_some() { - return unsafe { &*runtime_ptr } - .dom_host() - .resolve_reference_target_chain(target); - } - } - } else if let Some(index) = value.uint32_value(scope) { - let target = DomHandle::new(index as usize); - if unsafe { &*runtime_ptr }.dom_host().node(target).is_some() { - return unsafe { &*runtime_ptr } - .dom_host() - .resolve_reference_target_chain(target); - } - } - } - let runtime = unsafe { &*runtime_ptr }; - let popover_target = element_attribute(runtime, handle, "popovertarget")?; - if popover_target.is_empty() { - return None; - } - reference_target_for_id(runtime, handle, &popover_target) + resolved_reflected_element_attribute_handle(unsafe { &*runtime_ptr }, handle, "popovertarget") } fn command_for_element_target( - scope: &mut v8::PinScope<'_, '_>, runtime_ptr: *mut JsContextHost, handle: DomHandle, ) -> Option { - if let Some(target) = - unsafe { &*runtime_ptr }.button_element_target(handle, BUTTON_COMMAND_FOR_ELEMENT_SLOT) - { - return unsafe { &*runtime_ptr } - .dom_host() - .resolve_reference_target_chain(target); - } - if let Some(wrapper) = node_wrapper_from_handle(scope, handle) - && let Some(value) = get_private_value(scope, wrapper, BUTTON_COMMAND_FOR_ELEMENT_SLOT) - && !value.is_null_or_undefined() - { - if let Ok(big) = v8::Local::::try_from(value) { - let (index, lossless) = big.u64_value(); - if lossless { - let target = DomHandle::new(index as usize); - if unsafe { &*runtime_ptr }.dom_host().node(target).is_some() { - return Some(target); - } - } - } else if let Some(index) = value.uint32_value(scope) { - let target = DomHandle::new(index as usize); - if unsafe { &*runtime_ptr }.dom_host().node(target).is_some() { - return Some(target); - } - } - } - let runtime = unsafe { &*runtime_ptr }; - let command_for = element_attribute(runtime, handle, "commandfor")?; - if command_for.is_empty() { - return None; - } - reference_target_for_id(runtime, handle, &command_for) -} - -fn reference_target_for_id( - runtime: &JsContextHost, - handle: DomHandle, - id: &str, -) -> Option { - let root = runtime.dom_host().root_node_handle(handle)?; - let candidate = runtime - .dom_host() - .element_handle_by_id_in_subtree(root, id)?; - runtime.dom_host().resolve_reference_target_chain(candidate) + resolved_reflected_element_attribute_handle(unsafe { &*runtime_ptr }, handle, "commandfor") } fn is_valid_reset_button(runtime: &JsContextHost, handle: DomHandle) -> bool { diff --git a/moli-renderer-v8/src/native_bridge/element/focus.rs b/moli-renderer-v8/src/native_bridge/element/focus.rs index 51e7d093e1..4b1d7e4cc8 100644 --- a/moli-renderer-v8/src/native_bridge/element/focus.rs +++ b/moli-renderer-v8/src/native_bridge/element/focus.rs @@ -11,6 +11,7 @@ use super::styles::{StyleMode, style_property_value}; use super::{ construct_click_event, construct_focus_event, construct_interest_event, dispatch_popover_show_events, dispatch_public_event, + resolved_reflected_element_attribute_handle, }; use super::{ element_attribute, element_has_attribute, label_control_handle, @@ -18,9 +19,7 @@ use super::{ }; use crate::document_runtime::{DomHandle, EventTargetHandle}; use crate::runtime::RendererDomFocusOutcome; -use crate::util::{get_private_value, node_wrapper_from_handle, v8_string, v8str}; - -const BUTTON_INTEREST_FOR_ELEMENT_SLOT: &str = "__moliButtonInterestForElement"; +use crate::util::{node_wrapper_from_handle, v8_string, v8str}; struct SequentialFocusEntry { tab_index: i32, @@ -554,7 +553,7 @@ fn dispatch_interest_event_if_needed( source_handle: DomHandle, event_type: &str, ) -> Option { - let target = interest_for_element_target(scope, runtime_ptr, source_handle)?; + let target = interest_for_element_target(runtime_ptr, source_handle)?; let source = node_wrapper_from_handle(scope, source_handle)?; let event = construct_interest_event(scope, event_type, source.into())?; let _ = dispatch_public_event(scope, runtime_ptr, target, event); @@ -1216,52 +1215,10 @@ fn show_interest_popover_if_needed( } fn interest_for_element_target( - scope: &mut v8::PinScope<'_, '_>, runtime_ptr: *mut JsContextHost, handle: DomHandle, ) -> Option { - if let Some(target) = - unsafe { &*runtime_ptr }.button_element_target(handle, BUTTON_INTEREST_FOR_ELEMENT_SLOT) - { - return unsafe { &*runtime_ptr } - .dom_host() - .resolve_reference_target_chain(target); - } - if let Some(wrapper) = node_wrapper_from_handle(scope, handle) - && let Some(value) = get_private_value(scope, wrapper, BUTTON_INTEREST_FOR_ELEMENT_SLOT) - && !value.is_null_or_undefined() - { - if let Ok(big) = v8::Local::::try_from(value) { - let (index, lossless) = big.u64_value(); - if lossless { - let target = DomHandle::new(index as usize); - if unsafe { &*runtime_ptr }.dom_host().node(target).is_some() { - return unsafe { &*runtime_ptr } - .dom_host() - .resolve_reference_target_chain(target); - } - } - } else if let Some(index) = value.uint32_value(scope) { - let target = DomHandle::new(index as usize); - if unsafe { &*runtime_ptr }.dom_host().node(target).is_some() { - return unsafe { &*runtime_ptr } - .dom_host() - .resolve_reference_target_chain(target); - } - } - } - let runtime = unsafe { &*runtime_ptr }; - let interest_for = runtime - .dom_host() - .node(handle) - .and_then(Node::as_element) - .and_then(|element| element.attribute("interestfor")) - .filter(|value| !value.is_empty())?; - let root = runtime.dom_host().root_node_handle(handle)?; - let candidate = runtime - .dom_host() - .element_handle_by_id_in_subtree(root, interest_for)?; - runtime.dom_host().resolve_reference_target_chain(candidate) + resolved_reflected_element_attribute_handle(unsafe { &*runtime_ptr }, handle, "interestfor") } pub(crate) fn focus_element( diff --git a/moli-renderer-v8/src/native_bridge/element/forms.rs b/moli-renderer-v8/src/native_bridge/element/forms.rs index 28a20b093c..d6c9ee636e 100644 --- a/moli-renderer-v8/src/native_bridge/element/forms.rs +++ b/moli-renderer-v8/src/native_bridge/element/forms.rs @@ -16,7 +16,7 @@ use super::super::{ node::{ append_child_in_reaction_scope, append_child_to_current_reaction_queue, current_or_live_delegate_node_arg_handle, insert_before_in_reaction_scope, - insert_before_to_current_reaction_queue, node_or_foreign_arg_handle, + insert_before_to_current_reaction_queue, node_is_element, node_or_foreign_arg_handle, node_or_foreign_arg_handle_allow_detached, node_runtime_and_handle_from_args, node_runtime_and_handle_from_args_or_detached, node_runtime_and_handle_from_object, node_runtime_and_handle_from_object_or_detached, remove_child_in_reaction_scope, @@ -32,9 +32,9 @@ use super::{ element_has_attribute, html_element_getter_receiver, html_element_setter_receiver, navigate_form_target_browsing_context, parse_non_negative_dimension, property_usv_string_value, queue_deferred_named_iframe_target_navigation_from_document, - queue_deferred_named_iframe_target_request, resolve_url_like_attribute, - set_attribute_property_on_object_or_detached, set_reflected_attribute, - set_reflected_boolean_attribute, update_focus, + queue_deferred_named_iframe_target_request, reflected_element_attribute_handle, + resolve_url_like_attribute, set_attribute_property_on_object_or_detached, + set_reflected_attribute, set_reflected_boolean_attribute, update_focus, }; use std::str::FromStr; diff --git a/moli-renderer-v8/src/native_bridge/element/forms/simple_controls.rs b/moli-renderer-v8/src/native_bridge/element/forms/simple_controls.rs index 4864e8cc07..095aee47d4 100644 --- a/moli-renderer-v8/src/native_bridge/element/forms/simple_controls.rs +++ b/moli-renderer-v8/src/native_bridge/element/forms/simple_controls.rs @@ -5,7 +5,6 @@ use crate::native_bridge::document::{ }; use crate::native_bridge::element::{html_element_getter_receiver, html_element_setter_receiver}; use crate::native_bridge::set_wrapped_handle_or_null_for_receiver; -use crate::util::{get_private_value, node_wrapper_from_handle, set_private_value}; use moli_dom::forms::{ MeterElementValues, ProgressElementValues, meter_element_values, progress_element_values, }; @@ -748,10 +747,6 @@ fn set_number_return( rv.set(v8::Number::new(scope, value).into()); } -const BUTTON_COMMAND_FOR_ELEMENT_SLOT: &str = "__moliButtonCommandForElement"; -const BUTTON_INTEREST_FOR_ELEMENT_SLOT: &str = "__moliButtonInterestForElement"; -const BUTTON_POPOVER_TARGET_ELEMENT_SLOT: &str = "__moliButtonPopoverTargetElement"; - pub(in crate::native_bridge) fn button_disabled_getter_function<'s>( scope: &mut v8::PinScope<'s, '_>, args: v8::FunctionCallbackArguments<'s>, @@ -1007,66 +1002,100 @@ pub(in crate::native_bridge) fn button_type_setter_function<'s>( rv.set_undefined(); } -fn private_command_for_element_handle<'s>( - scope: &mut v8::PinScope<'s, '_>, +struct ElementReflectionValue<'s> { object: v8::Local<'s, v8::Object>, -) -> Option { - private_button_element_handle(scope, object, BUTTON_COMMAND_FOR_ELEMENT_SLOT) + runtime_ptr: *mut JsContextHost, + handle: DomHandle, } -fn private_interest_for_element_handle<'s>( - scope: &mut v8::PinScope<'s, '_>, - object: v8::Local<'s, v8::Object>, -) -> Option { - private_button_element_handle(scope, object, BUTTON_INTEREST_FOR_ELEMENT_SLOT) +impl<'s> webidl::WebIdlConverter<'s> for ElementReflectionValue<'s> { + type Options = (); + + fn convert( + scope: &mut v8::PinScope<'s, '_>, + value: v8::Local<'s, v8::Value>, + _context: webidl::Context, + _options: &Self::Options, + ) -> std::result::Result { + let object = v8::Local::::try_from(value).map_err(|_| { + webidl::WebIdlError::custom_message("Element reflection values must be Elements.") + })?; + let (runtime_ptr, handle) = node_runtime_and_handle_from_object_or_detached(scope, object) + .map_err(|_| { + webidl::WebIdlError::custom_message("Element reflection values must be Elements.") + })?; + if !node_is_element(unsafe { &*runtime_ptr }, handle) { + return Err(webidl::WebIdlError::custom_message( + "Element reflection values must be Elements.", + )); + } + Ok(Self { + object, + runtime_ptr, + handle, + }) + } } -fn private_popover_target_element_handle<'s>( - scope: &mut v8::PinScope<'s, '_>, - object: v8::Local<'s, v8::Object>, +fn element_reflection_handle_for_owner( + scope: &mut v8::PinScope<'_, '_>, + owner_runtime_ptr: *mut JsContextHost, + reference: ElementReflectionValue<'_>, ) -> Option { - private_button_element_handle(scope, object, BUTTON_POPOVER_TARGET_ELEMENT_SLOT) + current_or_live_delegate_node_arg_handle(scope, owner_runtime_ptr, reference.object.into()) + .filter(|handle| node_is_element(unsafe { &*owner_runtime_ptr }, *handle)) + .or_else(|| (reference.runtime_ptr == owner_runtime_ptr).then_some(reference.handle)) } -fn private_button_element_handle<'s>( +fn set_button_element_reflection<'s>( scope: &mut v8::PinScope<'s, '_>, - object: v8::Local<'s, v8::Object>, - slot: &str, -) -> Option { - let value = get_private_value(scope, object, slot)?; + source: v8::Local<'s, v8::Object>, + value: v8::Local<'s, v8::Value>, + property: &'static str, + attribute: &'static str, +) { + let Some((runtime_ptr, owner)) = button_setter_receiver(scope, source, property) else { + return; + }; if value.is_null_or_undefined() { - return None; + custom_elements::with_custom_element_reaction_scope(scope, runtime_ptr, |scope| { + let _ = unsafe { &mut *runtime_ptr } + .remove_attribute_appending_to_current_reaction_queue( + scope, + runtime_ptr, + owner, + attribute, + ); + }); + return; } - if let Ok(big) = v8::Local::::try_from(value) { - let (index, lossless) = big.u64_value(); - return lossless.then(|| DomHandle::new(index as usize)); - } - value - .uint32_value(scope) - .map(|index| DomHandle::new(index as usize)) -} - -fn reflected_reference_target_handle( - runtime: &JsContextHost, - candidate: DomHandle, -) -> Option { - runtime - .dom_host() - .resolve_reference_target_chain(candidate) - .map(|_| candidate) -} - -fn reflected_attribute_target( - runtime: &JsContextHost, - source_handle: DomHandle, - attribute: &str, -) -> Option { - let id = element_attribute(runtime, source_handle, attribute)?; - let root = runtime.dom_host().root_node_handle(source_handle)?; - let candidate = runtime - .dom_host() - .element_handle_by_id_in_subtree(root, &id)?; - reflected_reference_target_handle(runtime, candidate) + let reference = match webidl::convert::>( + scope, + value, + webidl::Context::member("HTMLButtonElement", property), + ) { + Ok(reference) => reference, + Err(error) => { + webidl::throw_error(scope, &error); + return; + } + }; + let target = element_reflection_handle_for_owner(scope, runtime_ptr, reference); + custom_elements::with_custom_element_reaction_scope(scope, runtime_ptr, |scope| { + let runtime = unsafe { &mut *runtime_ptr }; + let _ = runtime.set_attribute_appending_to_current_reaction_queue( + scope, + runtime_ptr, + owner, + attribute, + "", + ); + let _ = runtime.dom_host_mut().set_explicit_element_references( + owner, + attribute, + target.into_iter().collect(), + ); + }); } fn set_wrapped_button_element_or_null<'s>( @@ -1095,18 +1124,14 @@ pub(in crate::native_bridge) fn button_interest_for_element_getter_function<'s>( mut rv: v8::ReturnValue<'_, v8::Value>, ) { let source = args.this(); - let Ok((runtime_ptr, source_handle)) = - node_runtime_and_handle_from_object_or_detached(scope, source) + let Some((runtime_ptr, source_handle)) = + button_getter_receiver(scope, source, "interestForElement") else { rv.set_null(); return; }; let runtime = unsafe { &mut *runtime_ptr }; - let target = runtime - .button_element_target(source_handle, BUTTON_INTEREST_FOR_ELEMENT_SLOT) - .or_else(|| private_interest_for_element_handle(scope, source)) - .and_then(|candidate| reflected_reference_target_handle(runtime, candidate)) - .or_else(|| reflected_attribute_target(runtime, source_handle, "interestfor")); + let target = reflected_element_attribute_handle(runtime, source_handle, "interestfor"); set_wrapped_button_element_or_null(scope, &mut rv, runtime_ptr, source, target); } @@ -1115,11 +1140,12 @@ pub(in crate::native_bridge) fn button_interest_for_element_setter_function<'s>( args: v8::FunctionCallbackArguments<'s>, mut rv: v8::ReturnValue<'_, v8::Value>, ) { - set_private_button_element_handle( + set_button_element_reflection( scope, args.this(), args.get(0), - BUTTON_INTEREST_FOR_ELEMENT_SLOT, + "interestForElement", + "interestfor", ); rv.set_undefined(); } @@ -1130,18 +1156,14 @@ pub(in crate::native_bridge) fn button_popover_target_element_getter_function<'s mut rv: v8::ReturnValue<'_, v8::Value>, ) { let source = args.this(); - let Ok((runtime_ptr, source_handle)) = - node_runtime_and_handle_from_object_or_detached(scope, source) + let Some((runtime_ptr, source_handle)) = + button_getter_receiver(scope, source, "popoverTargetElement") else { rv.set_null(); return; }; let runtime = unsafe { &mut *runtime_ptr }; - let target = runtime - .button_element_target(source_handle, BUTTON_POPOVER_TARGET_ELEMENT_SLOT) - .or_else(|| private_popover_target_element_handle(scope, source)) - .and_then(|candidate| reflected_reference_target_handle(runtime, candidate)) - .or_else(|| reflected_attribute_target(runtime, source_handle, "popovertarget")); + let target = reflected_element_attribute_handle(runtime, source_handle, "popovertarget"); set_wrapped_button_element_or_null(scope, &mut rv, runtime_ptr, source, target); } @@ -1150,11 +1172,12 @@ pub(in crate::native_bridge) fn button_popover_target_element_setter_function<'s args: v8::FunctionCallbackArguments<'s>, mut rv: v8::ReturnValue<'_, v8::Value>, ) { - set_private_button_element_handle( + set_button_element_reflection( scope, args.this(), args.get(0), - BUTTON_POPOVER_TARGET_ELEMENT_SLOT, + "popoverTargetElement", + "popovertarget", ); rv.set_undefined(); } @@ -1210,18 +1233,14 @@ pub(in crate::native_bridge) fn button_command_for_element_getter_function<'s>( mut rv: v8::ReturnValue<'_, v8::Value>, ) { let source = args.this(); - let Ok((runtime_ptr, source_handle)) = - node_runtime_and_handle_from_object_or_detached(scope, source) + let Some((runtime_ptr, source_handle)) = + button_getter_receiver(scope, source, "commandForElement") else { rv.set_null(); return; }; let runtime = unsafe { &mut *runtime_ptr }; - let target = runtime - .button_element_target(source_handle, BUTTON_COMMAND_FOR_ELEMENT_SLOT) - .or_else(|| private_command_for_element_handle(scope, source)) - .and_then(|candidate| reflected_reference_target_handle(runtime, candidate)) - .or_else(|| reflected_attribute_target(runtime, source_handle, "commandfor")); + let target = reflected_element_attribute_handle(runtime, source_handle, "commandfor"); set_wrapped_button_element_or_null(scope, &mut rv, runtime_ptr, source, target); } @@ -1230,84 +1249,12 @@ pub(in crate::native_bridge) fn button_command_for_element_setter_function<'s>( args: v8::FunctionCallbackArguments<'s>, mut rv: v8::ReturnValue<'_, v8::Value>, ) { - set_private_button_element_handle( + set_button_element_reflection( scope, args.this(), args.get(0), - BUTTON_COMMAND_FOR_ELEMENT_SLOT, + "commandForElement", + "commandfor", ); rv.set_undefined(); } - -fn set_private_button_element_handle<'s>( - scope: &mut v8::PinScope<'s, '_>, - source: v8::Local<'s, v8::Object>, - value: v8::Local<'s, v8::Value>, - slot: &str, -) { - let source_handle = node_runtime_and_handle_from_object_or_detached(scope, source) - .ok() - .map(|(_, handle)| handle); - if value.is_null_or_undefined() { - if let Ok((runtime_ptr, source_handle)) = - node_runtime_and_handle_from_object_or_detached(scope, source) - { - unsafe { &mut *runtime_ptr }.clear_button_element_target(source_handle, slot); - } - set_private_value(scope, source, slot, v8::null(scope).into()); - if let Some(source_handle) = source_handle - && let Some(wrapper) = node_wrapper_from_handle(scope, source_handle) - { - set_private_value(scope, wrapper, slot, v8::null(scope).into()); - } - return; - } - let Ok(object) = v8::Local::::try_from(value) else { - if let Ok((runtime_ptr, source_handle)) = - node_runtime_and_handle_from_object_or_detached(scope, source) - { - unsafe { &mut *runtime_ptr }.clear_button_element_target(source_handle, slot); - } - set_private_value(scope, source, slot, v8::null(scope).into()); - if let Some(source_handle) = source_handle - && let Some(wrapper) = node_wrapper_from_handle(scope, source_handle) - { - set_private_value(scope, wrapper, slot, v8::null(scope).into()); - } - return; - }; - let Ok((source_runtime_ptr, source_handle)) = - node_runtime_and_handle_from_object_or_detached(scope, source) - else { - return; - }; - let Ok((target_runtime_ptr, target_handle)) = - node_runtime_and_handle_from_object_or_detached(scope, object) - else { - unsafe { &mut *source_runtime_ptr }.clear_button_element_target(source_handle, slot); - set_private_value(scope, source, slot, v8::null(scope).into()); - if let Some(wrapper) = node_wrapper_from_handle(scope, source_handle) { - set_private_value(scope, wrapper, slot, v8::null(scope).into()); - } - return; - }; - if source_runtime_ptr != target_runtime_ptr { - unsafe { &mut *source_runtime_ptr }.clear_button_element_target(source_handle, slot); - set_private_value(scope, source, slot, v8::null(scope).into()); - if let Some(wrapper) = node_wrapper_from_handle(scope, source_handle) { - set_private_value(scope, wrapper, slot, v8::null(scope).into()); - } - return; - } - unsafe { &mut *source_runtime_ptr }.remember_button_element_target( - source_handle, - slot, - target_handle, - ); - let handle_value = v8::BigInt::new_from_u64(scope, target_handle.index() as u64); - set_private_value(scope, source, slot, handle_value.into()); - if let Some(wrapper) = node_wrapper_from_handle(scope, source_handle) { - let handle_value = v8::BigInt::new_from_u64(scope, target_handle.index() as u64); - set_private_value(scope, wrapper, slot, handle_value.into()); - } -} diff --git a/moli-renderer-v8/src/script_vm/tests/dom_elements/live_document.rs b/moli-renderer-v8/src/script_vm/tests/dom_elements/live_document.rs index 3ac38b32f2..ce70ffa226 100644 --- a/moli-renderer-v8/src/script_vm/tests/dom_elements/live_document.rs +++ b/moli-renderer-v8/src/script_vm/tests/dom_elements/live_document.rs @@ -3555,6 +3555,68 @@ fn button_and_textarea_accessors_live_on_owner_prototypes() { assert_eq!(result, "ok"); } +#[test] +fn button_element_reflections_track_content_attribute_mutations() { + let mut vm = new_parsed_test_vm( + "https://button-element-reflection.test/", + r#"
"#, + ); + + let result = vm + .eval( + r#" +(() => { + const assert = (condition, message) => { + if (!condition) throw new Error(message); + }; + const button = document.getElementById("button"); + const target = document.getElementById("target"); + + assert(button.popoverTargetElement === target, "content attribute target"); + button.popoverTargetElement = null; + assert(!button.hasAttribute("popovertarget"), "null removes content attribute"); + assert(button.popoverTargetElement === null, "null clears explicit target"); + + for (const [property, attribute] of [ + ["commandForElement", "commandfor"], + ["interestForElement", "interestfor"], + ["popoverTargetElement", "popovertarget"] + ]) { + button[property] = target; + assert(button.getAttribute(attribute) === "", `${property} writes empty content attribute`); + assert(button[property] === target, `${property} retains explicit target`); + + button.setAttribute(attribute, "missing"); + assert(button[property] === null, `${attribute} mutation clears explicit target`); + + button[property] = target; + button.setAttribute(attribute, ""); + assert(button[property] === null, `${attribute} same-value mutation clears explicit target`); + + button.setAttribute(attribute, "target"); + assert(button[property] === target, `${property} falls back to ID lookup`); + + let threw = false; + try { + button[property] = {}; + } catch (error) { + threw = error instanceof TypeError; + } + assert(threw, `${property} rejects non-Element values`); + assert(button.getAttribute(attribute) === "target", `${property} conversion precedes mutation`); + + button.removeAttribute(attribute); + assert(button[property] === null, `${attribute} removal clears target`); + } + return "ok"; +})() +"#, + ) + .expect("button element reflections should remain synchronized with their attributes"); + + assert_eq!(result, "ok"); +} + #[test] fn button_submitter_override_accessors_reject_incompatible_receivers() { let mut vm = new_parsed_test_vm(