feat: comprehensive extension improvements and new features

[schmug]

This major update brings significant enhancements to functionality, UX, and code quality:

New Features

• RSS feed caching with configurable duration (1-60 min, default 5 min)
• Quick action buttons for Feedly, Inoreader, and NewsBlur
• Settings page for customizing videos shown and cache duration
• Keyboard shortcut support (Ctrl+Shift+R / Cmd+Shift+R)
• Update notifications on extension install/update
• Playlist and YouTube Music page detection with helpful messages

Improvements

• Retry mechanism with MutationObserver for dynamic content loading
• Better URL validation using URL constructor (XSS protection)
• Improved error messages with more context
• Enhanced accessibility (ARIA labels, alt text, keyboard nav)
• Fixed CSP violation (removed inline styles)

Developer Experience

• Added automated tests with Jest
• Code formatting standards (.editorconfig, .prettierrc, ESLint)
• Package.json with npm scripts for testing and linting
• Comprehensive test suite for utility functions
• Updated documentation (README, CLAUDE.md)

Architecture Changes

• Added background service worker for notifications
• Manifest updated with storage permission and keyboard commands
• Improved project structure with options page
• Better separation of concerns

Documentation

• Updated README with all new features
• Fixed architecture diagrams (removed outdated background.js references)
• Enhanced CLAUDE.md with detailed implementation notes
• Updated .gitignore for dev dependencies

All changes maintain security best practices and backward compatibility.

[claude]

Pull Request Review - PR #17

I've completed a comprehensive review of this PR. This is an impressive update that brings significant enhancements to the extension. Below is my detailed feedback:

---

✅ Strengths

Security

• Excellent XSS prevention: The escapeHtml() and escapeAttr() functions are properly implemented and used throughout
• URL validation: Good use of URL() constructor to validate URLs and protocol checks (popup.js:270-276)
• CSP compliance: Proper removal of inline styles and use of CSS classes
• Safe DOM manipulation: Using textContent and programmatic element creation instead of innerHTML in critical areas

Code Quality

• Clear separation of concerns: Well-organized code with dedicated files for different responsibilities
• Good error handling: Multiple error states with helpful user messages (PLAYLIST, YOUTUBE_MUSIC, etc.)
• Retry mechanism: Smart use of MutationObserver for dynamic content (content.js:32-59)
• Settings validation: Proper input validation in options.js (lines 31-39)

Features

• Cache implementation: Well-designed cache system with configurable expiry
• Quick actions: Nice UX improvement for popular RSS readers
• Accessibility: ARIA labels, alt text, and keyboard navigation support
• User experience: Loading states, error messages, and visual feedback

---

🔍 Issues & Recommendations

1. CRITICAL: HTML Escaping Bug 🔴

Location: popup.js:5-10

The escapeHtml() function doesn't actually escape anything. Setting textContent and immediately reading it back doesn't perform any escaping. This is a security vulnerability.

Current code:

function escapeHtml(str) {
  if (str == null) return '';
  const div = document.createElement('div');
  div.textContent = String(str);
  return div.textContent;  // This just returns the same string!
}

Fix needed:

function escapeHtml(str) {
  if (str == null) return '';
  return String(str)
    .replace(/&/g, '&')
    .replace(/</g, '&lt;')
    .replace(/>/g, '&gt;')
    .replace(/"/g, '&quot;')
    .replace(/'/g, '&#x27;');
}

Note: The same bug exists in __tests__/utils.test.js:67-72.

---

2. Async IIFE Without Error Handling 🟡

Location: content.js:2

The async IIFE has no error handling, which could lead to unhandled promise rejections.

Fix: Add try-catch wrapper around the main logic.

---

3. Test Coverage Gaps 🟡

Issues:

• Tests only cover isolated validation logic, not actual utility functions
• No tests for cache functionality or RSS parsing
• Tests duplicate code instead of importing from actual files
• The escapeHtml test has the same bug as the implementation

Recommendations:

• Extract utility functions to a separate utils.js file
• Import and test actual functions
• Add tests for getCachedFeed(), setCachedFeed()
• Consider integration tests for critical flows

---

4. Race Condition in Cache 🟡

Location: popup.js:48-63

If two popup instances open simultaneously (possible with keyboard shortcut), both might fetch and write to cache.

Impact: Low, but worth considering a locking mechanism.

---

5. Hard-coded Timeout Values 🟢

Location: popup.js:413, content.js:47

Consider extracting to constants:

const RSS_FETCH_TIMEOUT_MS = 10000;
const DOM_WAIT_TIMEOUT_MS = 3000;

---

📊 Performance Considerations

Good Practices:
✅ Caching reduces redundant RSS fetches
✅ Async/await used properly
✅ AbortController for fetch timeout
✅ Programmatic script injection only when needed

Potential Optimizations:

• Cache growth: No cleanup strategy for old cached feeds in chrome.storage.local
• RSS parsing: Parses all entries before slicing; consider limiting during parse

---

🧪 Test Coverage

Current: ~15% (estimated)

• ✅ Basic validation logic
• ❌ Cache functionality
• ❌ RSS parsing
• ❌ Integration/E2E tests

---

🔐 Security Assessment

Overall: Good practices with one critical issue.

✅ Strengths: URL validation, CSP compliance, minimal permissions
🔴 Critical: escapeHtml() doesn't work as intended

---

✅ Final Verdict

High-quality PR with excellent improvements. Code is well-organized and demonstrates good understanding of Chrome extension development.

Recommended Action: Approve with required changes

Must Fix Before Merge:

1. Fix escapeHtml() function in popup.js and __tests__/utils.test.js

Should Consider:
2. Add error handling to async IIFE in content.js
3. Improve test coverage
4. Add cache cleanup mechanism

Nice to Have:
5. Extract magic numbers to constants
6. Add JSDoc comments
7. Extract utility functions to separate module

---

Great work overall! The extension has evolved significantly with this PR. Just fix that escaping function and you're good to go! 🚀