From ab1498da5a6796eae47af1fe25ad79a41bb9a902 Mon Sep 17 00:00:00 2001 From: ldm0 Date: Mon, 24 Aug 2026 03:39:44 +0800 Subject: [PATCH] fix(images): reselect picture requests after removal --- .../wpt-cross-current/failed-cases.txt | 1 - .../wpt-cross-current/passed-cases.txt | 1 + .../mutation_commands/tree/removal.rs | 9 +- .../tree/removal_followups.rs | 6 ++ .../mutation_commands/tree/replacement.rs | 6 ++ .../mutation_commands/tree/resources.rs | 27 ++++- .../src/script_vm/tests/browser_api/images.rs | 99 +++++++++++++++++++ 7 files changed, 146 insertions(+), 3 deletions(-) diff --git a/moli-benchmark/wpt-cross-current/failed-cases.txt b/moli-benchmark/wpt-cross-current/failed-cases.txt index c0c1173051..b355a5ac35 100644 --- a/moli-benchmark/wpt-cross-current/failed-cases.txt +++ b/moli-benchmark/wpt-cross-current/failed-cases.txt @@ -3061,7 +3061,6 @@ html/semantics/embedded-content/the-img-element/sizes/parse-a-sizes-attribute-qu html/semantics/embedded-content/the-img-element/sizes/parse-a-sizes-attribute-standards-mode.html html/semantics/embedded-content/the-img-element/sizes/parse-a-sizes-attribute-width-1000px.html html/semantics/embedded-content/the-img-element/source-media-outside-doc.html -html/semantics/embedded-content/the-img-element/source-relevant-mutations.html html/semantics/embedded-content/the-img-element/svg-img-density-vs-zoom.html html/semantics/embedded-content/the-img-element/update-the-image-data/current-request-microtask-002.html html/semantics/embedded-content/the-img-element/update-the-image-data/lazy-out-of-band-load.html diff --git a/moli-benchmark/wpt-cross-current/passed-cases.txt b/moli-benchmark/wpt-cross-current/passed-cases.txt index a82370d948..fa64819f10 100644 --- a/moli-benchmark/wpt-cross-current/passed-cases.txt +++ b/moli-benchmark/wpt-cross-current/passed-cases.txt @@ -6168,6 +6168,7 @@ html/semantics/embedded-content/the-img-element/not-rendered-dimension-getter.ht html/semantics/embedded-content/the-img-element/not-rendered-image-loading-lazy.html html/semantics/embedded-content/the-img-element/null-image-source.html html/semantics/embedded-content/the-img-element/remove-element-and-scroll.html +html/semantics/embedded-content/the-img-element/source-relevant-mutations.html html/semantics/embedded-content/the-img-element/srcset-natural-size-in-dynamic-iframe.html html/semantics/embedded-content/the-img-element/srcset/avoid-reload-on-resize.html html/semantics/embedded-content/the-img-element/srcset/parse-a-srcset-attribute.html diff --git a/moli-renderer-v8/src/document_runtime/mutation_commands/tree/removal.rs b/moli-renderer-v8/src/document_runtime/mutation_commands/tree/removal.rs index 713674df01..d6d416c4ca 100644 --- a/moli-renderer-v8/src/document_runtime/mutation_commands/tree/removal.rs +++ b/moli-renderer-v8/src/document_runtime/mutation_commands/tree/removal.rs @@ -1,5 +1,8 @@ use super::super::{dom_binding_timing_started, record_dom_binding_timing}; -use super::{node_iterators::NodeIteratorRemovalPlan, policy::TreeMutationSourceProfile}; +use super::{ + node_iterators::NodeIteratorRemovalPlan, policy::TreeMutationSourceProfile, + resources::ImageRelevantMutationPlan, +}; use crate::{ custom_elements, document_runtime::{DocumentRuntime, DomHandle}, @@ -20,6 +23,7 @@ pub(super) struct TreeRemovalPlan { pub(super) node_iterator_plan: Option, pub(super) registry_retargets: Vec, pub(super) selected_option_owners_before_remove: Vec<(DomHandle, DomHandle)>, + pub(super) image_relevant_mutation_plan: ImageRelevantMutationPlan, } impl DocumentRuntime { @@ -61,6 +65,8 @@ impl DocumentRuntime { custom_elements::registry_association_retargets_before_removal(host_ptr, root); let selected_option_owners_before_remove = self.selected_option_owners_in_subtrees(std::slice::from_ref(&root)); + let image_relevant_mutation_plan = + self.image_relevant_mutation_plan_before_remove(parent, root); TreeRemovalPlan { parent, root, @@ -73,6 +79,7 @@ impl DocumentRuntime { node_iterator_plan, registry_retargets, selected_option_owners_before_remove, + image_relevant_mutation_plan, } } diff --git a/moli-renderer-v8/src/document_runtime/mutation_commands/tree/removal_followups.rs b/moli-renderer-v8/src/document_runtime/mutation_commands/tree/removal_followups.rs index a731a1971c..f25314cde4 100644 --- a/moli-renderer-v8/src/document_runtime/mutation_commands/tree/removal_followups.rs +++ b/moli-renderer-v8/src/document_runtime/mutation_commands/tree/removal_followups.rs @@ -45,6 +45,12 @@ impl DocumentRuntime { TreeMutationSideEffectSource::ParserTreeSink => {} } self.dispatch_tree_removal_custom_element_reactions(scope, host_ptr, removal_plan, profile); + self.queue_image_relevant_mutation_loads( + scope, + host_ptr, + &removal_plan.image_relevant_mutation_plan, + profile.subresource_request_initiator_type(), + ); } fn dispatch_tree_removal_pre_reaction_followups_after_change( diff --git a/moli-renderer-v8/src/document_runtime/mutation_commands/tree/replacement.rs b/moli-renderer-v8/src/document_runtime/mutation_commands/tree/replacement.rs index 2c36ea47e0..a723b4440d 100644 --- a/moli-renderer-v8/src/document_runtime/mutation_commands/tree/replacement.rs +++ b/moli-renderer-v8/src/document_runtime/mutation_commands/tree/replacement.rs @@ -323,5 +323,11 @@ impl DocumentRuntime { } } self.preserve_selectedness_for_insertion_plan(scope, host_ptr, insertion_plan); + self.queue_image_relevant_mutation_loads( + scope, + host_ptr, + &replacement_plan.removal.image_relevant_mutation_plan, + crate::types::SubresourceRequestInitiatorType::Script, + ); } } diff --git a/moli-renderer-v8/src/document_runtime/mutation_commands/tree/resources.rs b/moli-renderer-v8/src/document_runtime/mutation_commands/tree/resources.rs index 806b2d0c1d..049ca68a5f 100644 --- a/moli-renderer-v8/src/document_runtime/mutation_commands/tree/resources.rs +++ b/moli-renderer-v8/src/document_runtime/mutation_commands/tree/resources.rs @@ -222,7 +222,32 @@ impl DocumentRuntime { plan } - fn queue_image_relevant_mutation_loads( + pub(super) fn image_relevant_mutation_plan_before_remove( + &self, + parent: DomHandle, + root: DomHandle, + ) -> ImageRelevantMutationPlan { + if !self.dom_host.is_html_element_named(parent, "picture") { + return ImageRelevantMutationPlan::default(); + } + if self.dom_host.is_html_element_named(root, "source") { + return ImageRelevantMutationPlan { + pictures: vec![parent], + images: Vec::new(), + }; + } + ImageRelevantMutationPlan { + pictures: Vec::new(), + images: self + .dom_host + .is_html_element_named(root, "img") + .then_some(root) + .into_iter() + .collect(), + } + } + + pub(super) fn queue_image_relevant_mutation_loads( &self, scope: &mut v8::PinScope<'_, '_>, host_ptr: *mut JsContextHost, diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/images.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/images.rs index bbd26afbb3..c38e0d64fd 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/images.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/images.rs @@ -373,6 +373,105 @@ async fn image_current_src_is_empty_while_a_replacement_request_is_pending() { ); } +#[tokio::test] +async fn picture_source_and_image_tree_mutations_reselect_requests() { + let loader = ResourceRequestClient::new(&moli_fetch::FetchConfig::default()).expect("loader"); + loader.set_image_fetch_enabled(true); + let (mut vm, _resource_completion_queue) = + new_storage_test_vm_with_loader_and_resource_completion_queue( + "https://picture-source-removal.test/page.html", + &loader, + ); + + vm.eval( + r#" + (() => { + if (!document.documentElement) { + document.append(document.createElement("html")); + } + if (!document.body) { + document.documentElement.append(document.createElement("body")); + } + const picture = document.createElement("picture"); + const source1 = document.createElement("source"); + const source2 = document.createElement("source"); + const source3 = document.createElement("source"); + const source4 = document.createElement("source"); + const source5 = document.createElement("source"); + const image1 = document.createElement("img"); + const image2 = document.createElement("img"); + source1.srcset = "data:,s1"; + source2.srcset = "data:,s2"; + source3.srcset = "data:,s3"; + source4.srcset = "data:,s4"; + source5.srcset = "data:,s5"; + image1.src = "data:,img1"; + image2.src = "data:,img2"; + picture.append(source1, source2, source3, image1, image2, source4, source5); + const host = document.body; + host.append(picture); + globalThis.__pictureSourceMutation = { + host, + picture, + source1, + source2, + source3, + source4, + source5, + image1, + image2 + }; + })() + "#, + ) + .expect("picture source mutation setup should evaluate"); + + let current_sources = |vm: &mut ScriptVm| { + vm.eval( + "[__pictureSourceMutation.image1.currentSrc, \ + __pictureSourceMutation.image2.currentSrc].join(',')", + ) + .expect("picture image current sources should evaluate") + }; + assert_eq!(current_sources(&mut vm), "data:,s1,data:,s1"); + + vm.eval("__pictureSourceMutation.source1.remove()") + .expect("first picture source removal should evaluate"); + assert_eq!(current_sources(&mut vm), "data:,s2,data:,s2"); + + vm.eval("__pictureSourceMutation.source2.remove()") + .expect("second picture source removal should evaluate"); + assert_eq!(current_sources(&mut vm), "data:,s3,data:,s3"); + + vm.eval("__pictureSourceMutation.host.append(__pictureSourceMutation.source3)") + .expect("third picture source move should evaluate"); + assert_eq!(current_sources(&mut vm), "data:,img1,data:,img2"); + + vm.eval("__pictureSourceMutation.source4.remove()") + .expect("trailing picture source removal should evaluate"); + assert_eq!(current_sources(&mut vm), "data:,img1,data:,img2"); + + vm.eval("__pictureSourceMutation.host.append(__pictureSourceMutation.source5)") + .expect("trailing picture source move should evaluate"); + assert_eq!(current_sources(&mut vm), "data:,img1,data:,img2"); + + vm.eval("__pictureSourceMutation.picture.prepend(__pictureSourceMutation.source2)") + .expect("second picture source reinsertion should evaluate"); + assert_eq!(current_sources(&mut vm), "data:,s2,data:,s2"); + + vm.eval("__pictureSourceMutation.picture.prepend(__pictureSourceMutation.source1)") + .expect("first picture source reinsertion should evaluate"); + assert_eq!(current_sources(&mut vm), "data:,s1,data:,s1"); + + vm.eval("__pictureSourceMutation.source1.replaceWith(document.createElement('div'))") + .expect("first picture source replacement should evaluate"); + assert_eq!(current_sources(&mut vm), "data:,s2,data:,s2"); + + vm.eval("__pictureSourceMutation.image1.remove()") + .expect("first picture image removal should evaluate"); + assert_eq!(current_sources(&mut vm), "data:,img1,data:,s2"); +} + #[tokio::test] async fn image_fetch_enabled_dispatches_http_error_after_resource_timing() { let (base_url, request_rx, server) =