fix(forms): block submission for unclosed controls

This commit is contained in:
ldm0
2026-09-16 22:07:06 +08:00
parent f5145deffc
commit d37e2cb62f
12 changed files with 287 additions and 6 deletions
@@ -167,6 +167,7 @@ pub struct ElementControlState {
scroll_top: Option<f64>,
scroll_left: Option<f64>,
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()
+15
View File
@@ -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;
@@ -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,
+4 -2
View File
@@ -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(
+94 -1
View File
@@ -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::<T>().as_mut() }.mark_script_already_started_for_parser(node_id);
}
unsafe fn mark_unclosed_form_control_for_parser_impl<T: ParserDomMutationConsumer>(
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::<T>().as_mut() }.mark_unclosed_form_control_for_parser(node_id);
}
unsafe fn finish_parsing_script_children_impl<T: ParserDomMutationConsumer>(
data: NonNull<()>,
node_id: NativeNodeId,
@@ -859,6 +870,7 @@ impl ParserDomMutationSink {
push_parse_error: push_parse_error_impl::<T>,
set_html_quirks_mode_for_parser: set_html_quirks_mode_for_parser_impl::<T>,
mark_script_already_started_for_parser: mark_script_already_started_for_parser_impl::<T>,
mark_unclosed_form_control_for_parser: mark_unclosed_form_control_for_parser_impl::<T>,
finish_parsing_script_children: finish_parsing_script_children_impl::<T>,
finish_parsing_link_children: finish_parsing_link_children_impl::<T>,
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",
"<!doctype html><form><select><option>secret<element attribute></element>",
true,
),
(
"select",
"<!doctype html><form><select><option>safe</option></select></form>",
false,
),
(
"textarea",
"<!doctype html><form><textarea>secret<element attribute></element>",
true,
),
(
"textarea",
"<!doctype html><form><textarea>safe</textarea></form>",
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!(
+37 -3
View File
@@ -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<ParseHandle, DocumentSink>,
}
#[derive(Default)]
struct OpenFormControlTracer {
handles: RefCell<Vec<NativeNodeId>>,
}
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 {
+5
View File
@@ -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);
@@ -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
@@ -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
@@ -127,6 +127,25 @@ fn submit_form_with_submit_event_inner(
submitter_handle: Option<DomHandle>,
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)
@@ -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
@@ -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/");