Skip to content

Fix emoji loading inefficiency and 404 errors#70

Merged
zzstoatzz merged 2 commits intomainfrom
fix/emoji-loading-efficiency
Sep 24, 2025
Merged

Fix emoji loading inefficiency and 404 errors#70
zzstoatzz merged 2 commits intomainfrom
fix/emoji-loading-efficiency

Conversation

@zzstoatzz
Copy link
Owner

Summary

  • Eliminates 404 errors when loading custom emojis
  • Adds intelligent emoji resolver that maps names to correct filenames
  • Supports any file extension (.png, .gif, .jpg, .webp, etc.)
  • Handles URL-encoded filenames properly (e.g., se%C3%B1or-bufo.png)

Problem

The app was making unnecessary requests that resulted in 404s:

  1. Always trying .png first, then falling back to .gif on error
  2. Not knowing the correct file extension until after a failed request
  3. URL-encoded filenames in emoji picker causing additional 404s

Solution

  • Created emoji-resolver.js that loads the emoji mapping from /api/custom-emojis once
  • Uses placeholder images initially to avoid any 404s
  • Updates all emoji images with their correct filenames after the mapping loads
  • Handles emoji picker images asynchronously to prevent race conditions

Test Results

Before: Multiple 404 errors in console when loading pages with emojis
After: Zero 404 errors - all emojis load correctly on first try

🤖 Generated with Claude Code

zzstoatzz and others added 2 commits September 24, 2025 09:48
- Add emoji-resolver.js that loads filename mappings from API
- Replace hardcoded .png/.gif extensions with placeholder images
- Eliminate 404 errors by using correct extensions from the start
- Support any file extension (.png, .gif, .jpg, .webp, etc.)
- Maintain backward compatibility for existing data

The resolver loads once, caches the mappings, and updates all emoji
images with their correct filenames, preventing unnecessary 404s.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Use placeholder images in emoji picker initially
- Load actual images after a frame to prevent race conditions
- Properly handle URL-encoded filenames like se%C3%B1or-bufo.png
@claude
Copy link

claude bot commented Sep 24, 2025

Code Review for PR #70: Fix emoji loading inefficiency and 404 errors

✅ Strengths

  • Excellent performance optimization: Eliminates unnecessary 404 requests by using a proper mapping system instead of trial-and-error loading
  • Clean architecture: Well-structured module with clear separation of concerns and proper encapsulation
  • Smart use of placeholders: Using data URIs for initial loads prevents visual flashing while avoiding 404s
  • Handles dynamic content: MutationObserver ensures dynamically added emojis work correctly
  • Good error handling: Gracefully falls back when API fails or returns invalid data

🔍 Areas of Concern

1. Race Condition Risk 🟡

In templates/status.html:2257, using requestAnimationFrame to delay image loading might not guarantee the resolver is initialized:

requestAnimationFrame(() => {
    img.src = `/emojis/${emoji.filename}`;
});

Recommendation: Wait for the resolver to be ready:

if (window.EmojiResolver) {
    window.EmojiResolver.loadEmojiMap().then(() => {
        img.src = `/emojis/${emoji.filename}`;
    });
}

2. Missing Cache Invalidation 🟡

The emoji map is cached indefinitely. If emojis are added/removed, users need to refresh to see changes.
Recommendation: Add cache expiry or version-based invalidation to emoji-resolver.js

3. Security: No Input Sanitization 🔴

The emoji names from data-emoji-name attributes aren't sanitized before building URLs in emoji-resolver.js:45 and :52
While template escaping handles XSS in the HTML, consider adding validation:

const sanitizedName = emojiName.replace(/[^a-zA-Z0-9_-]/g, '');

4. Memory Leak Potential 🟡

The MutationObserver never disconnects, which could cause issues in single-page apps.
Recommendation: Expose a cleanup method:

window.EmojiResolver.cleanup = () => observer.disconnect();

📊 Performance Considerations

  • ✅ Single API call vs multiple 404s - huge win!
  • ✅ Efficient Map lookup (O(1)) for emoji resolution
  • ⚠️ Consider implementing lazy loading for emoji picker with many emojis
  • ⚠️ The observer processes all DOM mutations - consider debouncing for pages with heavy updates

🔒 Security Review

  • ✅ No direct eval or innerHTML usage
  • ✅ API endpoint validates file extensions server-side (src/api/status_read.rs:417)
  • ⚠️ Add input validation for emoji names as mentioned above
  • ⚠️ Consider adding CSP headers to restrict image sources

🧪 Test Coverage

Currently no test coverage for this feature. Recommend adding:

  • Unit tests for emoji-resolver.js functions
  • Integration test for the full emoji loading flow
  • Test for handling malformed API responses
  • Test for special characters/URL-encoded filenames

💡 Additional Suggestions

  1. Consider adding a loading indicator while the emoji map loads
  2. Add console logging levels (debug/info/error) for easier troubleshooting
  3. Document the expected API response format in comments
  4. Consider using WebP format with fallbacks for better performance

✨ Overall Assessment

This is a solid improvement that effectively solves the 404 issue. The code is well-structured and handles most edge cases. With the security fix for input sanitization and some minor improvements, this will be production-ready.

Verdict: Approve with minor changes

Great work on eliminating those pesky 404s! The placeholder approach is particularly clever. 🎉

@zzstoatzz zzstoatzz merged commit 05f8bd0 into main Sep 24, 2025
3 checks passed
@zzstoatzz zzstoatzz deleted the fix/emoji-loading-efficiency branch September 24, 2025 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant