From d37e2cb62f26c9dac7cd68f5f080cee48f8a9b6f Mon Sep 17 00:00:00 2001 From: ldm0 Date: Wed, 2 Sep 2026 05:58:51 +0800 Subject: [PATCH] fix(forms): block submission for unclosed controls --- moli-dom/src/native/element/control_state.rs | 13 +++ moli-dom/src/native/element/mod.rs | 15 +++ moli-dom/src/native/host/mutation/state.rs | 16 ++++ moli-parser/src/html.rs | 6 +- moli-parser/src/live_target.rs | 95 ++++++++++++++++++- moli-parser/src/session.rs | 40 +++++++- moli-parser/src/stream.rs | 5 + .../src/document_runtime/document_write.rs | 7 ++ .../child_documents/live_parser.rs | 7 ++ .../native_bridge/element/forms/submission.rs | 19 ++++ .../src/runtime/phase_one/parser_turn.rs | 8 ++ .../src/script_vm/tests/dom_xhr/forms.rs | 62 ++++++++++++ 12 files changed, 287 insertions(+), 6 deletions(-) diff --git a/moli-dom/src/native/element/control_state.rs b/moli-dom/src/native/element/control_state.rs index 9ee46680e6..63c4dadbd7 100644 --- a/moli-dom/src/native/element/control_state.rs +++ b/moli-dom/src/native/element/control_state.rs @@ -167,6 +167,7 @@ pub struct ElementControlState { scroll_top: Option, scroll_left: Option, custom_validation_message: String, + blocks_form_submission: bool, popover_open: bool, dialog_modal: bool, dialog_return_value: String, @@ -444,6 +445,10 @@ impl ElementControlState { &self.custom_validation_message } + pub fn blocks_form_submission(&self) -> bool { + self.blocks_form_submission + } + pub fn popover_open(&self) -> bool { self.popover_open } @@ -586,6 +591,14 @@ impl ElementControlState { true } + pub fn set_blocks_form_submission(&mut self, blocks: bool) -> bool { + if self.blocks_form_submission == blocks { + return false; + } + self.blocks_form_submission = blocks; + true + } + pub fn set_script_force_async(&mut self, force_async: bool) -> bool { self.script .as_deref_mut() diff --git a/moli-dom/src/native/element/mod.rs b/moli-dom/src/native/element/mod.rs index 5f6ad8db55..4545b6a669 100644 --- a/moli-dom/src/native/element/mod.rs +++ b/moli-dom/src/native/element/mod.rs @@ -587,6 +587,12 @@ impl Element { self.control_state().indeterminate() } + pub fn blocks_form_submission(&self) -> bool { + matches!(self.local_name(), "select" | "textarea") + && self.namespace() == "http://www.w3.org/1999/xhtml" + && self.control_state().blocks_form_submission() + } + pub fn script_async(&self) -> bool { self.has_attribute("async") || self.control_state().script_force_async() } @@ -797,6 +803,15 @@ impl Element { self.control_state_mut().set_indeterminate(indeterminate) } + pub fn set_blocks_form_submission(&mut self, blocks: bool) -> bool { + if self.namespace() != "http://www.w3.org/1999/xhtml" + || !matches!(self.local_name(), "select" | "textarea") + { + return false; + } + self.control_state_mut().set_blocks_form_submission(blocks) + } + pub fn set_script_force_async(&mut self, force_async: bool) -> bool { if !self.is_script_element() { return false; diff --git a/moli-dom/src/native/host/mutation/state.rs b/moli-dom/src/native/host/mutation/state.rs index 14a625f217..343b143d27 100644 --- a/moli-dom/src/native/host/mutation/state.rs +++ b/moli-dom/src/native/host/mutation/state.rs @@ -931,6 +931,22 @@ impl DomHost { did_change } + pub fn set_blocks_form_submission(&mut self, handle: DomHandle, blocks: bool) -> bool { + let did_change = { + let Some(element) = self + .node_mut(handle) + .and_then(|node| node.data_mut().as_element_mut()) + else { + return false; + }; + element.set_blocks_form_submission(blocks) + }; + if did_change { + self.record_mutation(MutationScope::LocalState); + } + did_change + } + pub fn set_script_parser_inserted_for_prepare( &mut self, handle: DomHandle, diff --git a/moli-parser/src/html.rs b/moli-parser/src/html.rs index 13184b7c7d..3115e5d17d 100644 --- a/moli-parser/src/html.rs +++ b/moli-parser/src/html.rs @@ -1616,8 +1616,10 @@ impl DocumentSink { .pop_pending_blocking_stylesheet_pause() } - pub(super) fn begin_tree_builder_finish(&self) { - self.target.borrow_mut().begin_tree_builder_finish(); + pub(super) fn begin_tree_builder_finish(&self, unclosed_form_controls: &[NativeNodeId]) { + self.target + .borrow_mut() + .begin_tree_builder_finish(unclosed_form_controls); } pub(super) fn drain_discovered_blocking_stylesheet_inputs( diff --git a/moli-parser/src/live_target.rs b/moli-parser/src/live_target.rs index 864a9ed1bb..4fd4dcbb59 100644 --- a/moli-parser/src/live_target.rs +++ b/moli-parser/src/live_target.rs @@ -614,6 +614,8 @@ pub trait ParserDomMutationConsumer { fn mark_script_already_started_for_parser(&mut self, node_id: NativeNodeId); + fn mark_unclosed_form_control_for_parser(&mut self, node_id: NativeNodeId); + fn finish_parsing_script_children(&mut self, node_id: NativeNodeId); fn finish_parsing_link_children(&mut self, node_id: NativeNodeId); @@ -649,6 +651,7 @@ struct ParserDomMutationSink { push_parse_error: unsafe fn(NonNull<()>, String), set_html_quirks_mode_for_parser: unsafe fn(NonNull<()>, QuirksMode), mark_script_already_started_for_parser: unsafe fn(NonNull<()>, NativeNodeId), + mark_unclosed_form_control_for_parser: unsafe fn(NonNull<()>, NativeNodeId), finish_parsing_script_children: unsafe fn(NonNull<()>, NativeNodeId), finish_parsing_link_children: unsafe fn(NonNull<()>, NativeNodeId), maybe_clone_an_option_into_selectedcontent: unsafe fn(NonNull<()>, NativeNodeId), @@ -794,6 +797,14 @@ impl ParserDomMutationSink { // pointed-to consumer to remain live and exclusive for the pump step. unsafe { data.cast::().as_mut() }.mark_script_already_started_for_parser(node_id); } + unsafe fn mark_unclosed_form_control_for_parser_impl( + data: NonNull<()>, + node_id: NativeNodeId, + ) { + // SAFETY: ParserDomMutationSink::from_consumer_unchecked requires the + // pointed-to consumer to remain live and exclusive for the pump step. + unsafe { data.cast::().as_mut() }.mark_unclosed_form_control_for_parser(node_id); + } unsafe fn finish_parsing_script_children_impl( data: NonNull<()>, node_id: NativeNodeId, @@ -859,6 +870,7 @@ impl ParserDomMutationSink { push_parse_error: push_parse_error_impl::, set_html_quirks_mode_for_parser: set_html_quirks_mode_for_parser_impl::, mark_script_already_started_for_parser: mark_script_already_started_for_parser_impl::, + mark_unclosed_form_control_for_parser: mark_unclosed_form_control_for_parser_impl::, finish_parsing_script_children: finish_parsing_script_children_impl::, finish_parsing_link_children: finish_parsing_link_children_impl::, maybe_clone_an_option_into_selectedcontent: @@ -973,6 +985,12 @@ impl ParserDomMutationSink { unsafe { (self.mark_script_already_started_for_parser)(self.data, node_id) }; } + fn mark_unclosed_form_control_for_parser(self, node_id: NativeNodeId) { + // SAFETY: construction ties the raw pointer and callback to the same + // consumer remains live for the current runtime-DOM sink step. + unsafe { (self.mark_unclosed_form_control_for_parser)(self.data, node_id) }; + } + fn finish_parsing_script_children(self, node_id: NativeNodeId) { // SAFETY: construction ties the raw pointer and callback to the same // consumer remains live for the current runtime-DOM sink step. @@ -1537,6 +1555,12 @@ impl ParserDomMutationConsumer for TestMutationEffectCollector<'_> { let _ = unsafe { &mut *self.host }.set_script_already_started(node_id, true); } + fn mark_unclosed_form_control_for_parser(&mut self, node_id: NativeNodeId) { + // SAFETY: tests keep the borrowed DomHost pointer alive and route the + // parser pump through this collector for the duration of the step. + let _ = unsafe { &mut *self.host }.set_blocks_form_submission(node_id, true); + } + fn finish_parsing_script_children(&mut self, node_id: NativeNodeId) { // SAFETY: tests keep the borrowed DomHost pointer alive and route the // parser pump through this collector for the duration of the step. @@ -1896,6 +1920,12 @@ impl ParserDomMutationConsumer for TestReadTrackingCollector<'_> { let _ = unsafe { &mut *self.host }.set_script_already_started(node_id, true); } + fn mark_unclosed_form_control_for_parser(&mut self, node_id: NativeNodeId) { + // SAFETY: tests keep the borrowed DomHost pointer alive and route the + // parser pump through this collector for the duration of the step. + let _ = unsafe { &mut *self.host }.set_blocks_form_submission(node_id, true); + } + fn finish_parsing_script_children(&mut self, node_id: NativeNodeId) { // SAFETY: tests keep the borrowed DomHost pointer alive and route the // parser pump through this collector for the duration of the step. @@ -2826,6 +2856,18 @@ impl ParserStreamHtmlTreeSinkTarget { } } + fn mark_unclosed_form_control_for_dom_host(&mut self, node_id: NativeNodeId) { + if let Some(owner) = &self.runtime_dom_sinks { + owner + .dom_mutation_sink() + .mark_unclosed_form_control_for_parser(node_id); + } else { + let _ = self + .dom_host_mut() + .set_blocks_form_submission(node_id, true); + } + } + fn finish_parsing_script_children_for_dom_host(&mut self, node_id: NativeNodeId) { if let Some(owner) = &self.runtime_dom_sinks { owner @@ -2941,8 +2983,11 @@ impl ParserStreamHtmlTreeSinkTarget { self.state.pending_blocking_stylesheet_pause.take() } - pub(super) fn begin_tree_builder_finish(&mut self) { + pub(super) fn begin_tree_builder_finish(&mut self, unclosed_form_controls: &[NativeNodeId]) { self.state.finishing_tree_builder = true; + for &node_id in unclosed_form_controls { + self.mark_unclosed_form_control_for_dom_host(node_id); + } } pub(super) fn drain_discovered_blocking_stylesheet_inputs( @@ -3764,6 +3809,54 @@ fn parser_stream_html_tree_sink_target_builds_dom_and_records_parser_state() { ); } +#[test] +fn parser_stream_marks_only_eof_unclosed_form_controls_as_submission_blocking() { + let cases = [ + ( + "select", + "
", + false, + ), + ( + "textarea", + "
", + false, + ), + ]; + + for (tag_name, html, expected) in cases { + let url = Url::parse("https://dangling-markup.test/").expect("test url"); + let mut stream = + crate::DocumentStream::new_scripting_enabled_parser_stream_for_testing(url); + stream.feed(html); + let document = stream.finish(); + let control = document + .elements_by_tag_name(document.document_node_id(), tag_name, false) + .into_iter() + .next() + .expect("form control should be parsed"); + + assert_eq!( + document + .node(control) + .and_then(Node::as_element) + .is_some_and(|element| element.blocks_form_submission()), + expected, + "{tag_name} EOF state for {html:?}" + ); + } +} + #[test] fn parser_stream_async_prefetch_uses_shared_script_type_classification() { let html = concat!( diff --git a/moli-parser/src/session.rs b/moli-parser/src/session.rs index cd7aa47ef6..f75b9de734 100644 --- a/moli-parser/src/session.rs +++ b/moli-parser/src/session.rs @@ -7,7 +7,7 @@ use html5ever::{ tokenizer::{ BufferQueue, TagKind, Token, TokenSink, TokenSinkResult, Tokenizer, TokenizerOpts, }, - tree_builder::{TreeBuilder, TreeBuilderOpts, TreeSink}, + tree_builder::{Tracer, TreeBuilder, TreeBuilderOpts, TreeSink}, }; use markup5ever::TokenizerResult; use moli_dom::native::NativeNodeId; @@ -39,6 +39,32 @@ struct EmbedderPausingTreeBuilder { inner: TreeBuilder, } +#[derive(Default)] +struct OpenFormControlTracer { + handles: RefCell>, +} + +impl Tracer for OpenFormControlTracer { + type Handle = ParseHandle; + + fn trace_handle(&self, node: &ParseHandle) { + let is_blocking_control = node.element_name.as_ref().is_some_and(|name| { + name.ns.as_ref() == "http://www.w3.org/1999/xhtml" + && matches!(name.local.as_ref(), "select" | "textarea") + }); + if !is_blocking_control { + return; + } + let Some(node_id) = node.dom_node_id() else { + return; + }; + let mut handles = self.handles.borrow_mut(); + if !handles.contains(&node_id) { + handles.push(node_id); + } + } +} + impl EmbedderPausingTreeBuilder { fn new(sink: DocumentSink, opts: TreeBuilderOpts) -> Self { Self { @@ -59,6 +85,14 @@ impl EmbedderPausingTreeBuilder { fn sink(&self) -> &DocumentSink { &self.inner.sink } + + fn begin_tree_builder_finish(&self) { + let tracer = OpenFormControlTracer::default(); + self.inner.trace_handles(&tracer); + self.inner + .sink + .begin_tree_builder_finish(&tracer.handles.into_inner()); + } } impl EmbedderPausingTreeBuilder { @@ -294,7 +328,7 @@ impl HtmlParserSession { .pop_pending_blocking_stylesheet_pause(); } debug_assert!(input_buffer.is_empty()); - tokenizer.sink.sink().begin_tree_builder_finish(); + tokenizer.sink.begin_tree_builder_finish(); tokenizer.end(); tokenizer.sink.inner.sink.finish() } @@ -315,7 +349,7 @@ impl HtmlParserSession { .pop_pending_blocking_stylesheet_pause(); } debug_assert!(input_buffer.is_empty()); - tokenizer.sink.sink().begin_tree_builder_finish(); + tokenizer.sink.begin_tree_builder_finish(); tokenizer.end(); let sink = tokenizer.sink.sink(); ParserFinishDiscoverySignals { diff --git a/moli-parser/src/stream.rs b/moli-parser/src/stream.rs index a0b189c2a6..3e4b09dba3 100644 --- a/moli-parser/src/stream.rs +++ b/moli-parser/src/stream.rs @@ -1156,6 +1156,11 @@ mod tests { let _ = unsafe { &mut *self.host }.set_script_already_started(node_id, true); } + fn mark_unclosed_form_control_for_parser(&mut self, node_id: NativeNodeId) { + // SAFETY: the test keeps the DomHost alive for this parser pump step. + let _ = unsafe { &mut *self.host }.set_blocks_form_submission(node_id, true); + } + fn finish_parsing_script_children(&mut self, node_id: NativeNodeId) { // SAFETY: the test keeps the DomHost alive for this parser pump step. let _ = unsafe { &mut *self.host }.finish_parsing_script_children(node_id); diff --git a/moli-renderer-v8/src/document_runtime/document_write.rs b/moli-renderer-v8/src/document_runtime/document_write.rs index e7050ff02d..e0f9d83397 100644 --- a/moli-renderer-v8/src/document_runtime/document_write.rs +++ b/moli-renderer-v8/src/document_runtime/document_write.rs @@ -305,6 +305,13 @@ impl ParserDomMutationConsumer for DocumentWriteParserMutationOwner<'_, '_, '_> .mark_script_already_started_for_parser_in_live_dom_host(node_id); } + fn mark_unclosed_form_control_for_parser(&mut self, node_id: DomHandle) { + let _ = self + .runtime + .dom_host_mut() + .set_blocks_form_submission(node_id, true); + } + fn finish_parsing_script_children(&mut self, node_id: DomHandle) { let _ = self .runtime diff --git a/moli-renderer-v8/src/native_bridge/context_host/child_documents/live_parser.rs b/moli-renderer-v8/src/native_bridge/context_host/child_documents/live_parser.rs index 02e6c88e61..ce2af2f7ba 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/child_documents/live_parser.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/child_documents/live_parser.rs @@ -470,6 +470,13 @@ impl ParserDomMutationConsumer for ChildFrameLiveParserOwner<'_, '_, '_> { .set_script_already_started(node_id, true); } + fn mark_unclosed_form_control_for_parser(&mut self, node_id: DomHandle) { + let _ = self + .host + .dom_host_mut() + .set_blocks_form_submission(node_id, true); + } + fn finish_parsing_script_children(&mut self, node_id: DomHandle) { let _ = self .host diff --git a/moli-renderer-v8/src/native_bridge/element/forms/submission.rs b/moli-renderer-v8/src/native_bridge/element/forms/submission.rs index 16102f6e19..f467eca239 100644 --- a/moli-renderer-v8/src/native_bridge/element/forms/submission.rs +++ b/moli-renderer-v8/src/native_bridge/element/forms/submission.rs @@ -127,6 +127,25 @@ fn submit_form_with_submit_event_inner( submitter_handle: Option, user_initiated: bool, ) -> bool { + let blocked_by_unclosed_form_control = { + let runtime = unsafe { &*runtime_ptr }; + form_control_elements(runtime, form_handle) + .into_iter() + .any(|handle| { + runtime + .dom_host() + .node(handle) + .and_then(Node::as_element) + .is_some_and(Element::blocks_form_submission) + }) + }; + if blocked_by_unclosed_form_control { + if let Some(event) = construct_simple_event(scope, "error", false, false, false) { + let _ = dispatch_public_event(scope, runtime_ptr, form_handle, event); + } + return false; + } + let skips_constraint_validation = { let runtime = unsafe { &*runtime_ptr }; form_submission_skips_constraint_validation(runtime, form_handle, submitter_handle) diff --git a/moli-renderer-v8/src/runtime/phase_one/parser_turn.rs b/moli-renderer-v8/src/runtime/phase_one/parser_turn.rs index ecabdb231a..57e064240a 100644 --- a/moli-renderer-v8/src/runtime/phase_one/parser_turn.rs +++ b/moli-renderer-v8/src/runtime/phase_one/parser_turn.rs @@ -322,6 +322,14 @@ impl ParserDomMutationConsumer for PhaseOneParserOwner<'_> { .mark_script_already_started_for_parser_in_live_dom_host(node_id); } + fn mark_unclosed_form_control_for_parser(&mut self, node_id: NativeNodeId) { + let _ = self + .vm + .document_runtime + .dom_host_mut() + .set_blocks_form_submission(node_id, true); + } + fn finish_parsing_script_children(&mut self, node_id: NativeNodeId) { let _ = self .vm diff --git a/moli-renderer-v8/src/script_vm/tests/dom_xhr/forms.rs b/moli-renderer-v8/src/script_vm/tests/dom_xhr/forms.rs index 22461bb0f5..d66282125f 100644 --- a/moli-renderer-v8/src/script_vm/tests/dom_xhr/forms.rs +++ b/moli-renderer-v8/src/script_vm/tests/dom_xhr/forms.rs @@ -2968,6 +2968,68 @@ fn live_form_request_submit_dispatches_submit_event_with_submitter() { assert_eq!(result, "true:true:submit:true:true:true|true:true:true"); } + +#[test] +fn eof_unclosed_form_control_dispatches_error_before_validation_or_submit() { + let mut vm = new_storage_test_vm("https://dangling-markup-form.test/"); + + vm.eval( + r#" +(() => { + const parent = document.body || document.documentElement || document; + const form = document.createElement('form'); + form.id = 'dangling-form'; + const required = document.createElement('input'); + required.required = true; + const select = document.createElement('select'); + select.id = 'unclosed-select'; + form.append(required, select); + parent.appendChild(form); + + globalThis.__danglingFormEvents = []; + form.addEventListener('error', event => { + __danglingFormEvents.push([ + event.type, + event.target === form, + event.bubbles, + event.cancelable + ].join(':')); + }); + required.addEventListener('invalid', () => __danglingFormEvents.push('invalid')); + form.addEventListener('submit', () => __danglingFormEvents.push('submit')); +})() +"#, + ) + .expect("dangling-markup form setup should evaluate"); + + let select = vm + .document_runtime + .dom_host() + .element_handle_by_id("unclosed-select") + .expect("select should exist"); + assert!( + vm.document_runtime + .dom_host_mut() + .set_blocks_form_submission(select, true), + "parser EOF state should be recorded on the select" + ); + + let result = vm + .eval( + r#" +document.getElementById('dangling-form').requestSubmit(); +JSON.stringify(__danglingFormEvents) +"#, + ) + .expect("blocked form submission should evaluate"); + + assert_eq!(result, r#"["error:true:false:false"]"#); + assert!( + vm.take_pending_location_navigation_with_seed().is_none(), + "blocked form submission must not queue navigation" + ); +} + #[test] fn live_form_request_submit_parses_webidl_submitter_argument() { let mut vm = new_storage_test_vm("https://request-submit-webidl.test/");