From a6fac77ff8eda64efa99573c732aac384d59ffd9 Mon Sep 17 00:00:00 2001 From: ldm0 Date: Mon, 24 Aug 2026 05:25:48 +0800 Subject: [PATCH] fix(dom): preserve surrogate IDs for selector matching --- moli-dom/src/native/attributes.rs | 30 +++++ moli-dom/src/native/element/mod.rs | 113 +++++++++++++++++- moli-dom/src/native/element/rare_data.rs | 38 ++++++ .../src/native/host/mutation/attributes.rs | 49 +++++++- .../src/document_runtime/mutation_commands.rs | 113 ++++++++++++++++++ moli-renderer-v8/src/native_bridge/element.rs | 58 +++++++-- .../src/native_bridge/element/reflection.rs | 41 ++++++- .../src/script_vm/tests/dom_xhr/dom.rs | 16 +-- moli-selector/src/stylo/query/element.rs | 10 ++ 9 files changed, 443 insertions(+), 25 deletions(-) diff --git a/moli-dom/src/native/attributes.rs b/moli-dom/src/native/attributes.rs index 73429a2964..8975b74818 100644 --- a/moli-dom/src/native/attributes.rs +++ b/moli-dom/src/native/attributes.rs @@ -73,6 +73,36 @@ impl NativeDom { changed } + pub fn set_attribute_utf16_units( + &mut self, + node_id: NativeNodeId, + name: &str, + value: &str, + units: Vec, + ) -> bool { + let Some(normalized_name) = self.normalized_attribute_name(node_id, name) else { + return false; + }; + let base_document = + self.base_element_attribute_owner_document(node_id, None, &normalized_name); + let changed = self + .node_mut(node_id) + .and_then(|node| node.data_mut().as_element_mut()) + .is_some_and(|element| { + element.set_attribute_utf16_units( + normalized_name, + String::new(), + None, + value.to_owned(), + units, + ) + }); + if changed && let Some(document) = base_document { + self.process_base_element(document, false); + } + changed + } + pub fn set_attribute_ns( &mut self, node_id: NativeNodeId, diff --git a/moli-dom/src/native/element/mod.rs b/moli-dom/src/native/element/mod.rs index 4124281696..69a0b54355 100644 --- a/moli-dom/src/native/element/mod.rs +++ b/moli-dom/src/native/element/mod.rs @@ -183,6 +183,15 @@ impl Element { .map(Attribute::value) } + pub fn attribute_utf16_units(&self, name: &str) -> Option<&[u16]> { + let qualified_name = self + .attributes + .iter() + .find(|attribute| attribute.name_matches(name)) + .map(Attribute::name)?; + self.rare_data.attribute_utf16_units(&qualified_name) + } + pub fn attribute_ns(&self, namespace: &str, local_name: &str) -> Option<&str> { self.attributes .iter() @@ -1136,10 +1145,18 @@ impl Element { // insertion paths). Once an attribute is matched by its existing qualified name, we // preserve its current namespace/prefix and only update the value. Callers that need // namespace/local-name replacement semantics must go through `set_attribute_ns()`. - if self.attributes[index].value() == value { + let qualified_name = self.attributes[index].name(); + if self.attributes[index].value() == value + && self + .rare_data + .attribute_utf16_units(&qualified_name) + .is_none() + { return false; } self.attributes[index].value = value.into_boxed_str(); + self.rare_data + .set_attribute_utf16_units(qualified_name, None); let attribute_local_name = self.attributes[index].local_name.clone(); self.sync_control_state_from_attribute( attribute_local_name.as_ref(), @@ -1159,6 +1176,58 @@ impl Element { true } + pub fn set_attribute_utf16_units( + &mut self, + local_name: String, + namespace: String, + prefix: Option, + value: String, + units: Vec, + ) -> bool { + let next_value = value.clone(); + let value_utf16_units = + utf16_units_contain_unpaired_surrogate(&units).then(|| units.into_boxed_slice()); + if let Some(index) = self + .attributes + .iter() + .position(|attribute| attribute.name_matches(&local_name)) + { + let qualified_name = self.attributes[index].name(); + if self.attributes[index].value.as_ref() == value + && self.rare_data.attribute_utf16_units(&qualified_name) + == value_utf16_units.as_deref() + { + return false; + } + self.attributes[index].value = value.into_boxed_str(); + self.rare_data + .set_attribute_utf16_units(qualified_name, value_utf16_units); + let attribute_local_name = self.attributes[index].local_name.clone(); + self.sync_control_state_from_attribute( + attribute_local_name.as_ref(), + Some(&next_value), + ); + return true; + } + + let attribute_local_name = LocalName::from(local_name); + self.attributes.push(Attribute { + local_name: attribute_local_name.clone(), + namespace: Namespace::from(namespace), + prefix: prefix.map(Prefix::from), + value: value.into_boxed_str(), + }); + let qualified_name = self + .attributes + .last() + .expect("new attribute must be present") + .name(); + self.rare_data + .set_attribute_utf16_units(qualified_name, value_utf16_units); + self.sync_control_state_from_attribute(attribute_local_name.as_ref(), Some(&next_value)); + true + } + pub fn set_attribute_ns( &mut self, local_name: String, @@ -1170,10 +1239,18 @@ impl Element { if let Some(index) = self.attributes.iter().position(|attribute| { attribute.local_name() == local_name && attribute.namespace() == namespace }) { - if self.attributes[index].value() == value { + let qualified_name = self.attributes[index].name(); + if self.attributes[index].value() == value + && self + .rare_data + .attribute_utf16_units(&qualified_name) + .is_none() + { return false; } self.attributes[index].value = value.into_boxed_str(); + self.rare_data + .set_attribute_utf16_units(qualified_name, None); self.sync_control_state_from_attribute(&local_name, Some(&next_value)); return true; } @@ -1197,7 +1274,10 @@ impl Element { else { return false; }; - let local_name = self.attributes.remove(index).local_name; + let removed = self.attributes.remove(index); + self.rare_data + .set_attribute_utf16_units(removed.name(), None); + let local_name = removed.local_name; self.sync_control_state_from_attribute(local_name.as_ref(), None); true } @@ -1208,7 +1288,10 @@ impl Element { }) else { return false; }; - let local_name = self.attributes.remove(index).local_name; + let removed = self.attributes.remove(index); + self.rare_data + .set_attribute_utf16_units(removed.name(), None); + let local_name = removed.local_name; self.sync_control_state_from_attribute(local_name.as_ref(), None); true } @@ -1220,3 +1303,25 @@ impl Element { } } } + +fn utf16_units_contain_unpaired_surrogate(units: &[u16]) -> bool { + let mut index = 0; + while index < units.len() { + let unit = units[index]; + if (0xD800..=0xDBFF).contains(&unit) { + if units + .get(index + 1) + .is_some_and(|next| (0xDC00..=0xDFFF).contains(next)) + { + index += 2; + continue; + } + return true; + } + if (0xDC00..=0xDFFF).contains(&unit) { + return true; + } + index += 1; + } + false +} diff --git a/moli-dom/src/native/element/rare_data.rs b/moli-dom/src/native/element/rare_data.rs index f43d4319b7..9a73fe3d07 100644 --- a/moli-dom/src/native/element/rare_data.rs +++ b/moli-dom/src/native/element/rare_data.rs @@ -30,6 +30,7 @@ struct ElementRareDataPayload { custom_element_is_name: Option, parser_associated_form_owner: Option, template_contents: Option, + attribute_utf16_units: Vec<(String, Box<[u16]>)>, } impl ElementRareDataPayload { @@ -39,6 +40,7 @@ impl ElementRareDataPayload { && self.custom_element_is_name.is_none() && self.parser_associated_form_owner.is_none() && self.template_contents.is_none() + && self.attribute_utf16_units.is_empty() } } @@ -63,10 +65,46 @@ impl ElementRareData { custom_element_is_name: None, parser_associated_form_owner: None, template_contents: None, + attribute_utf16_units: Vec::new(), })), } } + pub(super) fn attribute_utf16_units(&self, qualified_name: &str) -> Option<&[u16]> { + self.payload + .as_deref()? + .attribute_utf16_units + .iter() + .find(|(name, _)| name == qualified_name) + .map(|(_, units)| units.as_ref()) + } + + pub(super) fn set_attribute_utf16_units( + &mut self, + qualified_name: String, + units: Option>, + ) -> bool { + if self.attribute_utf16_units(&qualified_name) == units.as_deref() { + return false; + } + + if let Some(units) = units { + let values = &mut self.payload_mut().attribute_utf16_units; + if let Some((_, current)) = values.iter_mut().find(|(name, _)| name == &qualified_name) + { + *current = units; + } else { + values.push((qualified_name, units)); + } + } else if let Some(payload) = self.payload.as_deref_mut() { + payload + .attribute_utf16_units + .retain(|(name, _)| name != &qualified_name); + } + self.release_empty_payload(); + true + } + pub(super) fn control_state(&self) -> &ElementControlState { self.payload .as_deref() diff --git a/moli-dom/src/native/host/mutation/attributes.rs b/moli-dom/src/native/host/mutation/attributes.rs index 5a403c7cca..bf68241340 100644 --- a/moli-dom/src/native/host/mutation/attributes.rs +++ b/moli-dom/src/native/host/mutation/attributes.rs @@ -29,6 +29,17 @@ impl DomHost { self.set_attribute_effects(handle, name, value).did_change() } + pub fn set_attribute_utf16_units( + &mut self, + handle: DomHandle, + name: &str, + value: &str, + units: Vec, + ) -> bool { + self.set_attribute_utf16_units_effects(handle, name, value, units) + .did_change() + } + pub fn set_attribute_ns( &mut self, handle: DomHandle, @@ -56,6 +67,37 @@ impl DomHost { handle: DomHandle, name: &str, value: &str, + ) -> DomAttributeMutationOutcome { + self.set_attribute_mutation_outcome_with_utf16_units(handle, name, value, None) + } + + pub fn set_attribute_utf16_units_effects( + &mut self, + handle: DomHandle, + name: &str, + value: &str, + units: Vec, + ) -> DomMutationEffects { + self.set_attribute_utf16_units_mutation_outcome(handle, name, value, units) + .into_effects() + } + + pub fn set_attribute_utf16_units_mutation_outcome( + &mut self, + handle: DomHandle, + name: &str, + value: &str, + units: Vec, + ) -> DomAttributeMutationOutcome { + self.set_attribute_mutation_outcome_with_utf16_units(handle, name, value, Some(units)) + } + + fn set_attribute_mutation_outcome_with_utf16_units( + &mut self, + handle: DomHandle, + name: &str, + value: &str, + units: Option>, ) -> DomAttributeMutationOutcome { let records_enabled = self.mutation_records_enabled(); let prior_value = self.get_attribute(handle, name).map(Arc::from); @@ -91,7 +133,12 @@ impl DomHost { ) }) .unwrap_or_default(); - let changed = self.dom.set_attribute(handle, name, value); + let changed = if let Some(units) = units { + self.dom + .set_attribute_utf16_units(handle, name, value, units) + } else { + self.dom.set_attribute(handle, name, value) + }; if changed { self.invalidate_shadow_slot_name_index_for_attribute(handle, None, name); self.sync_select_state_after_option_selected_attribute(handle, name); diff --git a/moli-renderer-v8/src/document_runtime/mutation_commands.rs b/moli-renderer-v8/src/document_runtime/mutation_commands.rs index 13ce9879e1..b880a178da 100644 --- a/moli-renderer-v8/src/document_runtime/mutation_commands.rs +++ b/moli-renderer-v8/src/document_runtime/mutation_commands.rs @@ -699,6 +699,119 @@ impl DocumentRuntime { changed } + pub(crate) fn set_attribute_utf16_units_appending_to_current_reaction_queue( + &mut self, + scope: &mut v8::PinScope<'_, '_>, + host_ptr: *mut JsContextHost, + handle: DomHandle, + name: &str, + value: &str, + units: Vec, + ) -> bool { + self.set_attribute_utf16_units_with_reaction_policy( + scope, + host_ptr, + handle, + name, + value, + units, + AttributeChangedReactionPolicy::EnqueueInCurrentQueue, + ) + } + + #[allow(clippy::too_many_arguments)] + fn set_attribute_utf16_units_with_reaction_policy( + &mut self, + scope: &mut v8::PinScope<'_, '_>, + host_ptr: *mut JsContextHost, + handle: DomHandle, + name: &str, + value: &str, + units: Vec, + reaction_policy: AttributeChangedReactionPolicy, + ) -> bool { + let started = dom_binding_timing_started(); + unsafe { &mut *host_ptr }.break_on_dom_debugger_will_modify_dom_attribute(handle); + trace_resource_dom_mutation( + &self.dom_host, + "setAttribute", + handle, + Some(name), + Some(value), + ); + let style_impact = StyleAttributeImpact::for_attribute_name(name); + let state = style_state_impact_for_attribute_mutation(&self.dom_host, handle, None, name); + let old_style_state = self.retained_old_style_state_for_impact(host_ptr, handle, state); + let derived_old_style_states = self.retained_derived_old_style_states_for_attribute_impact( + host_ptr, handle, None, name, state, + ); + let (effects, old_value) = self + .dom_host + .set_attribute_utf16_units_mutation_outcome(handle, name, value, units) + .into_parts(); + if style_impact.affects_layout_metric() && old_value.as_deref() != Some(value) { + self.note_attribute_layout_activity(host_ptr, handle, name); + } + let changed = self.apply_runtime_mutation_effects( + scope, + host_ptr, + effects, + RuntimeMutationOptions::js_dom_api(), + ); + if changed { + self.handle_details_attribute_change( + scope, + host_ptr, + handle, + None, + name, + old_value.as_deref(), + Some(value), + reaction_policy, + ); + self.note_attribute_state_style_activity_if_needed( + host_ptr, + handle, + state, + old_style_state, + ); + self.note_derived_element_state_style_activity(host_ptr, &derived_old_style_states); + if name.eq_ignore_ascii_case("style") { + unsafe { &mut *host_ptr }.clear_element_inline_style_declaration_state(handle); + } + } + if should_dispatch_attribute_changed_for_set(changed, old_value.as_deref(), value) { + self.apply_attribute_changed_reaction_policy( + scope, + host_ptr, + handle, + name, + None, + old_value.as_deref(), + Some(value), + reaction_policy, + ); + } + if changed { + self.dispatch_custom_element_form_state_attribute_change(scope, host_ptr, handle, name); + } + if changed && name == "disabled" && self.dom_host.is_html_element_named(handle, "link") { + let _ = self.dom_host.set_link_explicitly_enabled(handle, false); + } + if changed + && !Self::apply_frame_owner_attribute_mutation_followup( + scope, host_ptr, handle, name, false, + ) + { + return changed; + } + record_dom_binding_timing( + attribute_operation_for_handle(&self.dom_host, "dom.setAttribute", handle, name), + started, + ); + changed + } + /// Applies frame-owner element semantics after an accepted attribute mutation. Returns false /// only when a `srcdoc` navigate event canceled the navigation. fn apply_frame_owner_attribute_mutation_followup( diff --git a/moli-renderer-v8/src/native_bridge/element.rs b/moli-renderer-v8/src/native_bridge/element.rs index a61654691a..611b349275 100644 --- a/moli-renderer-v8/src/native_bridge/element.rs +++ b/moli-renderer-v8/src/native_bridge/element.rs @@ -578,7 +578,7 @@ use reflection::{ property_dom_string_value, property_string_value, property_usv_string_value, remove_reflected_attribute, set_attribute_property_on_object_or_detached, set_boolean_attribute_property_on_object_or_detached, - set_dom_string_attribute_property_on_object, + set_dom_string_attribute_property_on_object, set_dom_string_attribute_property_utf16_on_object, set_nullable_dom_string_attribute_property_on_object, set_reflected_boolean_attribute, set_reflected_style_attribute_with_inline_base_url, set_usv_string_attribute_property_on_object, @@ -6519,6 +6519,53 @@ pub(in crate::native_bridge) fn set_live_element_attribute_appending_to_current_ did_set } +pub(in crate::native_bridge) fn set_live_element_attribute_utf16_units_appending_to_current_reaction_queue( + scope: &mut v8::PinScope<'_, '_>, + runtime_ptr: *mut JsContextHost, + handle: DomHandle, + name: &str, + value: &str, + units: Vec, +) -> bool { + clear_detached_iframe_context_before_navigation_attribute_change( + scope, + runtime_ptr, + handle, + name, + Some(value), + ); + let image_plan = + plan_image_attribute_mutation(unsafe { &*runtime_ptr }, handle, name, Some(value)); + let runtime = unsafe { &mut *runtime_ptr }; + let did_set = runtime.set_attribute_utf16_units_appending_to_current_reaction_queue( + scope, + runtime_ptr, + handle, + name, + value, + units, + ); + if did_set && name.eq_ignore_ascii_case("style") { + runtime.set_element_inline_style_current_base_url(handle); + } + if did_set { + apply_image_attribute_mutation_plan(scope, runtime_ptr, image_plan); + if name.eq_ignore_ascii_case("loading") { + queue_image_load_event_for_loading_change(scope, runtime_ptr, handle); + } + queue_media_load_if_source_or_loading_change(scope, runtime_ptr, handle, name); + queue_text_track_load_if_source(scope, runtime_ptr, handle, name); + } + crate::context_bootstrap::reset_html_canvas_backing_store_for_dimension_assignment( + scope, + runtime_ptr, + handle, + None, + name, + ); + did_set +} + pub(in crate::native_bridge) fn set_live_element_attribute_ns_appending_to_current_reaction_queue( scope: &mut v8::PinScope<'_, '_>, runtime_ptr: *mut JsContextHost, @@ -6745,14 +6792,7 @@ fn element_id_setter_function<'s>( if element_setter_receiver(scope, args.this(), "id").is_none() { return; } - set_dom_string_attribute_property_on_object( - scope, - args.this(), - "id", - args.get(0), - "Element", - "id", - ); + set_dom_string_attribute_property_utf16_on_object(scope, args.this(), "id", args.get(0)); rv.set_undefined(); } diff --git a/moli-renderer-v8/src/native_bridge/element/reflection.rs b/moli-renderer-v8/src/native_bridge/element/reflection.rs index f8a6c6d05f..6d19333525 100644 --- a/moli-renderer-v8/src/native_bridge/element/reflection.rs +++ b/moli-renderer-v8/src/native_bridge/element/reflection.rs @@ -1,6 +1,6 @@ use crate::custom_elements; use crate::document_runtime::DomHandle; -use crate::util::v8_string; +use crate::util::{string_from_utf16_units_lossy, v8_string, v8_value_to_dom_string_u16}; use crate::webidl; use super::super::node::{node_is_element, node_runtime_and_handle_from_object_or_detached}; @@ -8,6 +8,7 @@ use super::{ element_attribute, element_has_attribute, remove_live_element_attribute_appending_to_current_reaction_queue, set_live_element_attribute_appending_to_current_reaction_queue, + set_live_element_attribute_utf16_units_appending_to_current_reaction_queue, }; macro_rules! impl_reflection_callback_data { @@ -1203,6 +1204,26 @@ pub(super) fn set_reflected_style_attribute_with_inline_base_url( }); } +fn set_reflected_attribute_utf16_units( + scope: &mut v8::PinScope<'_, '_>, + runtime_ptr: *mut super::super::JsContextHost, + handle: DomHandle, + name: &str, + value: &str, + units: Vec, +) { + custom_elements::with_custom_element_reaction_scope(scope, runtime_ptr, |scope| { + let _ = set_live_element_attribute_utf16_units_appending_to_current_reaction_queue( + scope, + runtime_ptr, + handle, + name, + value, + units, + ); + }); +} + pub(super) fn remove_reflected_attribute( scope: &mut v8::PinScope<'_, '_>, runtime_ptr: *mut super::super::JsContextHost, @@ -1311,6 +1332,24 @@ pub(super) fn set_nullable_dom_string_attribute_property_on_object<'s>( set_reflected_attribute(scope, runtime_ptr, handle, name, &value); } +pub(super) fn set_dom_string_attribute_property_utf16_on_object<'s>( + scope: &mut v8::PinScope<'s, '_>, + object: v8::Local<'s, v8::Object>, + name: &str, + value: v8::Local<'s, v8::Value>, +) { + let Some(units) = v8_value_to_dom_string_u16(scope, value, false) else { + return; + }; + let Ok((runtime_ptr, handle)) = node_runtime_and_handle_from_object_or_detached(scope, object) + else { + return; + }; + let units = units.into_vec(); + let value = string_from_utf16_units_lossy(&units); + set_reflected_attribute_utf16_units(scope, runtime_ptr, handle, name, &value, units); +} + pub(super) fn attribute_property_getter_from_object_or_detached<'s>( scope: &mut v8::PinScope<'s, '_>, object: v8::Local<'s, v8::Object>, diff --git a/moli-renderer-v8/src/script_vm/tests/dom_xhr/dom.rs b/moli-renderer-v8/src/script_vm/tests/dom_xhr/dom.rs index 805312e52c..23eea9161b 100644 --- a/moli-renderer-v8/src/script_vm/tests/dom_xhr/dom.rs +++ b/moli-renderer-v8/src/script_vm/tests/dom_xhr/dom.rs @@ -4028,7 +4028,7 @@ fn dom_selector_queries_parse_webidl_strings() { ); } #[test] -fn reflected_id_lone_surrogates_use_the_lossy_dom_string_boundary() { +fn dom_selector_id_escapes_do_not_match_lone_surrogate_ids() { let mut vm = new_parsed_test_vm( "https://dom-selector-surrogate-escapes.test/path/index.html", r#""#, @@ -4058,14 +4058,10 @@ fn reflected_id_lone_surrogates_use_the_lossy_dom_string_boundary() { container.appendChild(surrogateSecond); return JSON.stringify({ - highProperty: surrogateFirst.id, - highAttribute: surrogateFirst.getAttribute("id"), - highMatchesReplacementEscape: surrogateFirst.matches("#\\d83d surrogateFirst"), - escapedHighFindsFirstReplacement: container.querySelector("#\\d83d surrogateFirst") === replacementFirst, - lowProperty: surrogateSecond.id, - lowAttribute: surrogateSecond.getAttribute("id"), - lowMatchesReplacementEscape: surrogateSecond.matches("#surrogateSecond\\dd11"), - escapedLowFindsFirstReplacement: container.querySelector("#surrogateSecond\\dd11") === replacementSecond + escapedHighMatchesReplacement: container.querySelector("#\\d83d surrogateFirst") === replacementFirst, + escapedHighDoesNotMatchSurrogate: container.querySelector("#\\d83d surrogateFirst") !== surrogateFirst, + escapedLowMatchesReplacement: container.querySelector("#surrogateSecond\\dd11") === replacementSecond, + escapedLowDoesNotMatchSurrogate: container.querySelector("#surrogateSecond\\dd11") !== surrogateSecond }); })() "##, @@ -4074,7 +4070,7 @@ fn reflected_id_lone_surrogates_use_the_lossy_dom_string_boundary() { assert_eq!( result, - r#"{"highProperty":"�surrogateFirst","highAttribute":"�surrogateFirst","highMatchesReplacementEscape":true,"escapedHighFindsFirstReplacement":true,"lowProperty":"surrogateSecond�","lowAttribute":"surrogateSecond�","lowMatchesReplacementEscape":true,"escapedLowFindsFirstReplacement":true}"# + r#"{"escapedHighMatchesReplacement":true,"escapedHighDoesNotMatchSurrogate":true,"escapedLowMatchesReplacement":true,"escapedLowDoesNotMatchSurrogate":true}"# ); } #[test] diff --git a/moli-selector/src/stylo/query/element.rs b/moli-selector/src/stylo/query/element.rs index 72afb044fa..ace6c3283c 100644 --- a/moli-selector/src/stylo/query/element.rs +++ b/moli-selector/src/stylo/query/element.rs @@ -688,6 +688,16 @@ impl SelectorsElement for QueryElement<'_> { id: &::Identifier, case_sensitivity: CaseSensitivity, ) -> bool { + if let Some(actual_units) = self.element().attribute_utf16_units("id") { + let expected_units = id.as_ref().encode_utf16().collect::>(); + return match case_sensitivity { + CaseSensitivity::CaseSensitive => actual_units == expected_units.as_slice(), + CaseSensitivity::AsciiCaseInsensitive => self + .element() + .id() + .is_some_and(|actual| actual.eq_ignore_ascii_case(id.as_ref())), + }; + } self.element() .id() .is_some_and(|actual| match case_sensitivity {