fix(images): read rendered dimensions from layout snapshots

moli-renderer-v8/src/native_bridge/element/images/dimensions.rs

@@ -2,14 +2,18 @@ use crate::document_runtime::DomHandle;
 use crate::webidl;
 
 use super::super::super::{JsContextHost, node::node_runtime_and_handle_from_object_or_detached};
-use super::super::{element_attribute, parse_non_negative_dimension, set_reflected_attribute};
+use super::super::{
+    element_attribute, geometry::observable_element_metrics, set_reflected_attribute,
+};
 
 pub(in crate::native_bridge) fn image_width_getter_function<'s>(
     scope: &mut v8::PinScope<'s, '_>,
     args: v8::FunctionCallbackArguments<'s>,
     mut rv: v8::ReturnValue<'s, v8::Value>,
 ) {
-    rv.set_uint32(image_width_value(scope, args.this()));
+    if let Some(value) = image_dimension_value(scope, args.this(), true) {
+        rv.set_uint32(value);
+    }
 }
 
 pub(in crate::native_bridge) fn image_width_setter_function<'s>(
@@ -26,7 +30,9 @@ pub(in crate::native_bridge) fn image_height_getter_function<'s>(
     args: v8::FunctionCallbackArguments<'s>,
     mut rv: v8::ReturnValue<'s, v8::Value>,
 ) {
-    rv.set_uint32(image_height_value(scope, args.this()));
+    if let Some(value) = image_dimension_value(scope, args.this(), false) {
+        rv.set_uint32(value);
+    }
 }
 
 pub(in crate::native_bridge) fn image_height_setter_function<'s>(
@@ -44,36 +50,68 @@ pub(in crate::native_bridge) fn image_height_setter_function<'s>(
     rv.set_undefined();
 }
 
-fn image_width_value<'s>(
+fn image_dimension_value<'s>(
     scope: &mut v8::PinScope<'s, '_>,
     object: v8::Local<'s, v8::Object>,
-) -> u32 {
+    horizontal: bool,
+) -> Option<u32> {
     let Ok((runtime_ptr, handle)) = node_runtime_and_handle_from_object_or_detached(scope, object)
     else {
-        return 0;
+        return Some(0);
     };
     let runtime = unsafe { &*runtime_ptr };
-    element_attribute(runtime, handle, "width")
-        .map(|value| parse_non_negative_dimension(Some(value)))
-        .filter(|value| *value > 0)
-        .or_else(|| image_intrinsic_dimensions(runtime, handle).map(|(width, _)| width))
-        .unwrap_or(0)
+    let attribute = if horizontal { "width" } else { "height" };
+    if runtime.layout_policy().uses_real_layout() {
+        // Like Blink's LayoutBoxWidth/Height, use the content box before CSS
+        // transforms and with absolute zoom removed. SynchronousGeometry keeps
+        // Moli's existing frozen-tree contract: this is not a forced refresh.
+        match observable_element_metrics(
+            runtime,
+            handle,
+            moli_layout::LayoutFlushReason::SynchronousGeometry,
+        ) {
+            Ok(Some(metrics)) => {
+                let size = if horizontal {
+                    metrics.content_size.width
+                } else {
+                    metrics.content_size.height
+                };
+                return Some(size.round() as u32);
+            }
+            Ok(None) => {}
+            Err(error) => {
+                let message = format!("Layout failed while reading image {attribute}: {error}");
+                if let Some(message) = crate::util::v8_string(scope, &message) {
+                    let exception = v8::Exception::error(scope, message);
+                    scope.throw_exception(exception);
+                }
+                return None;
+            }
+        }
+    }
+    // No rendered box (or Mock): a valid zero attribute is different from an
+    // absent/invalid one and must not fall back to the decoded natural size.
+    Some(
+        element_attribute(runtime, handle, attribute)
+            .as_deref()
+            .and_then(parse_dimension_attribute)
+            .or_else(|| {
+                image_intrinsic_dimensions(runtime, handle)
+                    .map(|(width, height)| if horizontal { width } else { height })
+            })
+            .unwrap_or(0),
+    )
 }
 
-fn image_height_value<'s>(
-    scope: &mut v8::PinScope<'s, '_>,
-    object: v8::Local<'s, v8::Object>,
-) -> u32 {
-    let Ok((runtime_ptr, handle)) = node_runtime_and_handle_from_object_or_detached(scope, object)
-    else {
-        return 0;
-    };
-    let runtime = unsafe { &*runtime_ptr };
-    element_attribute(runtime, handle, "height")
-        .map(|value| parse_non_negative_dimension(Some(value)))
-        .filter(|value| *value > 0)
-        .or_else(|| image_intrinsic_dimensions(runtime, handle).map(|(_, height)| height))
-        .unwrap_or(0)
+fn parse_dimension_attribute(value: &str) -> Option<u32> {
+    // Blink's ParseHTMLNonNegativeInteger accepts a digit prefix and -0, but
+    // rejects overflow, negative nonzero values and non-HTML whitespace.
+    let value = value.trim_start_matches([' ', '\t', '\r', '\n', '\u{000c}']);
+    let negative = value.starts_with('-');
+    let digits = value.strip_prefix(['+', '-']).unwrap_or(value);
+    let end = digits.bytes().take_while(u8::is_ascii_digit).count();
+    let parsed = digits[..end].parse::<u32>().ok()?;
+    (!negative || parsed == 0).then_some(parsed)
 }
 
 fn set_image_unsigned_long_attribute_on_object<'s>(

moli-renderer-v8/src/runtime/page_vm/tests/rendering_update.rs

@@ -1,5 +1,6 @@
 use super::*;
 
+mod image_dimensions;
 mod transform_precision;
 
 use base64::Engine as _;

moli-renderer-v8/src/runtime/page_vm/tests/rendering_update/image_dimensions.rs

@@ -0,0 +1,65 @@
+use super::*;
+
+#[tokio::test(flavor = "current_thread")]
+async fn image_dimensions_use_rendered_content_size_without_forcing_a_refresh() {
+    run_page_vm_async_test(async move {
+        let loader =
+            crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader");
+        let mut page_vm = test_page_vm_with_loader_and_document_url(
+            &loader,
+            Vec::new(),
+            Url::parse("https://example.com/image-dimensions.html")?,
+        );
+        page_vm.vm_mut().eval(include_str!(
+            "../../../../../tests/fixtures/image-layout-dimensions-setup.js"
+        ))?;
+        page_vm.vm_mut().sync_live_document_style_sources();
+        let viewport = moli_layout::LayoutViewport::new(200, 600, 1.0);
+        page_vm
+            .vm_mut()
+            .screenshot_layout_snapshot(viewport)?
+            .expect("image dimensions layout");
+        let source = include_str!("../../../../../tests/fixtures/image-layout-dimensions.js");
+        let result = page_vm
+            .vm_mut()
+            .eval(&format!("JSON.stringify({source})"))?;
+        assert_eq!(
+            serde_json::from_str::<serde_json::Value>(&result)?,
+            serde_json::json!({
+                "css": [40,30,0,0], "override": [40,30,0,0], "edges": [40,30,0,0],
+                "borderbox": [30,20,0,0], "transformed": [40,30,0,0],
+                "zoomed": [40,30,0,0], "vertical": [40,30,0,0],
+                "fractional": [41,31,0,0], "hidden": [33,22,0,0]
+            }),
+            "shared fixture must match Chromium's content-box image dimensions"
+        );
+        let passes = page_vm.vm().layout_pass_observability_for_test().1;
+        assert_eq!(
+            page_vm.vm_mut().eval(
+                r#"
+const image = document.getElementById('css');
+image.style.width = '70px'; image.style.height = '50px';
+image.width = 120; image.height = 90;
+[image.width,image.height].join('|')
+"#
+            )?,
+            "40|30",
+            "synchronous getters must retain the published snapshot after mutation"
+        );
+        assert_eq!(page_vm.vm().layout_pass_observability_for_test().1, passes);
+        page_vm
+            .vm_mut()
+            .screenshot_layout_snapshot(viewport)?
+            .expect("refreshed image dimensions layout");
+        assert_eq!(
+            page_vm
+                .vm_mut()
+                .eval("[image.width,image.height].join('|')")?,
+            "70|50",
+            "a rendering checkpoint publishes the new CSS size, not the HTML attributes"
+        );
+        Ok::<_, anyhow::Error>(())
+    })
+    .await
+    .expect("image dimension geometry fixture should run");
+}

moli-renderer-v8/src/script_vm/tests/browser_api/images.rs

@@ -6,6 +6,54 @@ use tokio::{
 
 const ONE_BY_ONE_GIF: &[u8] = b"GIF89a\x01\0\x01\0\x80\0\0\0\0\0\xff\xff\xff!\xf9\x04\x01\0\0\0\0,\0\0\0\0\x01\0\x01\0\0\x02\x02D\x01\0;";
 
+#[tokio::test]
+async fn image_dimensions_without_layout_preserve_zero_and_parse_html_integer_prefixes() {
+    let loader = ResourceRequestClient::new(&moli_fetch::FetchConfig::default()).expect("loader");
+    loader.set_image_fetch_enabled(true);
+    let mut vm = new_storage_page_task_executor_test_vm_with_loader(
+        "https://image-dimensions.test/page.html",
+        &loader,
+    );
+    vm.set_fetch_subresource_interception(true, Some(crate::types::SubresourceResourceType::Image));
+    vm.eval("globalThis.dimensionImage = new Image(); dimensionImage.src = 'image.gif'")
+        .expect("detached image request");
+    let pending = vm.take_pending_subresource_fetch_infos();
+    assert_eq!(pending.len(), 1);
+    vm.fulfill_pending_subresource_fetch(
+        pending[0].internal_id,
+        200,
+        vec![("Content-Type".into(), "image/gif".into())],
+        one_by_one_gif_response_body(),
+    )
+    .expect("decoded image response");
+    run_next_image_event_task(&mut vm, &loader, "image dimension decode").await;
+    let source = include_str!("../../../../tests/fixtures/image-attribute-dimensions.js");
+    let result = vm
+        .eval(&format!("JSON.stringify({source})"))
+        .expect("attribute fixture");
+    assert_eq!(
+        serde_json::from_str::<serde_json::Value>(&result).expect("attribute JSON"),
+        serde_json::json!([
+            [1, 1],
+            [0, 0],
+            [0, 0],
+            [0, 0],
+            [0, 0],
+            [1, 1],
+            [7, 7],
+            [8, 8],
+            [1, 1],
+            [1, 1],
+            [1, 1],
+            [2, 2],
+            [4294967295_u32, 4294967295_u32],
+            [1, 1],
+            [1, 1]
+        ]),
+        "unrendered image dimensions must match the shared Chromium fixture"
+    );
+}
+
 fn one_by_one_gif_response_body() -> crate::runtime::RendererSyntheticResponseBody {
     crate::runtime::RendererSyntheticResponseBody::from_bytes(ONE_BY_ONE_GIF.to_vec())
 }

moli-renderer-v8/tests/fixtures/image-attribute-dimensions.js

@@ -0,0 +1,13 @@
+(() => {
+  const image = globalThis.dimensionImage;
+  if (image.naturalWidth !== 1 || image.naturalHeight !== 1) throw new Error('decoded 1x1 image required');
+  const values = [null, '0', '0junk', '-0', '-00px', '-1', '+7px', ' \t\n8tail',
+    '\v8', '\u00a08', '1.9', '2e2', '4294967295', '4294967296', '999999999999'];
+  return values.map(value => {
+    for (const name of ['width','height']) {
+      if (value === null) image.removeAttribute(name);
+      else image.setAttribute(name,value);
+    }
+    return [image.width,image.height];
+  });
+})()

moli-renderer-v8/tests/fixtures/image-layout-dimensions-setup.js

@@ -0,0 +1,19 @@
+(() => {
+  document.head.innerHTML = `<style>
+html,body{margin:0;padding:0}
+img{display:block;width:40px;height:30px}
+#edges,#borderbox{padding:2px;border:3px solid red}
+#borderbox{box-sizing:border-box}
+#transformed{transform:scale(2)}
+#zoomed{zoom:2}
+#vertical{writing-mode:vertical-rl}
+#fractional{width:40.5px;height:30.5px}
+#hidden{display:none}
+</style>`;
+  document.body.innerHTML = `
+<img id=css><img id=override width=90 height=80>
+<img id=edges><img id=borderbox><img id=transformed>
+<img id=zoomed><img id=vertical><img id=fractional>
+<img id=hidden width=33 height=22>`;
+  return 'installed';
+})()

moli-renderer-v8/tests/fixtures/image-layout-dimensions.js

@@ -0,0 +1,3 @@
+(() => Object.fromEntries([...document.images].map(image => [
+  image.id, [image.width, image.height, image.naturalWidth, image.naturalHeight]
+])))()