From 95a14c6de5729b0cf21f9b0325c36a8215a20ac6 Mon Sep 17 00:00:00 2001 From: timmy Date: Mon, 10 Aug 2026 15:56:35 +0000 Subject: [PATCH] feat: optimize oversized mobile screenshots (Closes #495) --- README.md | 2 +- frontend/issue-attachment.js | 119 ++++++++++++++++++---- tests/test_issue_attachment_ui.py | 161 ++++++++++++++++++++++++++++++ 3 files changed, 264 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 51af481..94666a5 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ threads, create and self-assign issues, discover, claim, and release issue assig list repository labels and open milestones, set or clear due dates on assigned issues, create issue comments, close assigned issues, inspect/comment on assigned pull requests, merge assigned pull requests, and submit pull-request reviews. -Assigned-issue comments can include one PNG, JPEG, or WebP screenshot up to 2 MB. +Assigned-issue comments can include one PNG, JPEG, or WebP screenshot; the mobile composer automatically optimizes oversized screenshots on-device to fit the 2 MB upload boundary while leaving already-valid files unchanged. For online delivery, the screenshot uploads before the comment is posted; validation or upload failures keep both the typed comment and removable preview available for retry. Offline screenshot comments admit their text and image bytes to IndexedDB before confirmation, keep only bounded metadata in diff --git a/frontend/issue-attachment.js b/frontend/issue-attachment.js index 388ae1f..f97b2fb 100644 --- a/frontend/issue-attachment.js +++ b/frontend/issue-attachment.js @@ -8,6 +8,48 @@ const MAX_BYTES = 2 * 1024 * 1024; const IMAGE_TYPES = new Set(['image/png', 'image/jpeg', 'image/webp']); + function namedBlob(blob, name) { + if (typeof File === 'function') { + return new File([blob], name, { type: blob.type }); + } + Object.defineProperty(blob, 'name', { value: name, configurable: true }); + return blob; + } + + 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.'); + } + + let bitmap; + try { + bitmap = await decode(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); + 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)); + 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'); + if (blob.size > 0 && blob.size <= MAX_BYTES) return namedBlob(blob, file.name); + scale *= 0.8; + } + } catch (_error) { + throw new Error('The screenshot could not be optimized. Try cropping it and choose it again.'); + } finally { + if (bitmap && typeof bitmap.close === 'function') bitmap.close(); + } + throw new Error('The screenshot could not be optimized below 2 MB. Try cropping it and choose it again.'); + } + function multipart(attachment) { let blob = attachment?.blob; if (!blob && attachment?.data) { @@ -23,6 +65,7 @@ function create(options) { const upload = options.upload; + const optimizeSelectedImage = options.optimizeImage || optimizeImage; const createOperationId = options.createOperationId || (() => { if (typeof globalThis !== 'undefined' && globalThis.crypto?.randomUUID) { return globalThis.crypto.randomUUID(); @@ -33,13 +76,12 @@ let confirmed = null; let serialized = null; let operationId = null; + let selectionGeneration = 0; - function select(file) { - if (!file || !IMAGE_TYPES.has(file.type)) { - throw new Error('Choose a PNG, JPEG, or WebP screenshot.'); - } - if (!Number.isFinite(file.size) || file.size <= 0 || file.size > MAX_BYTES) { - throw new Error('Choose a screenshot that is 2 MB or smaller.'); + function commitSelection(file) { + if (!file || !IMAGE_TYPES.has(file.type) || !Number.isFinite(file.size) || + file.size <= 0 || file.size > MAX_BYTES) { + throw new Error('The screenshot could not be optimized below 2 MB. Try cropping it and choose it again.'); } selected = file; confirmed = null; @@ -48,7 +90,25 @@ return state(); } + function select(file) { + const generation = ++selectionGeneration; + if (!file || !IMAGE_TYPES.has(file.type)) { + throw new Error('Choose a PNG, JPEG, or WebP screenshot.'); + } + 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) { + return Promise.resolve(optimizeSelectedImage(file)).then(optimized => { + if (generation !== selectionGeneration) return state(); + return commitSelection(optimized); + }); + } + return commitSelection(file); + } + function clear() { + selectionGeneration += 1; selected = null; confirmed = null; serialized = null; @@ -119,31 +179,56 @@ const controller = create(options); const clearSelection = controller.clear; let previewUrl = ''; + let selectionSequence = 0; function clearPreview() { + selectionSequence += 1; if (previewUrl) options.revokeObjectURL(previewUrl); previewUrl = ''; options.image.src = ''; options.preview.hidden = true; options.input.value = ''; + options.input.disabled = false; clearSelection(); } - options.input.addEventListener('change', event => { - const file = event.target.files && event.target.files[0]; - try { - controller.select(file); - } catch (error) { - options.status.textContent = error.message; - options.input.value = ''; - return; - } + function showPreview(file, optimized) { if (previewUrl) options.revokeObjectURL(previewUrl); previewUrl = options.createObjectURL(file); options.image.src = previewUrl; options.meta.textContent = file.name + ' · ' + Math.ceil(file.size / 1024) + ' KB'; options.preview.hidden = false; - options.status.textContent = options.readyMessage || 'Screenshot ready to upload with this comment.'; + options.status.textContent = optimized ? 'Screenshot optimized and ready to upload.' : + (options.readyMessage || 'Screenshot ready to upload with this comment.'); + } + + options.input.addEventListener('change', event => { + const file = event.target.files && event.target.files[0]; + const sequence = ++selectionSequence; + let result; + try { + result = controller.select(file); + } catch (error) { + options.status.textContent = error.message; + options.input.value = ''; + return; + } + if (!result || typeof result.then !== 'function') { + showPreview(file, false); + return; + } + options.status.textContent = 'Optimizing screenshot…'; + options.input.disabled = true; + return result.then(() => controller.serialize()).then(value => { + if (sequence === selectionSequence) showPreview(value.blob, true); + }).catch(error => { + if (sequence === selectionSequence) { + options.status.textContent = error.message; + options.input.value = ''; + } + }).finally(() => { + if (sequence === selectionSequence) options.input.disabled = false; + }); }); options.remove.addEventListener('click', () => { clearPreview(); @@ -165,5 +250,5 @@ return Object.assign(controller, { clear: clearPreview, restore: restorePreview }); } - return { create, mount, multipart, MAX_BYTES }; + return { create, mount, multipart, optimizeImage, MAX_BYTES }; }); diff --git a/tests/test_issue_attachment_ui.py b/tests/test_issue_attachment_ui.py index 9e89811..b65fff9 100644 --- a/tests/test_issue_attachment_ui.py +++ b/tests/test_issue_attachment_ui.py @@ -112,6 +112,85 @@ controller.serialize().then(async value=>process.stdout.write(JSON.stringify({{ } +def test_oversized_screenshot_is_optimized_before_preview_and_durable_serialization(): + script = f""" +const attachment = require({json.dumps(str(ATTACHMENT))}); +const original = new Blob([new Uint8Array(attachment.MAX_BYTES + 200)], {{type:'image/png'}}); +original.name = 'phone.png'; +const calls = []; +const controller = attachment.create({{ + optimizeImage: async file => {{ + calls.push({{name:file.name,type:file.type,size:file.size}}); + const optimized = new Blob(['optimized-png'], {{type:'image/png'}}); + optimized.name = file.name; + return optimized; + }}, + upload: async () => {{ throw new Error('must not upload during selection'); }}, +}}); +(async()=>{{ + await controller.select(original); + const serialized = await controller.serialize(); + process.stdout.write(JSON.stringify({{ + calls,state:controller.state(),name:serialized.filename, + contentType:serialized.contentType,size:serialized.blob.size, + text:await serialized.blob.text(),sameBlob:serialized.blob===original, + }})); +}})().catch(error=>{{console.error(error);process.exit(1);}}); +""" + output = json.loads(run_node(script)) + + assert output == { + "calls": [{ + "name": "phone.png", + "type": "image/png", + "size": 2 * 1024 * 1024 + 200, + }], + "state": {"name": "phone.png", "size": 13, "uploaded": False}, + "name": "phone.png", + "contentType": "image/png", + "size": 13, + "text": "optimized-png", + "sameBlob": False, + } + + +def test_browser_optimizer_downscales_until_encoded_image_fits_limit(): + script = f""" +const attachment = require({json.dumps(str(ATTACHMENT))}); +const original = new Blob([new Uint8Array(attachment.MAX_BYTES + 1)], {{type:'image/jpeg'}}); +original.name = 'camera.jpg'; +const attempts = []; +let closed = false; +const canvas = {{ + width: 0, height: 0, + getContext: () => ({{drawImage: () => {{}}}}), + toBlob: callback => {{ + attempts.push([canvas.width, canvas.height]); + const size = attempts.length === 1 ? attachment.MAX_BYTES + 50 : attachment.MAX_BYTES - 50; + callback(new Blob([new Uint8Array(size)], {{type:'image/jpeg'}})); + }}, +}}; +(async()=>{{ + const result = await attachment.optimizeImage(original, {{ + createImageBitmap: async () => ({{width:2000,height:1000,close:()=>{{closed=true;}}}}), + createCanvas: () => canvas, + }}); + process.stdout.write(JSON.stringify({{ + attempts,closed,size:result.size,type:result.type,name:result.name, + }})); +}})().catch(error=>{{console.error(error);process.exit(1);}}); +""" + output = json.loads(run_node(script)) + + assert len(output["attempts"]) == 2 + assert output["attempts"][1][0] < output["attempts"][0][0] + assert output["attempts"][1][1] < output["attempts"][0][1] + assert output["closed"] is True + assert output["size"] == 2 * 1024 * 1024 - 50 + assert output["type"] == "image/jpeg" + assert output["name"] == "camera.jpg" + + def test_binary_screenshot_builds_multipart_body_with_original_bytes(): script = f""" const attachment=require({json.dumps(str(ATTACHMENT))}); @@ -246,6 +325,87 @@ process.stdout.write(JSON.stringify({{invalid,selected,removed:{{hidden:preview. } +def test_oversized_attachment_view_shows_progress_then_previews_optimized_blob(): + 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(type,fn) {{ this.listeners[type]=fn; }} + dispatch(type) {{ return this.listeners[type]({{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([new Uint8Array(attachment.MAX_BYTES+10)],{{type:'image/png'}}); original.name='large.png'; +const optimized=new Blob(['small'],{{type:'image/png'}}); optimized.name='large.png'; +let finish; +const gate=new Promise(resolve=>{{finish=resolve;}}); +const previewed=[]; +attachment.mount({{ + input,preview,image,meta,remove,status, + optimizeImage:async()=>{{await gate;return optimized;}}, + createObjectURL:blob=>{{previewed.push(blob===optimized);return 'blob:optimized';}}, + revokeObjectURL:()=>{{}},upload:async()=>{{}}, +}}); +input.files=[original]; +const pending=input.dispatch('change'); +const during={{message:status.textContent,disabled:input.disabled,hidden:preview.hidden}}; +finish(); +Promise.resolve(pending).then(()=>process.stdout.write(JSON.stringify({{ + during,after:{{message:status.textContent,disabled:input.disabled,hidden:preview.hidden, + src:image.src,meta:meta.textContent}},previewed +}}))).catch(error=>{{console.error(error);process.exit(1);}}); +""" + output = json.loads(run_node(script)) + + assert output["during"] == { + "message": "Optimizing screenshot…", "disabled": True, "hidden": True + } + assert output["after"]["message"] == "Screenshot optimized and ready to upload." + assert output["after"]["disabled"] is False + assert output["after"]["hidden"] is False + assert output["after"]["src"] == "blob:optimized" + assert "large.png" in output["after"]["meta"] + assert output["previewed"] == [True] + + +def test_removing_screenshot_during_optimization_cancels_stale_selection(): + 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(type,fn) {{ this.listeners[type]=fn; }} + dispatch(type) {{ return this.listeners[type]({{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([new Uint8Array(attachment.MAX_BYTES+10)],{{type:'image/webp'}}); original.name='large.webp'; +const optimized=new Blob(['small'],{{type:'image/webp'}}); optimized.name='large.webp'; +let finish; +const gate=new Promise(resolve=>{{finish=resolve;}}); +const controller=attachment.mount({{ + input,preview,image,meta,remove,status, + optimizeImage:async()=>{{await gate;return optimized;}}, + createObjectURL:()=> 'blob:stale',revokeObjectURL:()=>{{}},upload:async()=>{{}}, +}}); +input.files=[original]; +const pending=input.dispatch('change'); +remove.dispatch('click'); +finish(); +Promise.resolve(pending).then(async()=>process.stdout.write(JSON.stringify({{ + state:controller.state(),serialized:await controller.serialize(),disabled:input.disabled, + hidden:preview.hidden,src:image.src,status:status.textContent, +}}))).catch(error=>{{console.error(error);process.exit(1);}}); +""" + output = json.loads(run_node(script)) + + assert output == { + "state": None, + "serialized": None, + "disabled": False, + "hidden": True, + "src": "", + "status": "Screenshot removed. Your comment is unchanged.", + } + + def test_issue_comment_actions_upload_binary_multipart_before_posting_and_clear_after_acceptance(): source = DASHBOARD.read_text() @@ -276,6 +436,7 @@ def test_readme_documents_mobile_screenshot_limits_and_delivery_order(): readme = README.read_text() assert "PNG, JPEG, or WebP" in readme assert "2 MB" in readme + assert "automatically optimizes oversized screenshots" in readme assert "uploads before the comment is posted" in readme assert "New issue" in readme assert "retry resumes with the confirmed issue" in readme -- 2.43.0