fix(download): PNG 背景の透明状態が原因の barcode reader decode 失敗を白塗りで解消 - #458
Conversation
`svgContentToPngBlob` / `downloadPngFromSvgElement` は Canvas2D の default 透明背景の まま `drawImage(svg)` していたため、bwip-js / JsBarcode の SVG (背景 rect なし) を経由 した PNG は quiet zone / バー間 pixel が α=0 transparent になっていた。image-based barcode reader (Dynamsoft 等) は transparent pixel を「黒」と解釈する実装があり、 quiet zone が黒判定 → 全面ノイズ → decode 不能になっていた (実例: dev server 生成 PNG → Dynamsoft 0 件、同 PNG を screenshot 経由 → confidence=100 で decode 成功)。 Canvas に `fillStyle = 'white'` + `fillRect(0, 0, canvas.width, canvas.height)` を `ctx.scale()` 前に呼んで device px 単位で全面白塗りすることで、生成 PNG の quiet zone を実 RGB 白として記録する。GS1 DataBar と JAN コードの両 PNG 出力が恩恵を受ける。 陽性対照: unit 6 件 (fillStyle='white', fillRect 引数, call 順序) + E2E 1 件 (生成 PNG 4 隅 pixel の α=255 / RGB=255 を `getImageData` で実観測)。fix を revert すると α=0 (transparent) に戻り全件 fail する設計。 経緯詳細: docs/decisions.md [082] Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
🖼️ Visual Regression Test 結果
|
レビュー: #458 PNG 背景の透明状態が原因の barcode reader decode 失敗を白塗りで解消多角的に確認しました。LGTM, 軽微な改善提案あり (blocker なし) です。 1. 概要
2. ロジック正しさ ✅
3. セキュリティ ✅
4. テスト陽性対照 ✅
5. フロントエンドデザイン観点 ✅ (軽微な懸念 1 件)
6. 改善提案 (任意, 本 PR 内でも別 PR でも)
7. その他確認したこと
結論: 修正は局所的・対称的・陽性対照あり・実機検証済みで安全に merge 可能。A〜D は nice-to-have、E のみ運用ルール上 follow-up 推奨です。 🤖 Generated with Claude Code |
… 実 download 経路 / D: コメント圧縮) #458 のレビュー指摘 4 件 (A〜D) を本 PR 内で吸収する: - **A** (mock 重複集約): `download.test.ts` の canvas / Image / URL / anchor の stubGlobal 構築を `setupBrowserMocks({ output: 'toBlob' | 'toDataURL' })` ヘルパーに集約。観測対象 (callLog / capturedFillStyle / capturedCtx / imgInstance / anchors) を `MockHandles` 経由で取得。536 → ~390 行に圧縮、各 it() の stub セットアップが 1 行に。 - **B** (magic number): `[0, 0, 200, 100]` 等の生数値を `[0, 0, 100 * RETINA_SCALE, 50 * RETINA_SCALE]` に置換。test ファイル上部に `const RETINA_SCALE = 2` を `download.ts` private 定数の mirror として宣言。将来の RETINA_SCALE 変更時の link が明示的に。 - **C** (実 download 経路に置換): E2E `生成 PNG の quiet zone が透明ではなく白` を `page.evaluate` 内 `svgContentToPngBlob` 再実装 → `page.waitForEvent('download')` で 実 PNG 取得 → Node fs で読み込み → base64 化して browser に渡し `<img>` で decode する 経路に置換。本実装 `svgContentToPngBlob` を bypass する false negative リスクを除去。 - **D** (コメント圧縮): `download.ts:56-65` の 12 行コメントを「Canvas2D default は transparent → 詳細 decisions.md [082]」の 3 行に圧縮。decisions.md と重複していた WHY 説明を一元化。`downloadPngFromSvgElement` 側 (115-118) は元々短いため不変。 陽性対照は全件保持。fix を revert すると `fillStyle` undefined / `fillRect` 不在 / quiet zone α=0 で全件 fail する設計は維持される (test-gates 鉄則 1)。 Closes #459 (review A) Closes #460 (review C) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
レビューありがとうございます。指摘 A〜D を本 PR 内で対応しました ( 対応内容A (mock 集約) → #459 を close
B (magic number)
C (実 download 経路) → #460 を closeE2E
これで 実機テスト: 単独実行で 1.0s pass を確認。 D (コメント圧縮)
E (CSP regression) → #461 で tracking 継続
テスト結果
🤖 Generated with Claude Code |
追加レビュー: 指摘対応 (
|
| # | 内容 | 対応 | 評価 |
|---|---|---|---|
| A | mock 重複集約 | `setupBrowserMocks({ output: 'toBlob' | 'toDataURL', throwsOnInvoke? }) ヘルパーに集約。MockedCtx/AnchorStub/ImgInstance/MockHandlesを interface 化、観測 API はgetCapturedFillStyle()/getImgInstance()/getCreatedAnchors()` の getter で closure 安定化 |
| B | magic number | test ファイル上部に const RETINA_SCALE = 2 を download.ts private const の mirror として宣言し、[0, 0, 100 * RETINA_SCALE, 50 * RETINA_SCALE] に置換 |
✅ 完璧。コメントで mirror であることも明示されていて読み手が prod 定数との link を辿りやすい |
| C | 実 download 経路への置換 | page.waitForEvent('download') → download.path() → Node readFile → base64 → browser に転送 → <img> decode → getImageData で 4 隅 sampling、という 教科書通りの Playwright PNG 検証 pattern に置換。コメントで「旧版は再実装 false negative リスク → 実 production code 経由に置換」と意図が明示済 |
✅ 期待以上。これで svgContentToPngBlob を bypass した regression を確実に検知できる (issue #460 Closed) |
| D | コメント圧縮 | download.ts:56-65 の 12 行 → 3 行 (「Canvas2D default は transparent → 詳細 decisions.md [082]」)。重複していた WHY 説明が SoT (decisions.md) に一元化 |
✅ 完璧。両関数のコメント密度も揃った |
| E | CSP regression tracking | issue #461 fix(csp): preview build で多数の inline style が CSP style-src 違反を起こし E2E gate が機能していない (bug, P1, security, test ラベル付き) で起票済 |
✅ 完璧。CLAUDE.md 6.4 (先送り → issue 化) に準拠 |
追加で確認したこと
- CI 全 green:
e2e (3m11s) / test (1m0s) / visual-regression (1m26s)すべて pass - CSP 整合:
src/utils/csp.ts:30でimg-src 'self' data: blob:となっており、新 E2E のimg.src = 'data:image/png;base64,...'は production CSP 下で動作可能 (E2E pass で実証) - button locator:
getByRole('button', { name: 'PNGダウンロード' })はDownloadButtonGroup.tsx:13のlabel="PNGダウンロード"と完全一致 - 陽性対照の revert 検知能力: 新 E2E は本物の
svgContentToPngBlobを経由するため、download.tsのfillStyle/fillRect行を削除すると α=0 に戻り pixel assertion で 4 隅 × 4 channel = 16 件 fail する設計が維持されている - テスト統合の副作用なし: 既存 7 件 (downloadBlob / svgContentToPngBlob 既存 / downloadPngFromSvgElement 既存) も
setupBrowserMocks経由に書き換えられているが、観測項目 (anchor click / imageSmoothingEnabled / Promise reject 経路) は全て保持されている
nit (任意, 本 PR では不要)
setupBrowserMocksのcanvasEl: Record<string, unknown>キャストはtoBlob/toDataURLの出し分けで一時的に loose な型になっている箇所。将来 discriminated union 化したくなったら issue refactor(test): download.test.ts の canvas mock 重複を makeCanvasStub() ヘルパーに集約 #459 のフォローで対応すれば十分 (本 PR ではSetupOptions側で discriminate しているため呼び出し側からは型安全)
結論: 指摘 4 件 (A〜D) は本 PR で吸収、残 1 件 (E) は適切に issue 化済み。レビュー対応として申し分ない品質で、approve・merge 可です。
🤖 Generated with Claude Code
* feat(gs1-databar): 合成シンボル上部に AI テキスト ((17)... 等) 表示を復活 PR #450 (commit c563cf5) は「ディセンダーが composite 上端 1X quiet zone に侵入し Dynamsoft Reader decode 不能」「textRowH を 3X gap に広げる alternative も decode 不能で撤回」を根拠に `injectCompositeText` を撤去していた。だが PR #458 (commit 977f6b6) で真因は **Canvas2D の透明背景** (`svgContentToPngBlob` が `fillStyle = 'white'` 未設定で α=0) と判明、PR #450 の検証はすべて透明背景時だったため descender 仮説は red herring の可能性が高いと user が指摘。 実機検証: PR #458 の白背景 fix を merge した状態で `injectCompositeText` を旧 geometry (textRowH=24、text baseline y=21、barcode translate y=24) でそのまま復活 → user 環境 の Dynamsoft online reader で decode 成功確認。descender 仮説否定。 - `escapeHtml` / `injectCompositeText` を `src/utils/gs1-databar.ts` に復元 (旧 geometry そのまま、コメントブロックで復活経緯を明示) - `src/components/tools/Gs1Databar.tsx` の wiring 復元 (compositeText 非空時のみ `injectCompositeText(sizedSvg, compositeText)` を `addSvgDimensions` 後に挿入) - `.gs1-svg-container` クラスを `src/styles/global.css` に復元 (`<text>` の `fill="currentColor"` 継承用、preview wrapper に `barcode-preview` と併用) 陽性対照 (旧コードに当てて必ず fail する設計): - unit: `escapeHtml` 6 件 (XSS escape) + `injectCompositeText` 8 件 (geometry / text 配置 y=21 / viewBox 高さ +24 / barcode translate / 横拡張 centering / XSS escape / inner content 保持) — 計 14 件 - E2E: `composite シンボルに AI テキスト ((17)... (10)...) が SVG 上部に注入される` (preview SVG の <text> 要素 + 内容 + y=21 / text-anchor / fontSize / fill 観測) + `non-composite (AI フィールド未入力) シンボルには AI テキスト注入されない` (常時 injection regression 検知) — 計 2 件 経緯詳細: docs/decisions.md [083] Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(gs1-databar): #462 review A 対応 (injectCompositeText の width/height 置換 regex を <svg> ルートに anchor) 旧 `/width="\d+"/` / `/height="\d+"/` regex は SVG 内の最初の match を置換するため、 将来 bwip-js / addSvgDimensions の変更で svg root に width/height 属性が無くなった 場合に子要素 (`<rect width="N">` 等) を wrong match して破壊する silent regression リスクがあった。`/(<svg\s[^>]*?)width="\d+"/` に anchor することで svg 開始タグ内の 属性のみを確実に置換する。 現在の coding path (addSvgDimensions が必ず width/height を注入) では顕在化しないが、 将来のリファクタ時の防御として導入。 陽性対照テスト: `<svg>` ルートに width 属性無し + 子要素 `<rect width="10">` ありの fixture で injectCompositeText を呼び、子要素の `<rect width="10" height="20">` が そのまま残ることを assert。anchor を外すと最初の match が `<rect width="10">` に なり `<rect width="124">` (newW=124) に誤置換されて assert fail する設計。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Summary
Found 0 barcodesになる事象を修正svgContentToPngBlob/downloadPngFromSvgElementの Canvas2D にfillStyle = 'white' + fillRectをctx.scale()前に追加するのみ。SVG / preview / VRT への影響なしdocs/decisions.md [082]に記録 (誤仮説 [垂直 quiet zone 不足] → 実機 pixel 計測で真因確定の過程)影響範囲
svgContentToPngBlob経由) の PNG ダウンロードdownloadPngFromSvgElement経由) の PNG ダウンロードquiet zone を透明 → 白のみの修正)陽性対照テスト
src/utils/__tests__/download.test.tssvgContentToPngBlob経路 3 件 /downloadPngFromSvgElement経路 3 件fillStyle === 'white',fillRect(0, 0, canvas.width, canvas.height), call 順序 (fillRect → scale → drawImage) を観測tests/e2e/gs1-databar.spec.tsα=255 / RGB=(255,255,255)であることをgetImageDataで assert仮説差し替え経緯 (debugging note)
paddingheight: 10を試行Found 0→ 仮説外れα=0, RGB=(0,0,0)発見Found 1 barcode (confidence: 100)確認Test plan
Found 1 barcode, confidence: 100, GS1_COMPOSITE)🤖 Generated with Claude Code