feat: add local file selector for face images

[oshimaryo]

Closes #5

Adds local file selector functionality to allow users to upload and persist local face images.

Changes

• Add radio buttons to choose between URL or file upload
• Implement file input with image validation and preview
• Add base64 encoding for local image storage
• Maintain existing URL functionality
• Add Japanese labels for better UX
• Store imageType setting for persistence

Features

• ✅ Local file selection with image validation
• ✅ Real-time image preview
• ✅ Persistent storage using base64 encoding
• ✅ Bilingual UI support
• ✅ Backward compatibility with existing URL functionality

🤖 Generated with Claude Code

[claude]

Claude finished @oshimaryo's task —— View job

---

PR Review Complete ✅

Review Checklist:

• Read and analyze all changed files
• Assess code quality and best practices
• Identify potential bugs or issues
• Evaluate performance considerations
• Check for security concerns
• Review test coverage and suggest improvements
• Provide comprehensive feedback

📋 Review Summary

This PR successfully adds local file upload functionality with good backward compatibility. The implementation includes nice UX touches like image previews and bilingual labels. However, there are several areas that need attention for production readiness.

🐛 Critical Issues

1. Bug in insertImage function (popup/settings.js:36)

document.getElementsByClassName('option--image').appendChild(img);

Should be:

document.getElementsByClassName('option--image')[0].appendChild(img);

getElementsByClassName returns a collection, not a single element.

2. Missing error handling for FileReader
The handleFileSelect function doesn't handle FileReader errors, which could cause silent failures.

⚠️ Security Concerns

1. XSS Risk with Base64 Data
Base64 data is directly set as img.src without validation. While browser security typically prevents XSS through data URLs, consider adding additional validation.

2. Insufficient File Validation

• Only checks file.type.startsWith('image/') but doesn't validate actual file content
• No file size limits could enable DoS attacks or exhaust storage quota
• Malicious files could be disguised with image MIME types

Recommendations:

• Add file size limits (suggest 1-2MB max)
• Implement proper image validation using canvas/Image objects
• Add content-type validation beyond just MIME type checking

🚀 Performance Issues

1. Base64 Storage Inefficiency
Storing large images as base64 in browser storage is inefficient (33% size overhead). Consider:

• Adding image compression before storage
• Using IndexedDB for larger binary data
• Implementing size warnings for users

2. No Image Optimization
Large images are stored as-is without compression or resizing, which could impact:

• Storage quota usage
• Memory consumption
• Extension performance

📝 Code Quality Improvements

1. Code Duplication
The applySettings function has nearly identical Chrome/Firefox branches. Consider extracting common logic:

const applySettingsData = (results) => {
  // Common logic here
};

const applySettings = () => {
  if(window['is_chrome']) {
    browser.storage.local.get(null, applySettingsData);
  } else {
    browser.storage.local.get(null).then(applySettingsData);
  }
};

2. Inconsistent Language
The UI mixes English and Japanese (ファイル: label). Consider:

• Using consistent language throughout
• Adding proper i18n support if targeting multiple languages

3. Error Handling
Replace alert() with more user-friendly error handling:

const showError = (message) => {
  // Show error in UI instead of alert
  const errorDiv = document.createElement('div');
  errorDiv.className = 'error-message';
  errorDiv.textContent = message;
  // Add to appropriate container
};

🧪 Testing Recommendations

No tests exist for the new functionality. Consider adding:

• Unit tests for file validation logic
• Integration tests for storage operations
• E2E tests for user workflows
• Edge case testing (invalid files, large files, etc.)

🔧 Minor Issues

1. Unused Function (popup/settings.js:33-37)
The insertImage function appears unused and contains the bug mentioned above.

2. Console Logging (popup/settings.js:46, 53)
Remove debug console.log statements in production code.

3. Missing Null Checks
Several places where DOM elements are accessed without null checks.

✅ Positive Aspects

• ✅ Good backward compatibility with existing URL functionality
• ✅ Nice UX with image previews and radio button interface
• ✅ Proper base64 encoding implementation
• ✅ Consistent coding style with existing codebase
• ✅ Proper event handling and storage persistence

📊 Overall Assessment

This is a solid implementation that adds valuable functionality. The main concerns are around security validation, performance optimization, and the critical bug in the insertImage function. With the suggested fixes, this would be ready for production.

Recommendation: Request changes to address the critical bug and security concerns before merging.

---

[claude]

Claude finished @oshimaryo's task —— View job

---

PR Review Complete ✅

Review Checklist:

• Read and analyze all changed files
• Assess code quality and best practices
• Identify potential bugs or issues
• Evaluate performance considerations
• Check for security concerns
• Review test coverage and suggest improvements
• Provide comprehensive feedback

📋 Review Summary

This PR successfully adds local file upload functionality with good backward compatibility. The implementation includes nice UX touches like image previews and bilingual labels. However, there are several areas that need attention for production readiness.

🐛 Critical Issues

1. Bug in insertImage function (popup/settings.js:36)

document.getElementsByClassName('option--image').appendChild(img);

Should be:

document.getElementsByClassName('option--image')[0].appendChild(img);

getElementsByClassName returns a collection, not a single element.

2. Missing error handling for FileReader
The handleFileSelect function doesn't handle FileReader errors, which could cause silent failures.

⚠️ Security Concerns

1. XSS Risk with Base64 Data
Base64 data is directly set as img.src without validation. While browser security typically prevents XSS through data URLs, consider adding additional validation.

2. Insufficient File Validation

• Only checks file.type.startsWith('image/') but doesn't validate actual file content
• No file size limits could enable DoS attacks or exhaust storage quota
• Malicious files could be disguised with image MIME types

Recommendations:

• Add file size limits (suggest 1-2MB max)
• Implement proper image validation using canvas/Image objects
• Add content-type validation beyond just MIME type checking

🚀 Performance Issues

1. Base64 Storage Inefficiency
Storing large images as base64 in browser storage is inefficient (33% size overhead). Consider:

• Adding image compression before storage
• Using IndexedDB for larger binary data
• Implementing size warnings for users

2. No Image Optimization
Large images are stored as-is without compression or resizing, which could impact:

• Storage quota usage
• Memory consumption
• Extension performance

📝 Code Quality Improvements

1. Code Duplication
The applySettings function has nearly identical Chrome/Firefox branches. Consider extracting common logic:

const applySettingsData = (results) => {
  // Common logic here
};

const applySettings = () => {
  if(window['is_chrome']) {
    browser.storage.local.get(null, applySettingsData);
  } else {
    browser.storage.local.get(null).then(applySettingsData);
  }
};

2. Inconsistent Language
The UI mixes English and Japanese (ファイル: label). Consider:

• Using consistent language throughout
• Adding proper i18n support if targeting multiple languages

3. Error Handling
Replace alert() with more user-friendly error handling:

const showError = (message) => {
  // Show error in UI instead of alert
  const errorDiv = document.createElement('div');
  errorDiv.className = 'error-message';
  errorDiv.textContent = message;
  // Add to appropriate container
};

🧪 Testing Recommendations

No tests exist for the new functionality. Consider adding:

• Unit tests for file validation logic
• Integration tests for storage operations
• E2E tests for user workflows
• Edge case testing (invalid files, large files, etc.)

🔧 Minor Issues

1. Unused Function (popup/settings.js:33-37)
The insertImage function appears unused and contains the bug mentioned above.

2. Console Logging (popup/settings.js:46, 53)
Remove debug console.log statements in production code.

3. Missing Null Checks
Several places where DOM elements are accessed without null checks.

✅ Positive Aspects

• ✅ Good backward compatibility with existing URL functionality
• ✅ Nice UX with image previews and radio button interface
• ✅ Proper base64 encoding implementation
• ✅ Consistent coding style with existing codebase
• ✅ Proper event handling and storage persistence

📊 Overall Assessment

This is a solid implementation that adds valuable functionality. The main concerns are around security validation, performance optimization, and the critical bug in the insertImage function. With the suggested fixes, this would be ready for production.

Recommendation: Request changes to address the critical bug and security concerns before merging.

---

[claude]

Claude finished @oshimaryo's task —— View job

---

PR Review Complete ✅

Review Checklist:

• Read and analyze all changed files
• Assess code quality and best practices
• Identify potential bugs or issues
• Evaluate performance considerations
• Check for security concerns
• Review test coverage and suggest improvements
• Provide comprehensive feedback

📋 Review Summary

This PR successfully adds local file upload functionality for face images, with good backward compatibility and user experience improvements. The implementation includes proper file validation, error handling, and loading states. However, there are some areas that could be improved for production readiness.

🐛 Issues Found

1. Unused Function with Bug (popup/settings.js:33-37)

const insertImage = (src) => {
  const img = document.createElement('img');
  img.src = src;
  document.getElementsByClassName('option--image').appendChild(img); // ❌ Bug here
}

The insertImage function appears unused and has a bug - getElementsByClassName returns a collection, not a single element. Should be [0] if needed.

2. Debug Code Left in Production (popup/settings.js:46, 53, 172)
Console.log statements should be removed:

console.log(e); // Line 46, 53
console.log(results); // Line 172

⚠️ Security Considerations

1. File Type Validation
Current validation only checks MIME type:

if (!file.type.startsWith('image/')) {

Consider adding content-based validation for additional security, as MIME types can be spoofed.

2. Base64 Data Handling
Base64 data is directly used as image source without additional validation. While browsers typically prevent XSS through data URLs, consider adding validation for extra security.

🚀 Performance Concerns

1. Storage Efficiency

• Base64 encoding increases file size by ~33%
• 500KB file limit is reasonable for browser storage limits
• Consider image compression before storage for better efficiency

2. Memory Usage
Large images stored as base64 strings could impact memory usage. Consider:

• Showing file size warnings to users
• Implementing image resizing for very large images

📝 Code Quality Assessment

Positive Aspects:

• ✅ Excellent error handling with FileReader error callbacks
• ✅ Good file size validation (500KB limit)
• ✅ Proper loading states and user feedback
• ✅ Comprehensive null checks for DOM elements
• ✅ Good backward compatibility with existing URL functionality
• ✅ Clean separation of concerns with dedicated functions
• ✅ Cross-browser compatibility maintained

Areas for Improvement:

• Code duplication in applySettings has been addressed well with the processResults function
• Good internationalization with English labels
• Proper error recovery by clearing file input on errors

🧪 Testing Coverage

Current Status: No automated tests for the new functionality.

Recommendations:

• Unit tests for file validation logic
• Integration tests for storage operations
• Edge case testing (invalid files, large files, FileReader errors)
• Cross-browser testing for file handling

📊 Overall Assessment

This is a well-implemented feature that adds valuable functionality to the extension. The code follows good practices with proper error handling, file validation, and user experience considerations. The main issues are minor (unused function, debug statements) and easily addressable.

Key Strengths:

• Robust error handling throughout the file upload process
• Good user experience with previews and loading states
• Sensible file size limits to prevent storage issues
• Maintains backward compatibility
• Follows established coding patterns in the codebase

Minor Improvements Needed:

• Remove unused insertImage function or fix the bug
• Clean up debug console.log statements
• Consider additional file content validation

Recommendation: ✅ Approve with minor fixes

The implementation is solid and production-ready with the minor cleanup items addressed. The feature adds significant value to the extension while maintaining code quality standards.

---

[claude]

Claude finished @oshimaryo's task —— View job

---

PR Review Complete ✅

Review Checklist:

• Read and analyze all changed files
• Assess code quality and best practices
• Identify potential bugs or issues
• Evaluate performance considerations
• Check for security concerns
• Review test coverage and suggest improvements
• Provide comprehensive feedback

📋 Review Summary

This PR successfully adds local file upload functionality for face images, with good backward compatibility and user experience improvements. The implementation includes proper file validation, error handling, and loading states. However, there are some areas that could be improved for production readiness.

🐛 Issues Found

1. Unused Function with Bug (popup/settings.js:33-37)

const insertImage = (src) => {
  const img = document.createElement('img');
  img.src = src;
  document.getElementsByClassName('option--image').appendChild(img); // ❌ Bug here
}

The insertImage function appears unused and has a bug - getElementsByClassName returns a collection, not a single element. Should be [0] if needed.

2. Debug Code Left in Production (popup/settings.js:46, 53, 172)
Console.log statements should be removed:

console.log(e); // Line 46, 53
console.log(results); // Line 172

⚠️ Security Considerations

1. File Type Validation
Current validation only checks MIME type:

if (!file.type.startsWith('image/')) {

Consider adding content-based validation for additional security, as MIME types can be spoofed.

2. Base64 Data Handling
Base64 data is directly used as image source without additional validation. While browsers typically prevent XSS through data URLs, consider adding validation for extra security.

🚀 Performance Concerns

1. Storage Efficiency

• Base64 encoding increases file size by ~33%
• 500KB file limit is reasonable for browser storage limits
• Consider image compression before storage for better efficiency

2. Memory Usage
Large images stored as base64 strings could impact memory usage. Consider:

• Showing file size warnings to users
• Implementing image resizing for very large images

📝 Code Quality Assessment

Positive Aspects:

• ✅ Excellent error handling with FileReader error callbacks
• ✅ Good file size validation (500KB limit)
• ✅ Proper loading states and user feedback
• ✅ Comprehensive null checks for DOM elements
• ✅ Good backward compatibility with existing URL functionality
• ✅ Clean separation of concerns with dedicated functions
• ✅ Cross-browser compatibility maintained

Areas for Improvement:

• Code duplication in applySettings has been addressed well with the processResults function
• Good internationalization with English labels
• Proper error recovery by clearing file input on errors

🧪 Testing Coverage

Current Status: No automated tests for the new functionality.

Recommendations:

• Unit tests for file validation logic
• Integration tests for storage operations
• Edge case testing (invalid files, large files, FileReader errors)
• Cross-browser testing for file handling

📊 Overall Assessment

This is a well-implemented feature that adds valuable functionality to the extension. The code follows good practices with proper error handling, file validation, and user experience considerations. The main issues are minor (unused function, debug statements) and easily addressable.

Key Strengths:

• Robust error handling throughout the file upload process
• Good user experience with previews and loading states
• Sensible file size limits to prevent storage issues
• Maintains backward compatibility
• Follows established coding patterns in the codebase

Minor Improvements Needed:

• Remove unused insertImage function or fix the bug
• Clean up debug console.log statements
• Consider additional file content validation

Recommendation: ✅ Approve with minor fixes

The implementation is solid and production-ready with the minor cleanup items addressed. The feature adds significant value to the extension while maintaining code quality standards.

---

View job