feat: bound decoded screenshot pixels (Closes #837)

2026-08-14 16:35:21 +00:00 · 2026-08-14 16:35:21 +00:00 · a476170fca
commit a476170fca
parent 5a579362ff
3 changed files with 102 additions and 16 deletions
--- a/frontend/issue-attachment.js
+++ b/frontend/issue-attachment.js
@ -8,6 +8,7 @@
  'use strict';

  const MAX_BYTES = 2 * 1024 * 1024;
+  const MAX_PIXELS = issueEvidenceReview.MAX_PIXELS;
  const MAX_FILES = 5;
  const MAX_FILES_MESSAGE = 'Up to 5 screenshots. Remove one before adding another.';
  const IMAGE_TYPES = new Set(['image/png', 'image/jpeg', 'image/webp']);
@ -38,6 +39,7 @@
  function create(options) {
    const upload = options.upload;
    const maxFiles = options.maxFiles === MAX_FILES ? MAX_FILES : 1;
+    const inspectPixels = options.inspectPixels === true;
    const optimizeSelectedImage = options.optimizeImage || optimizeImage;
    const createOperationId = options.createOperationId || (() => {
      if (typeof globalThis !== 'undefined' && globalThis.crypto?.randomUUID) {
@ -72,7 +74,7 @@
      if (!Number.isFinite(file.size) || file.size <= 0) {
        throw new Error('Choose a screenshot that is 2 MB or smaller.');
      }
-      if (file.size > MAX_BYTES) {
+      if (file.size > MAX_BYTES || inspectPixels) {
        return Promise.resolve(optimizeSelectedImage(file)).then(optimized => {
          if (generation !== selectionGeneration) return state();
          return commitSelection(optimized);
@ -219,7 +221,8 @@

  function mount(options) {
    const controller = create({
-      ...options, maxFiles:options.maxFiles || (options.input?.multiple ? MAX_FILES : 1),
+      ...options, inspectPixels:true,
+      maxFiles:options.maxFiles || (options.input?.multiple ? MAX_FILES : 1),
    });
    const clearSelection = controller.clear;
    const restoreSelection = controller.restore;
@ -376,5 +379,5 @@
    return Object.assign(controller, { clear: clearPreview, restore: restorePreview, setBusy });
  }

-  return { create, mount, multipart, optimizeImage, MAX_BYTES };
+  return { create, mount, multipart, optimizeImage, MAX_BYTES, MAX_PIXELS };
 });
--- a/frontend/issue-evidence-review.js
+++ b/frontend/issue-evidence-review.js
@ -6,6 +6,8 @@
  'use strict';

  const MAX_BYTES = 2 * 1024 * 1024;
+  const MAX_PIXELS = 12 * 1024 * 1024;
+  const MAX_DIMENSION = 8192;

  function namedBlob(blob, name) {
    if (typeof File === 'function') return new File([blob], name, { type: blob.type });
@ -14,20 +16,28 @@
  }

  async function optimizeImage(file, environment = {}) {
-    if (file.size <= MAX_BYTES) return file;
    const decode = environment.createImageBitmap || globalThis.createImageBitmap;
    const makeCanvas = environment.createCanvas || (() => document.createElement('canvas'));
-    if (typeof decode !== 'function') throw new Error('This browser cannot optimize the screenshot. Try cropping it and choose it again.');
+    if (typeof decode !== 'function') throw new Error('This browser cannot safely inspect the screenshot. Try cropping it and choose it again.');
    let bitmap;
    try {
      bitmap = await decode(file);
+      const width = Number(bitmap.width);
+      const height = Number(bitmap.height);
+      if (!Number.isSafeInteger(width) || !Number.isSafeInteger(height) || width <= 0 || height <= 0) {
+        throw new Error('decode');
+      }
+      const pixelScale = Math.min(1, Math.sqrt(MAX_PIXELS / (width * height)));
+      const dimensionScale = Math.min(1, MAX_DIMENSION / Math.max(width, height));
+      const byteScale = file.size > MAX_BYTES ? Math.sqrt(MAX_BYTES / file.size) * 0.92 : 1;
+      let scale = Math.min(pixelScale, dimensionScale, byteScale);
+      if (scale === 1 && file.size <= MAX_BYTES) return file;
      const canvas = makeCanvas();
      const context = canvas && canvas.getContext && canvas.getContext('2d');
-      if (!context || !bitmap.width || !bitmap.height) throw new Error('decode');
-      let scale = Math.min(1, Math.sqrt(MAX_BYTES / file.size) * 0.92);
+      if (!context) throw new Error('decode');
      for (let attempt = 0; attempt < 10; attempt += 1) {
-        canvas.width = Math.max(1, Math.round(bitmap.width * scale));
-        canvas.height = Math.max(1, Math.round(bitmap.height * scale));
+        canvas.width = Math.max(1, Math.floor(width * scale));
+        canvas.height = Math.max(1, Math.floor(height * scale));
        context.drawImage(bitmap, 0, 0, canvas.width, canvas.height);
        const blob = await new Promise(resolve => canvas.toBlob(resolve, file.type, file.type === 'image/png' ? undefined : 0.86));
        if (!blob) throw new Error('encode');
@ -172,5 +182,5 @@
    return { activeIndex: () => activeIndex, afterRemoval, clear, render, setBusy };
  }

-  return { create, optimizeImage };
+  return { create, optimizeImage, MAX_PIXELS };
 });
--- a/tests/test_issue_attachment_ui.py
+++ b/tests/test_issue_attachment_ui.py
@ -280,6 +280,74 @@ const canvas = {{
    assert output["name"] == "camera.jpg"


+def test_browser_optimizer_downscales_small_file_when_decoded_pixels_exceed_budget():
+    script = f"""
+const attachment = require({json.dumps(str(ATTACHMENT))});
+const original = new Blob(['compressed'], {{type:'image/jpeg'}});
+original.name = 'panorama.jpg';
+let closed = false;
+const attempts = [];
+const canvas = {{
+  width: 0, height: 0,
+  getContext: () => ({{drawImage: () => {{}}}}),
+  toBlob: callback => {{
+    attempts.push([canvas.width, canvas.height]);
+    callback(new Blob(['bounded'], {{type:'image/jpeg'}}));
+  }},
+}};
+(async()=>{{
+  const result = await attachment.optimizeImage(original, {{
+    createImageBitmap: async () => ({{width:6000,height:4000,close:()=>{{closed=true;}}}}),
+    createCanvas: () => canvas,
+  }});
+  process.stdout.write(JSON.stringify({{
+    attempts,closed,size:result.size,type:result.type,name:result.name,
+    maxPixels:attachment.MAX_PIXELS,
+  }}));
+}})().catch(error=>{{console.error(error);process.exit(1);}});
+"""
+    output = json.loads(run_node(script))
+
+    assert len(output["attempts"]) == 1
+    width, height = output["attempts"][0]
+    assert width * height <= output["maxPixels"]
+    assert width < 6000
+    assert height < 4000
+    assert output["closed"] is True
+    assert output["size"] == 7
+    assert output["type"] == "image/jpeg"
+    assert output["name"] == "panorama.jpg"
+
+
+def test_mobile_preview_inspects_small_screenshot_before_rendering_it():
+    script = f"""
+const attachment = require({json.dumps(str(ATTACHMENT))});
+class Element{{constructor(){{this.listeners={{}};this.hidden=true;this.value='';this.files=[];this.textContent='';this.src='';this.disabled=false;}}addEventListener(t,f){{this.listeners[t]=f;}}dispatch(t){{return this.listeners[t]({{target:this}});}}}}
+const input=new Element(),preview=new Element(),image=new Element(),meta=new Element(),remove=new Element(),status=new Element();
+const original=new Blob(['compressed'],{{type:'image/jpeg'}}); original.name='panorama.jpg';
+const bounded=new Blob(['bounded'],{{type:'image/jpeg'}}); bounded.name='panorama.jpg';
+const inspected=[];
+attachment.mount({{
+  input,preview,image,meta,remove,status,
+  optimizeImage:async file=>{{inspected.push(file.name);return bounded;}},
+  createObjectURL:blob=>blob===bounded?'blob:bounded':'blob:unsafe',revokeObjectURL:()=>{{}},upload:async()=>{{}},
+}});
+input.files=[original];
+(async()=>{{
+  await input.dispatch('change');
+  process.stdout.write(JSON.stringify({{inspected,src:image.src,hidden:preview.hidden,status:status.textContent}}));
+}})().catch(error=>{{console.error(error);process.exit(1);}});
+"""
+    output = json.loads(run_node(script))
+
+    assert output == {
+        "inspected": ["panorama.jpg"],
+        "src": "blob:bounded",
+        "hidden": False,
+        "status": "Screenshot optimized and ready to upload.",
+    }
+
+
 def test_binary_screenshot_builds_multipart_body_with_original_bytes():
    script = f"""
 const attachment=require({json.dumps(str(ATTACHMENT))});
@ -348,6 +416,7 @@ input.multiple=true;
 const urls=[];
 const controller=attachment.mount({{input,preview,image,meta,remove,status,tray,earlier,later,document,
  removedMessage:'Screenshot removed. Your issue draft is unchanged.',
+  optimizeImage:async file=>file,
  createObjectURL:blob=>{{const url='blob:'+blob.name;urls.push(url);return url;}},revokeObjectURL:()=>{{}},upload:async()=>{{}}}});
 const file=name=>{{const blob=new Blob([name],{{type:'image/png'}});blob.name=name;return blob;}};
 input.files=['one.png','two.png','three.png'].map(file);
@ -393,6 +462,7 @@ const document={{createElement:tag=>new Element(tag)}};
 const input=new Element('input'),preview=new Element(),image=new Element('img'),meta=new Element(),remove=new Element('button'),status=new Element(),tray=new Element(),earlier=new Element('button'),later=new Element('button'),note=new Element('textarea'),noteLabel=new Element('label');
 input.multiple=true;
 const controller=attachment.mount({{input,preview,image,meta,remove,status,tray,earlier,later,note,noteLabel,document,
+  optimizeImage:async file=>file,
  createObjectURL:blob=>'blob:'+blob.name,revokeObjectURL:()=>{{}},upload:async()=>{{}}}});
 const file=name=>{{const blob=new Blob([name],{{type:'image/png'}});blob.name=name;return blob;}};
 input.files=['one.png','two.png','three.png'].map(file);
@ -529,15 +599,18 @@ const input=new Element(), preview=new Element(), image=new Element(), meta=new
 const revoked=[];
 const controller=attachment.mount({{
  input,preview,image,meta,remove,status,
+  optimizeImage:async file=>file,
  createObjectURL:()=> 'blob:preview', revokeObjectURL:url=>revoked.push(url),
  readDataUrl:async()=>'', upload:async()=>{{}},
 }});
-input.files=[{{name:'payload.svg',type:'image/svg+xml',size:20}}]; input.dispatch('change');
-const invalid={{message:status.textContent,hidden:preview.hidden}};
-input.files=[{{name:'screen.png',type:'image/png',size:2048}}]; input.dispatch('change');
-const selected={{src:image.src,meta:meta.textContent,hidden:preview.hidden,state:controller.state()}};
-remove.dispatch('click');
-process.stdout.write(JSON.stringify({{invalid,selected,removed:{{hidden:preview.hidden,src:image.src,revoked,state:controller.state()}}}}));
+;(async()=>{{
+  input.files=[{{name:'payload.svg',type:'image/svg+xml',size:20}}]; await input.dispatch('change');
+  const invalid={{message:status.textContent,hidden:preview.hidden}};
+  input.files=[{{name:'screen.png',type:'image/png',size:2048}}]; await input.dispatch('change');
+  const selected={{src:image.src,meta:meta.textContent,hidden:preview.hidden,state:controller.state()}};
+  await remove.dispatch('click');
+  process.stdout.write(JSON.stringify({{invalid,selected,removed:{{hidden:preview.hidden,src:image.src,revoked,state:controller.state()}}}}));
+}})().catch(error=>{{console.error(error);process.exit(1);}});
 """
    output = json.loads(run_node(script))
    assert output["invalid"] == {