From 33923da61e7a86b496b29bb6d204ab07001ac837 Mon Sep 17 00:00:00 2001 From: SGavrl <277843151+SGavrl@users.noreply.github.com> Date: Mon, 3 Aug 2026 14:07:25 +0000 Subject: [PATCH] Honor standard image source selection --- crates/obscura-js/js/bootstrap.js | 6 +- crates/obscura-js/src/runtime.rs | 121 ++++++++++++++++++++++++++++- crates/obscura-render/src/paint.rs | 98 ++++++++++++++++------- 3 files changed, 192 insertions(+), 33 deletions(-) diff --git a/crates/obscura-js/js/bootstrap.js b/crates/obscura-js/js/bootstrap.js index 8288fe7..03e7248 100644 --- a/crates/obscura-js/js/bootstrap.js +++ b/crates/obscura-js/js/bootstrap.js @@ -4216,8 +4216,7 @@ class HTMLImageElement extends Element { setAttribute(name, value) { const normalized = String(name).toLowerCase(); super.setAttribute(name, value); - if (normalized === "src" || normalized === "srcset" || normalized === "sizes" - || normalized === "data-src" || normalized === "data-srcset") { + if (normalized === "src" || normalized === "srcset" || normalized === "sizes") { this._imageSourceChanged(); } else if ((normalized === "onload" || normalized === "onerror") @@ -4227,8 +4226,7 @@ class HTMLImageElement extends Element { removeAttribute(name) { const normalized = String(name).toLowerCase(); super.removeAttribute(name); - if (normalized === "src" || normalized === "srcset" || normalized === "sizes" - || normalized === "data-src" || normalized === "data-srcset") { + if (normalized === "src" || normalized === "srcset" || normalized === "sizes") { this._imageSourceChanged(); } } diff --git a/crates/obscura-js/src/runtime.rs b/crates/obscura-js/src/runtime.rs index 8b2b09e..ad67771 100644 --- a/crates/obscura-js/src/runtime.rs +++ b/crates/obscura-js/src/runtime.rs @@ -2683,7 +2683,7 @@ mod tests { "1px", "2px", "3px", "4px", "5px", "2px", "rgb(70, 80, 90)", "column", "wrap", "center", - "space-between", "6px", "9px", "translate(3px, 4px)" + "space-between", "6px", "9px", "matrix(1, 0, 0, 1, 3, 4)" ]) ); @@ -5600,6 +5600,125 @@ mod tests { .unwrap() } + #[cfg(feature = "render")] + #[tokio::test(flavor = "current_thread")] + async fn parser_image_data_src_mutation_does_not_restart_lifecycle() { + let requests = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let seen = requests.clone(); + let png = two_by_three_png(); + let mut rt = parser_image_runtime( + r#""#, + move |url: &str| { + seen.lock().unwrap().push(url.to_string()); + Some(png.clone()) + }, + ); + rt.execute_script( + "observe-data-src-mutation", + r#" + globalThis.__dataSrcEvents = []; + const image = document.getElementById("image"); + image.addEventListener("load", () => __dataSrcEvents.push(image.currentSrc)); + void image.complete; + "#, + ) + .unwrap(); + rt.run_event_loop_bounded(100).await.unwrap(); + assert_eq!( + rt.evaluate("[image.complete, image.currentSrc, __dataSrcEvents]") + .unwrap(), + serde_json::json!([ + true, + "http://example.com/page/real.png", + ["http://example.com/page/real.png"] + ]) + ); + + rt.execute_script( + "mutate-non-source-data-attribute", + r#" + image.dataset.src = "ignored.png"; + globalThis.__afterDataSrcMutation = [image.complete, image.currentSrc]; + "#, + ) + .unwrap(); + rt.run_event_loop_bounded(100).await.unwrap(); + assert_eq!( + rt.evaluate("[__afterDataSrcMutation, __dataSrcEvents]") + .unwrap(), + serde_json::json!([ + [true, "http://example.com/page/real.png"], + ["http://example.com/page/real.png"] + ]) + ); + assert_eq!( + *requests.lock().unwrap(), + vec!["http://example.com/page/real.png".to_string()] + ); + } + + #[cfg(feature = "render")] + #[tokio::test(flavor = "current_thread")] + async fn parser_image_data_src_is_inert_until_script_assigns_src() { + let requests = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let seen = requests.clone(); + let png = two_by_three_png(); + let mut rt = parser_image_runtime( + r#""#, + move |url: &str| { + seen.lock().unwrap().push(url.to_string()); + Some(png.clone()) + }, + ); + assert_eq!( + rt.evaluate( + r#"(() => { + globalThis.image = document.getElementById("image"); + globalThis.__promotedEvents = []; + image.addEventListener("load", () => __promotedEvents.push(image.currentSrc)); + return [image.src, image.currentSrc, image.complete, + image.naturalWidth, image.naturalHeight]; + })()"#, + ) + .unwrap(), + serde_json::json!(["", "", true, 0, 0]) + ); + rt.run_event_loop_bounded(100).await.unwrap(); + assert!(requests.lock().unwrap().is_empty()); + + rt.execute_script( + "promote-data-src-through-page-script", + r#" + image.src = image.dataset.src; + globalThis.__afterSrcPromotion = [image.complete, image.currentSrc]; + "#, + ) + .unwrap(); + assert_eq!( + rt.evaluate("__afterSrcPromotion").unwrap(), + serde_json::json!([false, "http://example.com/page/promoted.png"]) + ); + rt.run_event_loop_bounded(100).await.unwrap(); + assert_eq!( + rt.evaluate( + "[image.complete, image.naturalWidth, image.naturalHeight, \ + image.currentSrc, __promotedEvents]" + ) + .unwrap(), + serde_json::json!([ + true, + 2, + 3, + "http://example.com/page/promoted.png", + ["http://example.com/page/promoted.png"] + ]) + ); + assert_eq!( + *requests.lock().unwrap(), + vec!["http://example.com/page/promoted.png".to_string()] + ); + } + #[cfg(feature = "render")] #[tokio::test(flavor = "current_thread")] async fn parser_image_lifecycle_uses_shared_render_resource() { diff --git a/crates/obscura-render/src/paint.rs b/crates/obscura-render/src/paint.rs index bc89dd5..448a17c 100644 --- a/crates/obscura-render/src/paint.rs +++ b/crates/obscura-render/src/paint.rs @@ -5166,12 +5166,10 @@ fn collect_content_image_intrinsics( /// Choose the URL to paint for an ``. Browsers do not use `src` alone: /// a wrapping ``'s ``s, `srcset`, and `sizes` select by -/// type/media/viewport/density, and lazy-loaded images keep the real URL in -/// `data-src`/`data-srcset` with `src` holding a 1x1 placeholder until script -/// swaps it in. Since obscura may not have run the site's lazy-load script, -/// resolve the same URL the browser would end up with: a matching `` -/// source first, then a real candidate from `srcset`/`data-srcset`, then a -/// non-inline `src`/`data-*` URL, then any `src` (an inlined data: image). +/// type/media/viewport/density. Non-standard lazy-loader attributes such as +/// `data-src` are deliberately ignored until page script copies them into the +/// real `src`/`srcset`; promoting them here changes `currentSrc`, lifecycle +/// events, and resource timing relative to browsers. fn resolve_img_url( tree: &DomTree, nid: obscura_dom::tree::NodeId, @@ -5184,30 +5182,15 @@ fn resolve_img_url( return Some(pick); } let sizes = node.get_attribute("sizes"); - for a in ["srcset", "data-srcset"] { - if let Some(v) = node.get_attribute(a) { - if let Some(pick) = best_srcset_candidate(v, sizes, viewport) { - return Some(pick); - } + if let Some(v) = node.get_attribute("srcset") { + if let Some(pick) = best_srcset_candidate(v, sizes, viewport) { + return Some(pick); } } - let url_attrs = ["src", "data-src", "data-lazy-src", "data-original", "data-fallback-src", "data-lazy"]; - // A non-inline URL first (a data: src is usually the lazy-load placeholder). - for a in url_attrs { - if let Some(v) = node.get_attribute(a) { - let v = v.trim(); - if !v.is_empty() && !v.starts_with("data:") { - return Some((v.to_string(), 1.0)); - } - } - } - // Otherwise fall back to whatever is there (an inlined data: image). - for a in url_attrs { - if let Some(v) = node.get_attribute(a) { - let v = v.trim(); - if !v.is_empty() { - return Some((v.to_string(), 1.0)); - } + if let Some(v) = node.get_attribute("src") { + let v = v.trim(); + if !v.is_empty() { + return Some((v.to_string(), 1.0)); } } None @@ -6538,6 +6521,65 @@ mod tests { ); } + #[test] + fn data_source_attributes_are_not_image_candidates_or_fetches() { + let tree = parse_html( + r#""#, + ); + let lazy = tree.get_element_by_id("lazy").expect("lazy image"); + let calls = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let loader_calls = calls.clone(); + let mut resources = RenderResourceCache::with_loader(move |_url: &str| { + loader_calls.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + None + }); + + assert_eq!(resolve_img_url(&tree, lazy, (800.0, 600.0)), None); + assert_eq!( + resources.image_element_metadata( + &tree, + lazy, + (800.0, 600.0), + Some("https://example.test/page"), + ), + None + ); + assert_eq!(calls.load(std::sync::atomic::Ordering::SeqCst), 0); + } + + #[test] + fn data_uri_src_remains_selected_when_data_src_is_present() { + const PLACEHOLDER: &str = "data:image/svg+xml,%3Csvg%20xmlns=%22http://www.w3.org/2000/svg%22%20width=%221%22%20height=%221%22/%3E"; + let tree = parse_html(&format!( + r#""# + )); + let lazy = tree.get_element_by_id("lazy").expect("lazy image"); + let calls = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let loader_calls = calls.clone(); + let mut resources = RenderResourceCache::with_loader(move |_url: &str| { + loader_calls.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + None + }); + + assert_eq!( + resolve_img_url(&tree, lazy, (800.0, 600.0)), + Some((PLACEHOLDER.to_string(), 1.0)) + ); + let metadata = resources + .image_element_metadata( + &tree, + lazy, + (800.0, 600.0), + Some("https://example.test/page"), + ) + .expect("selected placeholder"); + assert_eq!(metadata.0, PLACEHOLDER); + assert_eq!(metadata.2, Some((1.0, 1.0))); + assert_eq!(calls.load(std::sync::atomic::Ordering::SeqCst), 0); + } + #[test] fn enabled_bmp_and_ico_formats_have_metadata_and_full_raster_decode() { use std::io::Cursor;