From 0dd7ad815f0b05184a8b54bee6aba750a7b6ef20 Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Wed, 19 Nov 2025 20:45:23 +1100 Subject: [PATCH 01/13] LoadingView and Image Lazy Loading - Add an X button to Loading View to allow exiting out of V spinner view early - First attempt to add lazy loading to images - Attempt to add timeouts and in case of failure set "Dead Image" badge similar to Dead Tweets --- App/Misc/HTMLRenderingHelpers.swift | 40 +- App/Resources/RenderView.js | 734 +++++++++++++++--- .../Posts/PostsPageView.swift | 4 +- .../Posts/PostsPageViewController.swift | 33 +- App/Views/LoadingView.swift | 81 +- App/Views/RenderView.swift | 91 ++- .../Stylesheets/_dead-tweet-ghost.less | 42 +- 7 files changed, 862 insertions(+), 163 deletions(-) diff --git a/App/Misc/HTMLRenderingHelpers.swift b/App/Misc/HTMLRenderingHelpers.swift index 50f66aa98..e3339e33b 100644 --- a/App/Misc/HTMLRenderingHelpers.swift +++ b/App/Misc/HTMLRenderingHelpers.swift @@ -138,8 +138,12 @@ extension HTMLDocument { - Turns all non-smiley `` elements into `src` elements (if linkifyNonSmiles == true). - Adds .awful-smile to smilie elements. - Rewrites URLs for some external image hosts that have changed domains and/or URL schemes. + - Defers loading of post content images beyond the first 10 (lazy loading). */ func processImgTags(shouldLinkifyNonSmilies: Bool) { + var postContentImageCount = 0 + let initialLoadCount = 10 // First 10 post content images load immediately + for img in nodes(matchingSelector: "img") { guard let src = img["src"], @@ -150,10 +154,38 @@ extension HTMLDocument { if isSmilie { img.toggleClass("awful-smile") - } else if let postimageURL = fixPostimageURL(url) { - img["src"] = postimageURL.absoluteString - } else if let waffleURL = randomwaffleURLForWaffleimagesURL(url) { - img["src"] = waffleURL.absoluteString + } else { + // Check if this is an avatar (has class="avatar") + let isAvatar = img["class"]?.contains("avatar") ?? false + + // Skip attachment.php files (require auth, handled elsewhere) + let isAttachment = url.lastPathComponent == "attachment.php" + + if !isAvatar && !isAttachment { + // This is a post content image (not avatar, not smilie, not attachment) + postContentImageCount += 1 + + if postContentImageCount > initialLoadCount { + // Defer loading for images beyond the first 10 + img["data-lazy-src"] = src + img["src"] = "data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7" + } + } + + // Apply URL fixes + if let postimageURL = fixPostimageURL(url) { + if postContentImageCount <= initialLoadCount || isAvatar { + img["src"] = postimageURL.absoluteString + } else { + img["data-lazy-src"] = postimageURL.absoluteString + } + } else if let waffleURL = randomwaffleURLForWaffleimagesURL(url) { + if postContentImageCount <= initialLoadCount || isAvatar { + img["src"] = waffleURL.absoluteString + } else { + img["data-lazy-src"] = waffleURL.absoluteString + } + } } if shouldLinkifyNonSmilies, !isSmilie { diff --git a/App/Resources/RenderView.js b/App/Resources/RenderView.js index 24f7ffac2..cbd63c38f 100644 --- a/App/Resources/RenderView.js +++ b/App/Resources/RenderView.js @@ -34,6 +34,100 @@ Awful.fetchOEmbed = async function(url) { }); }; +/** + * Embeds tweets within a specific post element using Twitter's OEmbed API. + * Called by IntersectionObserver when a post enters the viewport. + * + * @param {Element} thisPostElement - The post element to process for tweet embeds + */ +Awful.embedTweetNow = function(thisPostElement) { + if (!thisPostElement.classList.contains("embed-processed")) { + thisPostElement.classList.add("embed-processed"); + + const enableGhost = (window.Awful.renderGhostTweets == true); + const tweetLinks = thisPostElement.querySelectorAll('a[data-tweet-id]'); + + if (tweetLinks.length == 0) { + return; + } + + // Group tweet links by ID for deduplication + const tweetIDsToLinks = {}; + Array.prototype.forEach.call(tweetLinks, function(a) { + // Skip tweets with NWS content + if (a.parentElement.querySelector('img.awful-smile[title=":nws:"]')) { + return; + } + const tweetID = a.dataset.tweetId; + if (!(tweetID in tweetIDsToLinks)) { + tweetIDsToLinks[tweetID] = []; + } + tweetIDsToLinks[tweetID].push(a); + }); + + // Fetch and embed each unique tweet + Object.keys(tweetIDsToLinks).forEach(function(tweetID) { + const callback = `jsonp_callback_${tweetID}`; + const tweetTheme = Awful.tweetTheme(); + + const script = document.createElement('script'); + script.src = `https://api.twitter.com/1/statuses/oembed.json?id=${tweetID}&omit_script=true&dnt=true&theme=${tweetTheme}&callback=${callback}`; + + window[callback] = function(data) { + cleanUp(script); + + // Replace all links for this tweet with the embedded HTML + tweetIDsToLinks[tweetID].forEach(function(a) { + if (a.parentNode) { + const div = document.createElement('div'); + div.classList.add('tweet'); + div.innerHTML = data.html; + a.parentNode.replaceChild(div, a); + } + }); + + // Load Twitter widgets to render the embedded tweets + if (window.twttr) { + twttr.widgets.load(); + } + }; + + script.onerror = function() { + cleanUp(this); + console.error(`The embed markup for tweet ${tweetID} failed to load`); + + // Replace failed tweets with ghost Lottie animation + if (enableGhost) { + tweetIDsToLinks[tweetID].forEach(function(a) { + if (a.parentNode) { + const div = document.createElement('div'); + div.classList.add('dead-tweet-container'); + div.innerHTML = Awful.deadTweetBadgeHTML(a.href.toString(), `${tweetID}`); + a.parentNode.replaceChild(div, a); + + const player = div.querySelectorAll("lottie-player"); + player.forEach((lottiePlayer) => { + lottiePlayer.addEventListener("rendered", () => { + lottiePlayer.load(document.getElementById("ghost-json-data").innerText); + }); + }); + } + }); + } + }; + + function cleanUp(script) { + delete window[callback]; + if (script.parentNode) { + script.parentNode.removeChild(script); + } + } + + document.body.appendChild(script); + }); + } +}; + /** Callback for fetchOEmbed. @@ -82,145 +176,510 @@ Awful.embedBlueskyPosts = function() { }; /** - Turns apparent links to tweets into actual embedded tweets. + * Initializes lazy-loading tweet embeds using IntersectionObserver. + * Tweets are embedded as posts enter the viewport (with a 600px lookahead). + * Also sets up Lottie animation play/pause for ghost tweets in the viewport. */ Awful.embedTweets = function() { Awful.loadTwitterWidgets(); const enableGhost = (window.Awful.renderGhostTweets == true); - // if ghost is enabled, add IntersectionObserver so that we know when to play and stop the animations + // Set up IntersectionObserver for ghost Lottie animations (play/pause on scroll) if (enableGhost) { - const topMarginOffset = 0; - - let config = { - root: document.body.posts, - rootMargin: `${topMarginOffset}px 0px`, - threshold: 0.000001, + const ghostConfig = { + root: document.body.posts, + rootMargin: '0px', + threshold: 0.000001, }; - - let observer = new IntersectionObserver(function (posts, observer) { - // each element will be checked by the browser as scolling occurs - posts.forEach((post, index) => { - if (post.isIntersecting) { - const player = post.target.querySelectorAll("lottie-player"); - player.forEach((lottiePlayer) => { + + const ghostObserver = new IntersectionObserver(function(posts) { + posts.forEach((post) => { + const players = post.target.querySelectorAll("lottie-player"); + players.forEach((lottiePlayer) => { + if (post.isIntersecting) { lottiePlayer.play(); - // comment out when not testing - //console.log("Lottie playing."); - }); - } else { - // pause all lottie players if this post is not intersecting - const player = post.target.querySelectorAll("lottie-player"); - player.forEach((lottiePlayer) => { - lottiePlayer.pause(); - // this log is to confirm that pausing actually occurs while scrolling. comment out when not testing - //console.log("Lottie paused."); - }); - } + } else { + lottiePlayer.pause(); + } + }); }); - }, config); - - const viewbox = document.querySelectorAll("post"); - viewbox.forEach((post) => { - observer.observe(post); + }, ghostConfig); + + const postElements = document.querySelectorAll("post"); + postElements.forEach((post) => { + ghostObserver.observe(post); }); - } - - var tweetLinks = document.querySelectorAll('a[data-tweet-id]'); - if (tweetLinks.length == 0) { - return; + + // Apply timeout detection to initial images (first 10) + Awful.applyTimeoutToLoadingImages(); + + // Setup lazy loading for deferred images (11+) + Awful.setupImageLazyLoading(); + + // Setup retry handler + Awful.setupRetryHandler(); } - var tweetIDsToLinks = {}; - Array.prototype.forEach.call(tweetLinks, function(a) { - if (a.parentElement.querySelector('img.awful-smile[title=":nws:"]')) { - return; - } - var tweetID = a.dataset.tweetId; - if (!(tweetID in tweetIDsToLinks)) { - tweetIDsToLinks[tweetID] = []; - } - tweetIDsToLinks[tweetID].push(a); + // Set up lazy-loading IntersectionObserver for tweet embeds + // 600px rootMargin means tweets are loaded ~600px before entering the viewport + const lazyLoadConfig = { + root: null, + rootMargin: '600px 0px', + threshold: 0.000001, + }; + + const lazyLoadObserver = new IntersectionObserver(function(entries) { + entries.forEach((entry) => { + if (entry.isIntersecting) { + Awful.embedTweetNow(entry.target); + } + }); + }, lazyLoadConfig); + + // Observe all post elements for lazy loading + const posts = document.querySelectorAll("post"); + posts.forEach((post) => { + lazyLoadObserver.observe(post); }); - var totalFetchCount = Object.keys(tweetIDsToLinks).length; - var completedFetchCount = 0; + // Notify native side when tweets are loaded + if (window.twttr) { + twttr.ready(function() { + if (webkit.messageHandlers.didFinishLoadingTweets) { + twttr.events.bind('loaded', function() { + webkit.messageHandlers.didFinishLoadingTweets.postMessage({}); + }); + } + }); + } +}; - Object.keys(tweetIDsToLinks).forEach(function(tweetID) { - var callback = `jsonp_callback_${tweetID}`; - var tweetTheme = Awful.tweetTheme(); +// Image load progress tracker +Awful.imageLoadTracker = { + loaded: 0, + total: 0, + + initialize: function(totalCount) { + this.loaded = 0; + this.total = totalCount; + this.reportProgress(); + }, + + incrementLoaded: function() { + this.loaded++; + this.reportProgress(); + }, + + reportProgress: function() { + if (window.webkit?.messageHandlers?.imageLoadProgress) { + webkit.messageHandlers.imageLoadProgress.postMessage({ + loaded: this.loaded, + total: this.total, + complete: this.loaded >= this.total + }); + } + } +}; - var script = document.createElement('script'); - script.src = `https://api.twitter.com/1/statuses/oembed.json?id=${tweetID}&omit_script=true&dnt=true&theme=${tweetTheme}&callback=${callback}`; +// Image loading with smart timeout detection +// Monitors download progress and only times out stalled connections +Awful.loadImageWithProgressDetection = async function(url, img, imageID, enableGhost) { + const initialTimeout = 1000; // 1s timeout if no bytes received + const stallTimeout = 2500; // 2.5s timeout if download stalls + const heartbeatInterval = 500; // Check progress every 500ms + + const controller = new AbortController(); + let lastProgressTime = Date.now(); + let totalBytes = 0; + let heartbeatTimer = null; + + // Heartbeat check: abort if no progress + heartbeatTimer = setInterval(() => { + const timeSinceProgress = Date.now() - lastProgressTime; + + if (totalBytes === 0 && timeSinceProgress > initialTimeout) { + // No bytes received at all - connection never started + console.warn(`Image timeout: no connection after ${initialTimeout}ms - ${url}`); + clearInterval(heartbeatTimer); + controller.abort(); + } else if (totalBytes > 0 && timeSinceProgress > stallTimeout) { + // Download started but stalled + console.warn(`Image stalled: no progress for ${stallTimeout}ms after ${totalBytes} bytes - ${url}`); + clearInterval(heartbeatTimer); + controller.abort(); + } + }, heartbeatInterval); - window[callback] = function(data) { - cleanUp(script); + try { + const response = await fetch(url, { signal: controller.signal }); - tweetIDsToLinks[tweetID].forEach(function(a) { - if (a.parentNode) { - var div = document.createElement('div'); - div.classList.add('tweet'); - div.innerHTML = data.html; - a.parentNode.replaceChild(div, a); + if (!response.ok) { + throw new Error(`HTTP ${response.status}`); } - }); - didCompleteFetch(); - }; + const reader = response.body.getReader(); + const chunks = []; - script.onerror = function() { - cleanUp(this); - console.error(`The embed markup for tweet ${tweetID} failed to load`); - - // when a tweet errors out, insert a floating ghost lottie in somber rememberence of the tweet that used to be - if (enableGhost) { - tweetIDsToLinks[tweetID].forEach(function(a) { - if (a.parentNode) { - var div = document.createElement('div'); - div.classList.add('dead-tweet-container'); - div.innerHTML = Awful.deadTweetBadgeHTML(a.href.toString(), `${tweetID}`); - a.parentNode.replaceChild(div, a); - - const player = div.querySelectorAll("lottie-player"); - player.forEach((lottiePlayer) => { - lottiePlayer.addEventListener("rendered", (e) => { - lottiePlayer.load(document.getElementById("ghost-json-data").innerText); - }); - }); - } - }); - } - - didCompleteFetch(); - }; + while (true) { + const { done, value } = await reader.read(); - function cleanUp(script) { - delete window[callback]; - if (script.parentNode) { - script.parentNode.removeChild(script); - } + if (done) break; + + // Bytes received! Update progress tracking + chunks.push(value); + totalBytes += value.length; + lastProgressTime = Date.now(); + } + + clearInterval(heartbeatTimer); + + // Success! Create blob and set image source + const blob = new Blob(chunks); + const objectURL = URL.createObjectURL(blob); + img.src = objectURL; + + // Clean up object URL after image loads + img.onload = () => { + URL.revokeObjectURL(objectURL); + Awful.imageLoadTracker.incrementLoaded(); + }; + + } catch (error) { + clearInterval(heartbeatTimer); + + // Replace image with dead image badge + if (enableGhost && img.parentNode) { + const div = document.createElement('div'); + div.classList.add('dead-embed-container'); + div.innerHTML = Awful.deadImageBadgeHTML(url, imageID); + img.parentNode.replaceChild(div, img); + + // Setup Lottie animation + const player = div.querySelector("lottie-player"); + if (player) { + player.addEventListener("rendered", () => { + const ghostData = document.getElementById("ghost-json-data"); + if (ghostData) { + player.load(ghostData.innerText); + } + }); + } + } + + console.error(`Image load failed: ${error.message} - ${url}`); + Awful.imageLoadTracker.incrementLoaded(); } +}; - document.body.appendChild(script); - }); +// Main function to apply timeout detection to all post images +Awful.loadImagesWithTimeout = function() { + const enableGhost = Awful.renderGhostTweets || false; + + // Only process images in post content, exclude avatars and smilies + const contentImages = document.querySelectorAll('section.postbody img:not(.awful-smile):not(.awful-avatar)'); - function didCompleteFetch() { - completedFetchCount += 1; + contentImages.forEach((img, index) => { + const imageID = `img-${Date.now()}-${index}`; - if (completedFetchCount == totalFetchCount) { - if (window.twttr) { - twttr.ready(function() { - twttr.widgets.load(); + // Skip if already loaded + if (img.complete && img.naturalHeight !== 0) { + return; + } + + // Clear the src to prevent default loading, we'll handle it with fetch + const originalSrc = img.src; + img.removeAttribute('src'); + + // Load with progress detection + Awful.loadImageWithProgressDetection(originalSrc, img, imageID, enableGhost); + }); + + // Setup retry click handler (using event delegation) + document.addEventListener('click', function(event) { + const retryLink = event.target; + if (retryLink.hasAttribute('data-retry-image')) { + event.preventDefault(); + + const imageURL = retryLink.getAttribute('data-retry-image'); + const imageID = retryLink.getAttribute('data-image-id'); + const container = retryLink.closest('.dead-embed-container'); + + if (container) { + const img = document.createElement('img'); + img.setAttribute('alt', ''); + container.parentNode.replaceChild(img, container); + + // Retry loading with timeout detection + Awful.loadImageWithProgressDetection(imageURL, img, imageID, enableGhost); + } + } + }, { once: false }); +}; + +// Lazy loads post content images using IntersectionObserver (for images 11+) +Awful.setupImageLazyLoading = function() { + const enableGhost = Awful.renderGhostTweets || false; + + // IntersectionObserver with 600px lookahead (same as tweets) + const imageObserver = new IntersectionObserver(function(entries) { + entries.forEach(entry => { + if (entry.isIntersecting) { + const img = entry.target; + const lazySrc = img.dataset.lazySrc; + + if (lazySrc) { + // Skip attachment.php files (defensive check, shouldn't be lazy-loaded) + if (lazySrc.includes('attachment.php')) { + delete img.dataset.lazySrc; + img.src = lazySrc; // Load normally without timeout + imageObserver.unobserve(img); + return; + } + + const imageID = `lazy-${Date.now()}-${Math.random().toString(36).substring(2, 11)}`; + delete img.dataset.lazySrc; + + // Load with smart timeout detection + Awful.loadImageWithProgressDetection(lazySrc, img, imageID, enableGhost); + + imageObserver.unobserve(img); + } + } }); + }, { + rootMargin: '600px 0px', // Load 600px before entering viewport + threshold: 0.01 + }); + + // Observe all lazy-loadable images + document.querySelectorAll('img[data-lazy-src]').forEach(img => { + imageObserver.observe(img); + }); +}; - if (webkit.messageHandlers.didFinishLoadingTweets) { - twttr.events.bind('loaded', function() { - webkit.messageHandlers.didFinishLoadingTweets.postMessage({}); - }); +// Apply timeout detection to images that are loading normally (first 10) +Awful.applyTimeoutToLoadingImages = function() { + const enableGhost = Awful.renderGhostTweets || false; + + // Find images with real src (not data-lazy-src) - these are the first 10 images + const loadingImages = document.querySelectorAll('section.postbody img[src]:not(.awful-smile):not(.awful-avatar):not([data-lazy-src])'); + + // Count only the initially loading images (first 10), excluding attachment.php and data URLs + const initialImages = Array.from(loadingImages).filter(img => + !img.src.includes('attachment.php') && !img.src.startsWith('data:') + ); + const totalImages = initialImages.length; + + // Initialize progress tracker (only tracks first 10 images, not lazy-loaded ones) + Awful.imageLoadTracker.initialize(totalImages); + + loadingImages.forEach((img, index) => { + const imageID = `img-${Date.now()}-${index}`; + const imageURL = img.src; + + // Skip data URLs (placeholders) + if (imageURL.startsWith('data:')) { + return; } - } - } - } + + // Skip attachment.php files (require auth, handled elsewhere) + if (imageURL.includes('attachment.php')) { + return; + } + + // Skip if already loaded (but count it) + if (img.complete && img.naturalHeight !== 0) { + Awful.imageLoadTracker.incrementLoaded(); + return; + } + + // Let browser load the image naturally, but monitor for timeout + // Track if we've already handled this image to prevent double-counting + let handled = false; + + const handleSuccess = () => { + if (handled) return; + handled = true; + Awful.imageLoadTracker.incrementLoaded(); + }; + + const handleFailure = () => { + if (handled) return; + handled = true; + + if (enableGhost && img.parentNode) { + const div = document.createElement('div'); + div.classList.add('dead-embed-container'); + div.innerHTML = Awful.deadImageBadgeHTML(imageURL, imageID); + img.parentNode.replaceChild(div, img); + + const player = div.querySelector("lottie-player"); + if (player) { + player.addEventListener("rendered", () => { + const ghostData = document.getElementById("ghost-json-data"); + if (ghostData) { + player.load(ghostData.innerText); + } + }); + } + } + + Awful.imageLoadTracker.incrementLoaded(); + }; + + // Set up timeout checker + let checkCount = 0; + const maxChecks = 3; // Check 3 times (at 1s, 2s, 3s) + const checkInterval = 1000; // Check every 1 second + + const timeoutChecker = setInterval(() => { + checkCount++; + + // If image loaded successfully + if (img.complete && img.naturalHeight !== 0) { + clearInterval(timeoutChecker); + handleSuccess(); + return; + } + + // If image failed to load (error state) + if (img.complete && img.naturalHeight === 0) { + clearInterval(timeoutChecker); + handleFailure('failed'); + return; + } + + // If we've checked enough times and it's still not loaded, timeout + if (checkCount >= maxChecks) { + clearInterval(timeoutChecker); + handleFailure(`timed out after ${checkCount * checkInterval}ms`); + } + }, checkInterval); + + // Also listen for load/error events to handle immediately + img.addEventListener('load', () => { + clearInterval(timeoutChecker); + handleSuccess(); + }, { once: true }); + + img.addEventListener('error', () => { + clearInterval(timeoutChecker); + handleFailure('error event'); + }, { once: true }); + }); +}; + +// Setup retry click handler (using event delegation) - call once on page load +Awful.setupRetryHandler = function() { + document.addEventListener('click', function(event) { + const retryLink = event.target; + if (retryLink.hasAttribute('data-retry-image')) { + event.preventDefault(); + + const imageURL = retryLink.getAttribute('data-retry-image'); + const container = retryLink.closest('.dead-embed-container'); + + if (container) { + // Update retry link to show "Retrying..." state + retryLink.textContent = 'Retrying...'; + retryLink.style.pointerEvents = 'none'; // Disable clicking during retry + + // Create hidden image element to test loading + const img = document.createElement('img'); + img.style.display = 'none'; + container.parentNode.insertBefore(img, container); + + // Custom retry with success/failure callbacks + const retryWithFeedback = async function() { + const initialTimeout = 1000; + const stallTimeout = 2500; + const heartbeatInterval = 500; + + const controller = new AbortController(); + let lastProgressTime = Date.now(); + let totalBytes = 0; + let heartbeatTimer = null; + + heartbeatTimer = setInterval(() => { + const timeSinceProgress = Date.now() - lastProgressTime; + + if (totalBytes === 0 && timeSinceProgress > initialTimeout) { + console.warn(`Retry failed: no connection after ${initialTimeout}ms - ${imageURL}`); + clearInterval(heartbeatTimer); + controller.abort(); + } else if (totalBytes > 0 && timeSinceProgress > stallTimeout) { + console.warn(`Retry failed: stalled after ${totalBytes} bytes - ${imageURL}`); + clearInterval(heartbeatTimer); + controller.abort(); + } + }, heartbeatInterval); + + try { + const response = await fetch(imageURL, { signal: controller.signal }); + + if (!response.ok) { + throw new Error(`HTTP ${response.status}`); + } + + const reader = response.body.getReader(); + const chunks = []; + + while (true) { + const { done, value } = await reader.read(); + if (done) break; + + chunks.push(value); + totalBytes += value.length; + lastProgressTime = Date.now(); + } + + clearInterval(heartbeatTimer); + + // SUCCESS! Create blob and replace badge with image + const blob = new Blob(chunks); + const objectURL = URL.createObjectURL(blob); + + const successImg = document.createElement('img'); + successImg.setAttribute('alt', ''); + successImg.src = objectURL; + + successImg.onload = () => { + URL.revokeObjectURL(objectURL); + // Replace the container with the successful image + container.parentNode.replaceChild(successImg, container); + // Remove the hidden test image + if (img.parentNode) { + img.parentNode.removeChild(img); + } + }; + + } catch (error) { + clearInterval(heartbeatTimer); + + // FAILED - restore retry button with "Failed" feedback + console.error(`Retry failed: ${error.message} - ${imageURL}`); + + retryLink.textContent = 'Retry Failed - Try Again'; + retryLink.style.pointerEvents = 'auto'; // Re-enable clicking + + // Remove the hidden test image + if (img.parentNode) { + img.parentNode.removeChild(img); + } + + // Reset to just "Retry" after 3 seconds + setTimeout(() => { + if (retryLink.textContent === 'Retry Failed - Try Again') { + retryLink.textContent = 'Retry'; + } + }, 3000); + } + }; + + retryWithFeedback(); + } + } + }, { once: false }); }; Awful.tweetTheme = function() { @@ -259,20 +718,6 @@ Awful.loadTwitterWidgets = function() { }; }; -/** - Loads the Lottie player library into the document - */ -Awful.loadLotties = function() { - if (document.getElementById('lottie-js')) { - return; - } - - var script = document.createElement('script'); - script.id = 'lottie-js'; - script.src = "awful-resource://lottie-player.js"; - document.body.appendChild(script); -}; - /** Scrolls the document past a fraction of the document. @@ -726,7 +1171,7 @@ Awful.setAnnouncementHTML = function(html) { Awful.deadTweetBadgeHTML = function(url, tweetID){ // get twitter username from url var tweeter = url.match(/(?:https?:\/\/)?(?:www\.)?twitter\.com\/(?:#!\/)?@?([^\/\?\s]*)/)[1]; - + var html = `
@@ -735,7 +1180,25 @@ Awful.deadTweetBadgeHTML = function(url, tweetID){ DEAD TWEET @${tweeter} `; - + + return html; +}; + +// Dead Image Badge (similar to dead tweet) +Awful.deadImageBadgeHTML = function(url, imageID) { + // Extract filename from URL + var filename = url.split('/').pop().split('?')[0] || 'unknown'; + + var html = + `
+ + +
+ DEAD IMAGE + ${filename} + Retry + `; + return html; }; @@ -953,6 +1416,25 @@ Awful.embedGfycat = function() { } Awful.embedGfycat(); + +// Set up image loading if DOM is ready (DOMContentLoaded may have already fired) +// The early user script in RenderView.swift tracks when DOMContentLoaded fires +if (Awful.domContentLoadedFired) { + if (typeof Awful.applyTimeoutToLoadingImages === 'function') { + Awful.applyTimeoutToLoadingImages(); + Awful.setupImageLazyLoading(); + Awful.setupRetryHandler(); + } +} else { + document.addEventListener('DOMContentLoaded', function() { + if (typeof Awful.applyTimeoutToLoadingImages === 'function') { + Awful.applyTimeoutToLoadingImages(); + Awful.setupImageLazyLoading(); + Awful.setupRetryHandler(); + } + }); +} + // THIS SHOULD STAY AT THE BOTTOM OF THE FILE! // All done; tell the native side we're ready. window.webkit.messageHandlers.didRender.postMessage({}); diff --git a/App/View Controllers/Posts/PostsPageView.swift b/App/View Controllers/Posts/PostsPageView.swift index 2e9519cb5..4d8c52f9f 100644 --- a/App/View Controllers/Posts/PostsPageView.swift +++ b/App/View Controllers/Posts/PostsPageView.swift @@ -23,8 +23,8 @@ final class PostsPageView: UIView { // MARK: Loading view - var loadingView: UIView? { - get { return loadingViewContainer.subviews.first } + var loadingView: LoadingView? { + get { return loadingViewContainer.subviews.first as? LoadingView } set { loadingViewContainer.subviews.forEach { $0.removeFromSuperview() } if let newValue = newValue { diff --git a/App/View Controllers/Posts/PostsPageViewController.swift b/App/View Controllers/Posts/PostsPageViewController.swift index 31e12e867..ce84cc4a1 100644 --- a/App/View Controllers/Posts/PostsPageViewController.swift +++ b/App/View Controllers/Posts/PostsPageViewController.swift @@ -105,6 +105,7 @@ final class PostsPageViewController: ViewController { postsView.renderView.registerMessage(RenderView.BuiltInMessage.DidTapPostActionButton.self) postsView.renderView.registerMessage(RenderView.BuiltInMessage.DidTapAuthorHeader.self) postsView.renderView.registerMessage(RenderView.BuiltInMessage.FetchOEmbedFragment.self) + postsView.renderView.registerMessage(RenderView.BuiltInMessage.ImageLoadProgress.self) postsView.topBar.goToParentForum = { [unowned self] in guard let forum = self.thread.forum else { return } AppDelegate.instance.open(route: .forum(id: forum.forumID)) @@ -254,7 +255,10 @@ final class PostsPageViewController: ViewController { let initialTheme = theme let fetch = Task { - try await ForumsClient.shared.listPosts(in: thread, writtenBy: author, page: newPage, updateLastReadPost: updateLastReadPost) + await MainActor.run { + self.postsView.loadingView?.updateStatus("Fetching posts from server...") + } + return try await ForumsClient.shared.listPosts(in: thread, writtenBy: author, page: newPage, updateLastReadPost: updateLastReadPost) } networkOperation = fetch Task { [weak self] in @@ -300,6 +304,7 @@ final class PostsPageViewController: ViewController { self.scrollToFractionAfterLoading = self.postsView.renderView.scrollView.fractionalContentOffset.y } + self.postsView.loadingView?.updateStatus("Generating page...") self.renderPosts() self.updateUserInterface() @@ -416,6 +421,9 @@ final class PostsPageViewController: ViewController { Task { await postsView.renderView.eraseDocument() + await MainActor.run { + self.postsView.loadingView?.updateStatus("Loading page...") + } self.postsView.renderView.render(html: html, baseURL: ForumsClient.shared.baseURL) } } @@ -662,7 +670,12 @@ final class PostsPageViewController: ViewController { private func showLoadingView() { guard postsView.loadingView == nil else { return } - postsView.loadingView = LoadingView.loadingViewWithTheme(theme) + let loadingView = LoadingView.loadingViewWithTheme(theme) + loadingView.updateStatus("Loading...") + loadingView.onDismiss = { [weak self] in + self?.clearLoadingMessage() + } + postsView.loadingView = loadingView } private func clearLoadingMessage() { @@ -1743,9 +1756,7 @@ extension PostsPageViewController: RenderViewDelegate { view.embedTweets() } - if frogAndGhostEnabled { - view.loadLottiePlayer() - } + // Note: Image loading tracking is set up automatically via DOMContentLoaded event in RenderView.js webViewDidLoadOnce = true @@ -1769,7 +1780,8 @@ extension PostsPageViewController: RenderViewDelegate { postsView.renderView.scrollToFractionalOffset(fractionalOffset) } - clearLoadingMessage() + // Note: Loading view is now dismissed when image loading completes (via ImageLoadProgress message) + // or when user taps "Show Now" button } func didReceive(message: RenderViewMessage, in view: RenderView) { @@ -1792,6 +1804,15 @@ extension PostsPageViewController: RenderViewDelegate { case let message as RenderView.BuiltInMessage.FetchOEmbedFragment: fetchOEmbed(url: message.url, id: message.id) + case let message as RenderView.BuiltInMessage.ImageLoadProgress: + let statusText = "Downloading images: \(message.loaded)/\(message.total)" + postsView.loadingView?.updateStatus(statusText) + + // Dismiss loading view when all images are done + if message.complete { + clearLoadingMessage() + } + case is FYADFlagRequest: fetchNewFlag() diff --git a/App/Views/LoadingView.swift b/App/Views/LoadingView.swift index f8b597543..1d05db42f 100644 --- a/App/Views/LoadingView.swift +++ b/App/Views/LoadingView.swift @@ -36,11 +36,19 @@ class LoadingView: UIView { return DefaultLoadingView(theme: theme) } } - + + /// Callback invoked when user requests early dismissal + var onDismiss: (() -> Void)? + + /// Update status text (override in subclasses to implement) + func updateStatus(_ text: String) { + // Override in subclasses + } + fileprivate func retheme() { // nop } - + override func willMove(toSuperview newSuperview: UIView?) { guard let newSuperview = newSuperview else { return } frame = CGRect(origin: .zero, size: newSuperview.bounds.size) @@ -52,27 +60,64 @@ class LoadingView: UIView { private class DefaultLoadingView: LoadingView { private let animationView: LottieAnimationView - + private let statusLabel: UILabel + private let showNowButton: UIButton + override init(theme: Theme?) { animationView = LottieAnimationView( animation: LottieAnimation.named("mainthrobber60"), configuration: LottieConfiguration(renderingEngine: .mainThread)) + statusLabel = UILabel() + showNowButton = UIButton(type: .system) + super.init(theme: theme) + // Setup animation view animationView.currentFrame = 0 animationView.contentMode = .scaleAspectFit animationView.animationSpeed = 1 animationView.isOpaque = true animationView.translatesAutoresizingMaskIntoConstraints = false - addSubview(animationView) - - animationView.widthAnchor.constraint(equalToConstant: 90).isActive = true - animationView.heightAnchor.constraint(equalToConstant: 90).isActive = true - animationView.centerXAnchor.constraint(equalTo: self.centerXAnchor).isActive = true - animationView.centerYAnchor.constraint(equalTo: self.centerYAnchor).isActive = true - + + // Setup status label + statusLabel.text = "Loading..." + statusLabel.font = .preferredFont(forTextStyle: .subheadline) + statusLabel.textAlignment = .center + statusLabel.translatesAutoresizingMaskIntoConstraints = false + addSubview(statusLabel) + + // Setup Show Now button as X in circle icon + let xCircleImage = UIImage(systemName: "xmark.circle.fill") + showNowButton.setImage(xCircleImage, for: .normal) + showNowButton.addTarget(self, action: #selector(showNowTapped), for: .touchUpInside) + showNowButton.translatesAutoresizingMaskIntoConstraints = false + showNowButton.contentHorizontalAlignment = .fill + showNowButton.contentVerticalAlignment = .fill + addSubview(showNowButton) + + // Layout constraints + NSLayoutConstraint.activate([ + // Animation centered, shifted up + animationView.widthAnchor.constraint(equalToConstant: 90), + animationView.heightAnchor.constraint(equalToConstant: 90), + animationView.centerXAnchor.constraint(equalTo: centerXAnchor), + animationView.centerYAnchor.constraint(equalTo: centerYAnchor, constant: -40), + + // Status label below animation + statusLabel.topAnchor.constraint(equalTo: animationView.bottomAnchor, constant: 16), + statusLabel.centerXAnchor.constraint(equalTo: centerXAnchor), + statusLabel.leadingAnchor.constraint(greaterThanOrEqualTo: leadingAnchor, constant: 20), + trailingAnchor.constraint(greaterThanOrEqualTo: statusLabel.trailingAnchor, constant: 20), + + // Button below status (X icon) + showNowButton.topAnchor.constraint(equalTo: statusLabel.bottomAnchor, constant: 16), + showNowButton.centerXAnchor.constraint(equalTo: centerXAnchor), + showNowButton.widthAnchor.constraint(equalToConstant: 32), + showNowButton.heightAnchor.constraint(equalToConstant: 32) + ]) + animationView.play(fromFrame: 0, toFrame: 25, loopMode: .playOnce, completion: { [weak self] (finished) in if finished { // first animation complete! start second one and loop @@ -82,6 +127,14 @@ private class DefaultLoadingView: LoadingView { } }) } + + @objc private func showNowTapped() { + onDismiss?() + } + + override func updateStatus(_ text: String) { + statusLabel.text = text + } required init?(coder: NSCoder) { fatalError("init(coder:) has not been implemented") @@ -89,13 +142,19 @@ private class DefaultLoadingView: LoadingView { override func retheme() { super.retheme() - + backgroundColor = theme?[uicolor: "postsLoadingViewTintColor"] if let tintColor = theme?[uicolor: "tintColor"] { animationView.setValueProvider( ColorValueProvider(tintColor.lottieColorValue), keypath: "**.Fill 1.Color" ) + showNowButton.tintColor = tintColor + } + + // Apply text color to status label + if let textColor = theme?[uicolor: "listTextColor"] { + statusLabel.textColor = textColor } } diff --git a/App/Views/RenderView.swift b/App/Views/RenderView.swift index 2bc995bea..489dab432 100644 --- a/App/Views/RenderView.swift +++ b/App/Views/RenderView.swift @@ -29,8 +29,31 @@ final class RenderView: UIView { private lazy var webView: WKWebView = { let configuration = WKWebViewConfiguration() - + let bundle = Bundle(for: RenderView.self) + + // Conditionally load lottie-player.js if frog and ghost animations are enabled + let frogAndGhostEnabled = FoilDefaultStorage(Settings.frogAndGhostEnabled).wrappedValue + if frogAndGhostEnabled { + configuration.userContentController.addUserScript({ + let url = bundle.url(forResource: "lottie-player.js", withExtension: nil)! + let script = try! String(contentsOf: url) + return WKUserScript(source: script, injectionTime: .atDocumentEnd, forMainFrameOnly: true) + }()) + } + + // Inject early script to track when DOMContentLoaded fires + configuration.userContentController.addUserScript({ + let script = """ + if (!window.Awful) { window.Awful = {}; } + window.Awful.domContentLoadedFired = false; + document.addEventListener('DOMContentLoaded', function() { + window.Awful.domContentLoadedFired = true; + }); + """ + return WKUserScript(source: script, injectionTime: .atDocumentStart, forMainFrameOnly: true) + }()) + configuration.userContentController.addUserScript({ let url = bundle.url(forResource: "RenderView.js", withExtension: nil)! let script = try! String(contentsOf: url) @@ -260,13 +283,13 @@ extension RenderView: WKScriptMessageHandler { struct FetchOEmbedFragment: RenderViewMessage { static let messageName = "fetchOEmbedFragment" - + /// An opaque `id` to use when calling back with the response. let id: String - + /// The OEmbed URL to fetch. let url: URL - + init?(rawMessage: WKScriptMessage, in renderView: RenderView) { assert(rawMessage.name == Self.messageName) guard let body = rawMessage.body as? [String: Any], @@ -274,11 +297,38 @@ extension RenderView: WKScriptMessageHandler { let rawURL = body["url"] as? String, let url = URL(string: rawURL) else { return nil } - + self.id = id self.url = url } } + + /// Sent from the web view to report image loading progress. + struct ImageLoadProgress: RenderViewMessage { + static let messageName = "imageLoadProgress" + + /// Number of images loaded so far + let loaded: Int + + /// Total number of images to load + let total: Int + + /// Whether all images have finished loading + let complete: Bool + + init?(rawMessage: WKScriptMessage, in renderView: RenderView) { + assert(rawMessage.name == Self.messageName) + guard let body = rawMessage.body as? [String: Any], + let loaded = body["loaded"] as? Int, + let total = body["total"] as? Int, + let complete = body["complete"] as? Bool + else { return nil } + + self.loaded = loaded + self.total = total + self.complete = complete + } + } } } @@ -325,13 +375,36 @@ extension RenderView { } } } - - func loadLottiePlayer() { + + /// Applies timeout detection to images that are loading immediately (first 10). + func applyTimeoutToLoadingImages() { + Task { + do { + try await webView.eval("if (window.Awful) { Awful.applyTimeoutToLoadingImages(); }") + } catch { + self.mentionError(error, explanation: "could not evaluate applyTimeoutToLoadingImages") + } + } + } + + /// Sets up lazy loading for deferred images (11+). + func setupImageLazyLoading() { + Task { + do { + try await webView.eval("if (window.Awful) { Awful.setupImageLazyLoading(); }") + } catch { + self.mentionError(error, explanation: "could not evaluate setupImageLazyLoading") + } + } + } + + /// Sets up retry handler for dead image badges. + func setupRetryHandler() { Task { do { - try await webView.eval("if (window.Awful) Awful.loadLotties()") + try await webView.eval("if (window.Awful) { Awful.setupRetryHandler(); }") } catch { - self.mentionError(error, explanation: "could not evaluate loadLotties") + self.mentionError(error, explanation: "could not evaluate setupRetryHandler") } } } diff --git a/AwfulTheming/Sources/AwfulTheming/Stylesheets/_dead-tweet-ghost.less b/AwfulTheming/Sources/AwfulTheming/Stylesheets/_dead-tweet-ghost.less index fd2383e45..82b559092 100644 --- a/AwfulTheming/Sources/AwfulTheming/Stylesheets/_dead-tweet-ghost.less +++ b/AwfulTheming/Sources/AwfulTheming/Stylesheets/_dead-tweet-ghost.less @@ -1,5 +1,6 @@ -.dead-tweet(@tweetLinkColor, @backgroundColor, @ghostColor) { - .dead-tweet-container { +// Generic mixin for all dead embeds (tweets, images, etc.) +.dead-embed(@linkColor, @backgroundColor, @ghostColor) { + .dead-embed-container { display: grid; grid-column: 2; place-items: start; @@ -14,7 +15,7 @@ width: 6em; } - .dead-tweet-title { + .dead-embed-title { grid-column: 2; grid-row: 3; place-items: start; @@ -24,16 +25,29 @@ font-weight: 600; } - .dead-tweet-link { + .dead-embed-link { grid-column: 2; grid-row: 4; place-items: start; padding-right: 23px; - color: @tweetLinkColor; + color: @linkColor; font: -apple-system-headline; font-weight: 450; } + .dead-embed-retry { + grid-column: 2; + grid-row: 5; + place-items: start; + padding-right: 23px; + padding-top: 0.5em; + color: @linkColor; + font: -apple-system-body; + font-weight: 600; + text-decoration: underline; + cursor: pointer; + } + .ghost-fill { fill: @ghostColor; } @@ -45,4 +59,22 @@ .ghost-stroke { stroke: @ghostColor; } +} + +// Backward-compatible wrapper for dead tweets +.dead-tweet(@tweetLinkColor, @backgroundColor, @ghostColor) { + .dead-embed(@tweetLinkColor, @backgroundColor, @ghostColor); + + // Aliases for backward compatibility with existing dead tweet code + .dead-tweet-container { + .dead-embed-container(); + } + + .dead-tweet-title { + .dead-embed-title(); + } + + .dead-tweet-link { + .dead-embed-link(); + } } \ No newline at end of file From dbbfda16966733c533334529bc770d7fb323b9ae Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Thu, 20 Nov 2025 21:18:21 +1100 Subject: [PATCH 02/13] Added a 3 second delay to the loading view exit button and progress messages --- App/Views/LoadingView.swift | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/App/Views/LoadingView.swift b/App/Views/LoadingView.swift index 1d05db42f..0ae974abd 100644 --- a/App/Views/LoadingView.swift +++ b/App/Views/LoadingView.swift @@ -62,6 +62,7 @@ private class DefaultLoadingView: LoadingView { private let animationView: LottieAnimationView private let statusLabel: UILabel private let showNowButton: UIButton + private var visibilityTimer: Timer? override init(theme: Theme?) { animationView = LottieAnimationView( @@ -86,6 +87,7 @@ private class DefaultLoadingView: LoadingView { statusLabel.font = .preferredFont(forTextStyle: .subheadline) statusLabel.textAlignment = .center statusLabel.translatesAutoresizingMaskIntoConstraints = false + statusLabel.alpha = 0 // Initially hidden addSubview(statusLabel) // Setup Show Now button as X in circle icon @@ -95,6 +97,7 @@ private class DefaultLoadingView: LoadingView { showNowButton.translatesAutoresizingMaskIntoConstraints = false showNowButton.contentHorizontalAlignment = .fill showNowButton.contentVerticalAlignment = .fill + showNowButton.alpha = 0 // Initially hidden addSubview(showNowButton) // Layout constraints @@ -135,7 +138,11 @@ private class DefaultLoadingView: LoadingView { override func updateStatus(_ text: String) { statusLabel.text = text } - + + deinit { + visibilityTimer?.invalidate() + } + required init?(coder: NSCoder) { fatalError("init(coder:) has not been implemented") } @@ -160,7 +167,24 @@ private class DefaultLoadingView: LoadingView { fileprivate override func willMove(toSuperview newSuperview: UIView?) { super.willMove(toSuperview: newSuperview) - + + if newSuperview != nil { + // Start 3-second timer to show status and button + visibilityTimer = Timer.scheduledTimer(withTimeInterval: 3.0, repeats: false) { [weak self] _ in + self?.showStatusElements() + } + } else { + // Clean up timer when view is removed + visibilityTimer?.invalidate() + visibilityTimer = nil + } + } + + private func showStatusElements() { + UIView.animate(withDuration: 0.3) { [weak self] in + self?.statusLabel.alpha = 1.0 + self?.showNowButton.alpha = 1.0 + } } } From ca3530d72b00b604a17735ee9a8bcf07f78263e4 Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Sat, 29 Nov 2025 13:08:31 +1100 Subject: [PATCH 03/13] First review performed to improve code quality --- App/Misc/HTMLRenderingHelpers.swift | 52 ++-- App/Resources/RenderView.js | 288 +++++++++--------- .../Posts/PostsPageViewController.swift | 17 +- App/Views/LoadingView.swift | 38 ++- App/Views/RenderView.swift | 48 +-- 5 files changed, 252 insertions(+), 191 deletions(-) diff --git a/App/Misc/HTMLRenderingHelpers.swift b/App/Misc/HTMLRenderingHelpers.swift index e3339e33b..e6aa38545 100644 --- a/App/Misc/HTMLRenderingHelpers.swift +++ b/App/Misc/HTMLRenderingHelpers.swift @@ -6,6 +6,17 @@ import HTMLReader extension HTMLDocument { + // MARK: - Constants + + /// Number of post images to load immediately before deferring to lazy loading. + /// First 10 images load right away for better perceived performance. + private static let immediatelyLoadedImageCount = 10 + + /// 1x1 transparent GIF used as placeholder for lazy-loaded images + private static let transparentPixelPlaceholder = "data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7" + + // MARK: - HTML Processing Methods + /// Finds links that appear to be to Bluesky posts and adds a `data-bluesky-post` attribute to those links. func addAttributeToBlueskyLinks() { for a in nodes(matchingSelector: "a[href *= 'bsky.app']") { @@ -142,16 +153,15 @@ extension HTMLDocument { */ func processImgTags(shouldLinkifyNonSmilies: Bool) { var postContentImageCount = 0 - let initialLoadCount = 10 // First 10 post content images load immediately for img in nodes(matchingSelector: "img") { guard let src = img["src"], let url = URL(string: src) else { continue } - + let isSmilie = isSmilieURL(url) - + if isSmilie { img.toggleClass("awful-smile") } else { @@ -161,30 +171,30 @@ extension HTMLDocument { // Skip attachment.php files (require auth, handled elsewhere) let isAttachment = url.lastPathComponent == "attachment.php" + // Apply URL fixes first to get the final URL + var finalURL = src + if let postimageURL = fixPostimageURL(url) { + finalURL = postimageURL.absoluteString + } else if let waffleURL = randomwaffleURLForWaffleimagesURL(url) { + finalURL = waffleURL.absoluteString + } + + // Determine whether to load immediately or defer based on image type and count if !isAvatar && !isAttachment { // This is a post content image (not avatar, not smilie, not attachment) postContentImageCount += 1 - if postContentImageCount > initialLoadCount { - // Defer loading for images beyond the first 10 - img["data-lazy-src"] = src - img["src"] = "data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7" - } - } - - // Apply URL fixes - if let postimageURL = fixPostimageURL(url) { - if postContentImageCount <= initialLoadCount || isAvatar { - img["src"] = postimageURL.absoluteString - } else { - img["data-lazy-src"] = postimageURL.absoluteString - } - } else if let waffleURL = randomwaffleURLForWaffleimagesURL(url) { - if postContentImageCount <= initialLoadCount || isAvatar { - img["src"] = waffleURL.absoluteString + if postContentImageCount > Self.immediatelyLoadedImageCount { + // Defer loading for images beyond the immediately loaded count + img["data-lazy-src"] = finalURL + img["src"] = Self.transparentPixelPlaceholder } else { - img["data-lazy-src"] = waffleURL.absoluteString + // Load immediately + img["src"] = finalURL } + } else { + // Avatars and attachments always load immediately + img["src"] = finalURL } } diff --git a/App/Resources/RenderView.js b/App/Resources/RenderView.js index cbd63c38f..593f30f32 100644 --- a/App/Resources/RenderView.js +++ b/App/Resources/RenderView.js @@ -10,6 +10,122 @@ if (!window.Awful) { window.Awful = {}; } +// MARK: - Configuration Constants + +/// Number of post images to load immediately before lazy loading kicks in +const IMMEDIATELY_LOADED_IMAGE_COUNT = 10; + +/// How far ahead (in pixels) to start loading lazy content before it enters viewport +const LAZY_LOAD_LOOKAHEAD_DISTANCE = '600px'; + +/// Timeout configuration for image loading with progress detection +const IMAGE_LOAD_TIMEOUT_CONFIG = { + /// Milliseconds to wait for initial connection before aborting (no bytes received) + connectionTimeout: 1000, + + /// Milliseconds to wait during download if progress stalls + downloadStallTimeout: 2500, + + /// Milliseconds between progress checks during download + progressCheckInterval: 500 +}; + +// MARK: - Utility Functions + +/** + * Fetches a URL with smart timeout detection that monitors download progress. + * Times out only when connection fails to start or download stalls. + * + * @param {string} url - The URL to fetch + * @param {Object} config - Optional timeout configuration (uses IMAGE_LOAD_TIMEOUT_CONFIG by default) + * @returns {Promise} The fetched content as a Blob + * @throws {Error} If connection times out, download stalls, or HTTP error occurs + */ +Awful.fetchWithTimeout = async function(url, config = IMAGE_LOAD_TIMEOUT_CONFIG) { + const connectionTimeout = config.connectionTimeout; + const downloadStallTimeout = config.downloadStallTimeout; + const progressCheckInterval = config.progressCheckInterval; + + const controller = new AbortController(); + let lastProgressTime = Date.now(); + let totalBytesReceived = 0; + let progressCheckTimer = null; + + // Monitor download progress and abort if stalled + progressCheckTimer = setInterval(() => { + const timeSinceLastProgress = Date.now() - lastProgressTime; + + if (totalBytesReceived === 0 && timeSinceLastProgress > connectionTimeout) { + // No connection established within timeout + clearInterval(progressCheckTimer); + controller.abort(); + } else if (totalBytesReceived > 0 && timeSinceLastProgress > downloadStallTimeout) { + // Download started but has stalled + clearInterval(progressCheckTimer); + controller.abort(); + } + }, progressCheckInterval); + + try { + const response = await fetch(url, { signal: controller.signal }); + + if (!response.ok) { + throw new Error(`HTTP ${response.status}`); + } + + const reader = response.body.getReader(); + const chunks = []; + + while (true) { + const { done, value } = await reader.read(); + if (done) break; + + chunks.push(value); + totalBytesReceived += value.length; + lastProgressTime = Date.now(); + } + + clearInterval(progressCheckTimer); + return new Blob(chunks); + + } catch (error) { + clearInterval(progressCheckTimer); + throw error; + } +}; + +/** + * Helper for consistent error handling when images fail to load. + * Replaces failed images with dead image badges and updates progress tracker. + * + * @param {Error} error - The error that occurred + * @param {string} url - The URL that failed to load + * @param {HTMLImageElement} img - The image element that failed + * @param {string} imageID - Unique ID for this image + * @param {boolean} enableGhost - Whether to show dead image badge + */ +Awful.handleImageLoadError = function(error, url, img, imageID, enableGhost) { + console.error(`Image load failed: ${error.message} - ${url}`); + + if (enableGhost && img.parentNode) { + const div = document.createElement('div'); + div.classList.add('dead-embed-container'); + div.innerHTML = Awful.deadImageBadgeHTML(url, imageID); + img.parentNode.replaceChild(div, img); + + const player = div.querySelector("lottie-player"); + if (player) { + player.addEventListener("rendered", () => { + const ghostData = document.getElementById("ghost-json-data"); + if (ghostData) { + player.load(ghostData.innerText); + } + }); + } + } + + Awful.imageLoadTracker.incrementLoaded(); +}; /** Retrieves an OEmbed HTML fragment. @@ -209,22 +325,23 @@ Awful.embedTweets = function() { postElements.forEach((post) => { ghostObserver.observe(post); }); + } - // Apply timeout detection to initial images (first 10) - Awful.applyTimeoutToLoadingImages(); + // Image lazy loading (works regardless of ghost feature being enabled) + // Apply timeout detection to initial images (first 10) + Awful.applyTimeoutToLoadingImages(); - // Setup lazy loading for deferred images (11+) - Awful.setupImageLazyLoading(); + // Setup lazy loading for deferred images (11+) + Awful.setupImageLazyLoading(); - // Setup retry handler - Awful.setupRetryHandler(); - } + // Setup retry handler + Awful.setupRetryHandler(); // Set up lazy-loading IntersectionObserver for tweet embeds - // 600px rootMargin means tweets are loaded ~600px before entering the viewport + // Tweets are loaded before entering the viewport based on LAZY_LOAD_LOOKAHEAD_DISTANCE const lazyLoadConfig = { root: null, - rootMargin: '600px 0px', + rootMargin: `${LAZY_LOAD_LOOKAHEAD_DISTANCE} 0px`, threshold: 0.000001, }; @@ -281,60 +398,18 @@ Awful.imageLoadTracker = { } }; -// Image loading with smart timeout detection -// Monitors download progress and only times out stalled connections +/** + * Loads an image with smart timeout detection. + * Monitors download progress and only times out stalled connections. + * + * @param {string} url - The image URL to load + * @param {HTMLImageElement} img - The image element + * @param {string} imageID - Unique ID for this image + * @param {boolean} enableGhost - Whether to show dead image badge on failure + */ Awful.loadImageWithProgressDetection = async function(url, img, imageID, enableGhost) { - const initialTimeout = 1000; // 1s timeout if no bytes received - const stallTimeout = 2500; // 2.5s timeout if download stalls - const heartbeatInterval = 500; // Check progress every 500ms - - const controller = new AbortController(); - let lastProgressTime = Date.now(); - let totalBytes = 0; - let heartbeatTimer = null; - - // Heartbeat check: abort if no progress - heartbeatTimer = setInterval(() => { - const timeSinceProgress = Date.now() - lastProgressTime; - - if (totalBytes === 0 && timeSinceProgress > initialTimeout) { - // No bytes received at all - connection never started - console.warn(`Image timeout: no connection after ${initialTimeout}ms - ${url}`); - clearInterval(heartbeatTimer); - controller.abort(); - } else if (totalBytes > 0 && timeSinceProgress > stallTimeout) { - // Download started but stalled - console.warn(`Image stalled: no progress for ${stallTimeout}ms after ${totalBytes} bytes - ${url}`); - clearInterval(heartbeatTimer); - controller.abort(); - } - }, heartbeatInterval); - try { - const response = await fetch(url, { signal: controller.signal }); - - if (!response.ok) { - throw new Error(`HTTP ${response.status}`); - } - - const reader = response.body.getReader(); - const chunks = []; - - while (true) { - const { done, value } = await reader.read(); - - if (done) break; - - // Bytes received! Update progress tracking - chunks.push(value); - totalBytes += value.length; - lastProgressTime = Date.now(); - } - - clearInterval(heartbeatTimer); - - // Success! Create blob and set image source - const blob = new Blob(chunks); + const blob = await Awful.fetchWithTimeout(url); const objectURL = URL.createObjectURL(blob); img.src = objectURL; @@ -345,29 +420,8 @@ Awful.loadImageWithProgressDetection = async function(url, img, imageID, enableG }; } catch (error) { - clearInterval(heartbeatTimer); - - // Replace image with dead image badge - if (enableGhost && img.parentNode) { - const div = document.createElement('div'); - div.classList.add('dead-embed-container'); - div.innerHTML = Awful.deadImageBadgeHTML(url, imageID); - img.parentNode.replaceChild(div, img); - - // Setup Lottie animation - const player = div.querySelector("lottie-player"); - if (player) { - player.addEventListener("rendered", () => { - const ghostData = document.getElementById("ghost-json-data"); - if (ghostData) { - player.load(ghostData.innerText); - } - }); - } - } - - console.error(`Image load failed: ${error.message} - ${url}`); - Awful.imageLoadTracker.incrementLoaded(); + // Handle failure with consistent error handler + Awful.handleImageLoadError(error, url, img, imageID, enableGhost); } }; @@ -420,7 +474,7 @@ Awful.loadImagesWithTimeout = function() { Awful.setupImageLazyLoading = function() { const enableGhost = Awful.renderGhostTweets || false; - // IntersectionObserver with 600px lookahead (same as tweets) + // IntersectionObserver with lookahead (same as tweets) const imageObserver = new IntersectionObserver(function(entries) { entries.forEach(entry => { if (entry.isIntersecting) { @@ -447,7 +501,7 @@ Awful.setupImageLazyLoading = function() { } }); }, { - rootMargin: '600px 0px', // Load 600px before entering viewport + rootMargin: `${LAZY_LOAD_LOOKAHEAD_DISTANCE} 0px`, threshold: 0.01 }); @@ -473,7 +527,7 @@ Awful.applyTimeoutToLoadingImages = function() { // Initialize progress tracker (only tracks first 10 images, not lazy-loaded ones) Awful.imageLoadTracker.initialize(totalImages); - loadingImages.forEach((img, index) => { + initialImages.forEach((img, index) => { const imageID = `img-${Date.now()}-${index}`; const imageURL = img.src; @@ -529,8 +583,8 @@ Awful.applyTimeoutToLoadingImages = function() { // Set up timeout checker let checkCount = 0; - const maxChecks = 3; // Check 3 times (at 1s, 2s, 3s) - const checkInterval = 1000; // Check every 1 second + const maxChecks = 3; // Check 3 times total + const checkInterval = IMAGE_LOAD_TIMEOUT_CONFIG.connectionTimeout; // Check every second const timeoutChecker = setInterval(() => { checkCount++; @@ -571,6 +625,12 @@ Awful.applyTimeoutToLoadingImages = function() { // Setup retry click handler (using event delegation) - call once on page load Awful.setupRetryHandler = function() { + // Prevent duplicate event listener registration (memory leak fix) + if (Awful.retryHandlerInitialized) { + return; // Already set up + } + Awful.retryHandlerInitialized = true; + document.addEventListener('click', function(event) { const retryLink = event.target; if (retryLink.hasAttribute('data-retry-image')) { @@ -589,54 +649,10 @@ Awful.setupRetryHandler = function() { img.style.display = 'none'; container.parentNode.insertBefore(img, container); - // Custom retry with success/failure callbacks + // Retry loading with feedback const retryWithFeedback = async function() { - const initialTimeout = 1000; - const stallTimeout = 2500; - const heartbeatInterval = 500; - - const controller = new AbortController(); - let lastProgressTime = Date.now(); - let totalBytes = 0; - let heartbeatTimer = null; - - heartbeatTimer = setInterval(() => { - const timeSinceProgress = Date.now() - lastProgressTime; - - if (totalBytes === 0 && timeSinceProgress > initialTimeout) { - console.warn(`Retry failed: no connection after ${initialTimeout}ms - ${imageURL}`); - clearInterval(heartbeatTimer); - controller.abort(); - } else if (totalBytes > 0 && timeSinceProgress > stallTimeout) { - console.warn(`Retry failed: stalled after ${totalBytes} bytes - ${imageURL}`); - clearInterval(heartbeatTimer); - controller.abort(); - } - }, heartbeatInterval); - try { - const response = await fetch(imageURL, { signal: controller.signal }); - - if (!response.ok) { - throw new Error(`HTTP ${response.status}`); - } - - const reader = response.body.getReader(); - const chunks = []; - - while (true) { - const { done, value } = await reader.read(); - if (done) break; - - chunks.push(value); - totalBytes += value.length; - lastProgressTime = Date.now(); - } - - clearInterval(heartbeatTimer); - - // SUCCESS! Create blob and replace badge with image - const blob = new Blob(chunks); + const blob = await Awful.fetchWithTimeout(imageURL); const objectURL = URL.createObjectURL(blob); const successImg = document.createElement('img'); @@ -654,8 +670,6 @@ Awful.setupRetryHandler = function() { }; } catch (error) { - clearInterval(heartbeatTimer); - // FAILED - restore retry button with "Failed" feedback console.error(`Retry failed: ${error.message} - ${imageURL}`); diff --git a/App/View Controllers/Posts/PostsPageViewController.swift b/App/View Controllers/Posts/PostsPageViewController.swift index ce84cc4a1..54dbde6c8 100644 --- a/App/View Controllers/Posts/PostsPageViewController.swift +++ b/App/View Controllers/Posts/PostsPageViewController.swift @@ -422,7 +422,7 @@ final class PostsPageViewController: ViewController { Task { await postsView.renderView.eraseDocument() await MainActor.run { - self.postsView.loadingView?.updateStatus("Loading page...") + self.postsView.loadingView?.updateStatus("Rendering page...") } self.postsView.renderView.render(html: html, baseURL: ForumsClient.shared.baseURL) } @@ -1805,12 +1805,17 @@ extension PostsPageViewController: RenderViewDelegate { fetchOEmbed(url: message.url, id: message.id) case let message as RenderView.BuiltInMessage.ImageLoadProgress: - let statusText = "Downloading images: \(message.loaded)/\(message.total)" - postsView.loadingView?.updateStatus(statusText) - - // Dismiss loading view when all images are done - if message.complete { + if message.total == 0 { + // No images to load, dismiss immediately clearLoadingMessage() + } else { + let statusText = "Downloading images: \(message.loaded)/\(message.total)" + postsView.loadingView?.updateStatus(statusText) + + // Dismiss loading view when all images are done + if message.complete { + clearLoadingMessage() + } } case is FYADFlagRequest: diff --git a/App/Views/LoadingView.swift b/App/Views/LoadingView.swift index 0ae974abd..195acd8a5 100644 --- a/App/Views/LoadingView.swift +++ b/App/Views/LoadingView.swift @@ -9,17 +9,26 @@ import Lottie /// A view that covers its superview with an indeterminate progress indicator. class LoadingView: UIView { + + // MARK: - Constants + + /// Duration in seconds before showing the exit button and status messages. + /// 3 seconds gives users time to see loading begin while preventing accidental early dismissal. + fileprivate static let statusElementsVisibilityDelay: TimeInterval = 3.0 + + // MARK: - Properties + fileprivate let theme: Theme? - + fileprivate init(theme: Theme?) { self.theme = theme super.init(frame: .zero) } - + convenience init() { self.init(theme: nil) } - + required init?(coder: NSCoder) { fatalError("init(coder:) has not been implemented") } @@ -37,10 +46,23 @@ class LoadingView: UIView { } } - /// Callback invoked when user requests early dismissal + /// Callback invoked when user dismisses the loading view via the X button. + /// + /// Use weak/unowned captures to avoid retain cycles: + /// ```swift + /// loadingView.onDismiss = { [weak self] in + /// self?.handleDismissal() + /// } + /// ``` var onDismiss: (() -> Void)? - /// Update status text (override in subclasses to implement) + /// Updates the status text displayed in the loading view. + /// + /// This method should be overridden in subclasses to implement status updates. + /// The default implementation does nothing. Only the default loading view theme + /// currently supports status updates. + /// + /// - Parameter text: The status message to display func updateStatus(_ text: String) { // Override in subclasses } @@ -169,8 +191,10 @@ private class DefaultLoadingView: LoadingView { super.willMove(toSuperview: newSuperview) if newSuperview != nil { - // Start 3-second timer to show status and button - visibilityTimer = Timer.scheduledTimer(withTimeInterval: 3.0, repeats: false) { [weak self] _ in + // Invalidate any existing timer first to prevent race conditions + visibilityTimer?.invalidate() + // Start timer to show status and button after delay + visibilityTimer = Timer.scheduledTimer(withTimeInterval: LoadingView.statusElementsVisibilityDelay, repeats: false) { [weak self] _ in self?.showStatusElements() } } else { diff --git a/App/Views/RenderView.swift b/App/Views/RenderView.swift index 489dab432..7bcc8cffd 100644 --- a/App/Views/RenderView.swift +++ b/App/Views/RenderView.swift @@ -376,37 +376,45 @@ extension RenderView { } } - /// Applies timeout detection to images that are loading immediately (first 10). - func applyTimeoutToLoadingImages() { + // MARK: - Private Helpers + + /// Helper to evaluate Awful JavaScript functions with consistent error handling. + private func evalAwfulFunction(_ functionName: String) { Task { do { - try await webView.eval("if (window.Awful) { Awful.applyTimeoutToLoadingImages(); }") + try await webView.eval("if (window.Awful) { Awful.\(functionName)(); }") } catch { - self.mentionError(error, explanation: "could not evaluate applyTimeoutToLoadingImages") + self.mentionError(error, explanation: "could not evaluate \(functionName)") } } } - /// Sets up lazy loading for deferred images (11+). + // MARK: - Image Loading Methods + + /// Applies timeout detection to images that are loading immediately (first 10 images). + /// + /// Monitors download progress and replaces stalled images with dead image badges. + /// This function should be called after the page has loaded to set up monitoring + /// for the initial batch of images. + func applyTimeoutToLoadingImages() { + evalAwfulFunction("applyTimeoutToLoadingImages") + } + + /// Sets up lazy loading for deferred images (images 11+). + /// + /// Uses IntersectionObserver to load images as they approach the viewport. + /// Images are loaded approximately 600px before they would be visible to the user + /// for a seamless scrolling experience. func setupImageLazyLoading() { - Task { - do { - try await webView.eval("if (window.Awful) { Awful.setupImageLazyLoading(); }") - } catch { - self.mentionError(error, explanation: "could not evaluate setupImageLazyLoading") - } - } + evalAwfulFunction("setupImageLazyLoading") } - /// Sets up retry handler for dead image badges. + /// Sets up click handler for retry links on failed images. + /// + /// Allows users to retry loading images that timed out or failed to load. + /// The retry mechanism uses the same timeout detection as initial image loading. func setupRetryHandler() { - Task { - do { - try await webView.eval("if (window.Awful) { Awful.setupRetryHandler(); }") - } catch { - self.mentionError(error, explanation: "could not evaluate setupRetryHandler") - } - } + evalAwfulFunction("setupRetryHandler") } /// iOS 15 and transparent webviews = dark "missing" scroll thumbs, regardless of settings applied From 2944b0a2d1e12694900607fd13f0ef2bae7532eb Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Sat, 29 Nov 2025 13:29:17 +1100 Subject: [PATCH 04/13] Another code review performed --- App/Resources/RenderView.js | 297 +++++++++++++++++++++++------------- App/Views/LoadingView.swift | 5 + 2 files changed, 195 insertions(+), 107 deletions(-) diff --git a/App/Resources/RenderView.js b/App/Resources/RenderView.js index 593f30f32..fa3659e87 100644 --- a/App/Resources/RenderView.js +++ b/App/Resources/RenderView.js @@ -27,11 +27,69 @@ const IMAGE_LOAD_TIMEOUT_CONFIG = { downloadStallTimeout: 2500, /// Milliseconds between progress checks during download - progressCheckInterval: 500 + progressCheckInterval: 500, + + /// Maximum number of checks for initial image loading timeout detection + maxImageChecks: 3, + + /// Milliseconds to wait before resetting retry button text after failed retry + retryResetDelay: 3000 }; // MARK: - Utility Functions +/** + * Sets up a Lottie player to load ghost animation data. + * Helper to avoid code duplication for dead tweet/image badge initialization. + * + * @param {HTMLElement} container - The container element containing lottie-player elements + */ +Awful.setupGhostLottiePlayer = function(container) { + const players = container.querySelectorAll("lottie-player"); + players.forEach((lottiePlayer) => { + lottiePlayer.addEventListener("rendered", () => { + const ghostData = document.getElementById("ghost-json-data"); + if (ghostData) { + lottiePlayer.load(ghostData.innerText); + } + }); + }); +}; + +/** + * Sanitizes a URL to prevent XSS attacks. + * Ensures URLs don't contain dangerous protocols like javascript: or data:text/html + * + * @param {string} url - The URL to sanitize + * @returns {string} The sanitized URL or '#' if dangerous + */ +Awful.sanitizeURL = function(url) { + if (!url) return '#'; + + const urlLower = url.trim().toLowerCase(); + + // Block dangerous protocols + if (urlLower.startsWith('javascript:') || + urlLower.startsWith('data:text/html') || + urlLower.startsWith('vbscript:')) { + return '#'; + } + + return url; +}; + +/** + * HTML-escapes a string to prevent XSS when used in HTML context. + * + * @param {string} str - The string to escape + * @returns {string} The escaped string + */ +Awful.escapeHTML = function(str) { + const div = document.createElement('div'); + div.textContent = str; + return div.innerHTML; +}; + /** * Fetches a URL with smart timeout detection that monitors download progress. * Times out only when connection fails to start or download stalls. @@ -96,15 +154,16 @@ Awful.fetchWithTimeout = async function(url, config = IMAGE_LOAD_TIMEOUT_CONFIG) /** * Helper for consistent error handling when images fail to load. - * Replaces failed images with dead image badges and updates progress tracker. + * Replaces failed images with dead image badges and optionally updates progress tracker. * * @param {Error} error - The error that occurred * @param {string} url - The URL that failed to load * @param {HTMLImageElement} img - The image element that failed * @param {string} imageID - Unique ID for this image * @param {boolean} enableGhost - Whether to show dead image badge + * @param {boolean} trackProgress - Whether to increment the progress tracker (default: true) */ -Awful.handleImageLoadError = function(error, url, img, imageID, enableGhost) { +Awful.handleImageLoadError = function(error, url, img, imageID, enableGhost, trackProgress = true) { console.error(`Image load failed: ${error.message} - ${url}`); if (enableGhost && img.parentNode) { @@ -113,18 +172,14 @@ Awful.handleImageLoadError = function(error, url, img, imageID, enableGhost) { div.innerHTML = Awful.deadImageBadgeHTML(url, imageID); img.parentNode.replaceChild(div, img); - const player = div.querySelector("lottie-player"); - if (player) { - player.addEventListener("rendered", () => { - const ghostData = document.getElementById("ghost-json-data"); - if (ghostData) { - player.load(ghostData.innerText); - } - }); - } + // Use helper function to set up Lottie player (fixes code duplication) + Awful.setupGhostLottiePlayer(div); } - Awful.imageLoadTracker.incrementLoaded(); + // Only increment progress for initially loaded images, not lazy-loaded ones + if (trackProgress) { + Awful.imageLoadTracker.incrementLoaded(); + } }; /** @@ -221,12 +276,8 @@ Awful.embedTweetNow = function(thisPostElement) { div.innerHTML = Awful.deadTweetBadgeHTML(a.href.toString(), `${tweetID}`); a.parentNode.replaceChild(div, a); - const player = div.querySelectorAll("lottie-player"); - player.forEach((lottiePlayer) => { - lottiePlayer.addEventListener("rendered", () => { - lottiePlayer.load(document.getElementById("ghost-json-data").innerText); - }); - }); + // Use helper function to set up Lottie player (fixes code duplication and null check) + Awful.setupGhostLottiePlayer(div); } }); } @@ -302,13 +353,18 @@ Awful.embedTweets = function() { // Set up IntersectionObserver for ghost Lottie animations (play/pause on scroll) if (enableGhost) { + // Disconnect previous observer if it exists (prevents memory leak on re-render) + if (Awful.ghostLottieObserver) { + Awful.ghostLottieObserver.disconnect(); + } + const ghostConfig = { root: document.body.posts, rootMargin: '0px', threshold: 0.000001, }; - const ghostObserver = new IntersectionObserver(function(posts) { + Awful.ghostLottieObserver = new IntersectionObserver(function(posts) { posts.forEach((post) => { const players = post.target.querySelectorAll("lottie-player"); players.forEach((lottiePlayer) => { @@ -323,7 +379,7 @@ Awful.embedTweets = function() { const postElements = document.querySelectorAll("post"); postElements.forEach((post) => { - ghostObserver.observe(post); + Awful.ghostLottieObserver.observe(post); }); } @@ -339,13 +395,18 @@ Awful.embedTweets = function() { // Set up lazy-loading IntersectionObserver for tweet embeds // Tweets are loaded before entering the viewport based on LAZY_LOAD_LOOKAHEAD_DISTANCE + // Disconnect previous observer if it exists (prevents memory leak on re-render) + if (Awful.tweetLazyLoadObserver) { + Awful.tweetLazyLoadObserver.disconnect(); + } + const lazyLoadConfig = { root: null, rootMargin: `${LAZY_LOAD_LOOKAHEAD_DISTANCE} 0px`, threshold: 0.000001, }; - const lazyLoadObserver = new IntersectionObserver(function(entries) { + Awful.tweetLazyLoadObserver = new IntersectionObserver(function(entries) { entries.forEach((entry) => { if (entry.isIntersecting) { Awful.embedTweetNow(entry.target); @@ -356,7 +417,7 @@ Awful.embedTweets = function() { // Observe all post elements for lazy loading const posts = document.querySelectorAll("post"); posts.forEach((post) => { - lazyLoadObserver.observe(post); + Awful.tweetLazyLoadObserver.observe(post); }); // Notify native side when tweets are loaded @@ -406,8 +467,9 @@ Awful.imageLoadTracker = { * @param {HTMLImageElement} img - The image element * @param {string} imageID - Unique ID for this image * @param {boolean} enableGhost - Whether to show dead image badge on failure + * @param {boolean} trackProgress - Whether to track progress (default: false, only for initial images) */ -Awful.loadImageWithProgressDetection = async function(url, img, imageID, enableGhost) { +Awful.loadImageWithProgressDetection = async function(url, img, imageID, enableGhost, trackProgress = false) { try { const blob = await Awful.fetchWithTimeout(url); const objectURL = URL.createObjectURL(blob); @@ -416,66 +478,44 @@ Awful.loadImageWithProgressDetection = async function(url, img, imageID, enableG // Clean up object URL after image loads img.onload = () => { URL.revokeObjectURL(objectURL); - Awful.imageLoadTracker.incrementLoaded(); + // Only track progress for initial images, not lazy-loaded ones + if (trackProgress) { + Awful.imageLoadTracker.incrementLoaded(); + } + }; + + // Handle image decode failures (corrupt image data) + img.onerror = () => { + URL.revokeObjectURL(objectURL); + const decodeError = new Error("Image decode failed"); + Awful.handleImageLoadError(decodeError, url, img, imageID, enableGhost, trackProgress); }; } catch (error) { - // Handle failure with consistent error handler - Awful.handleImageLoadError(error, url, img, imageID, enableGhost); + // Handle fetch/network failures + Awful.handleImageLoadError(error, url, img, imageID, enableGhost, trackProgress); } }; -// Main function to apply timeout detection to all post images -Awful.loadImagesWithTimeout = function() { +/** + * Lazy loads post content images using IntersectionObserver (for images 11+). + * Images load as they approach the viewport with a lookahead distance. + */ +Awful.setupImageLazyLoading = function() { const enableGhost = Awful.renderGhostTweets || false; - // Only process images in post content, exclude avatars and smilies - const contentImages = document.querySelectorAll('section.postbody img:not(.awful-smile):not(.awful-avatar)'); - - contentImages.forEach((img, index) => { - const imageID = `img-${Date.now()}-${index}`; - - // Skip if already loaded - if (img.complete && img.naturalHeight !== 0) { - return; - } - - // Clear the src to prevent default loading, we'll handle it with fetch - const originalSrc = img.src; - img.removeAttribute('src'); - - // Load with progress detection - Awful.loadImageWithProgressDetection(originalSrc, img, imageID, enableGhost); - }); - - // Setup retry click handler (using event delegation) - document.addEventListener('click', function(event) { - const retryLink = event.target; - if (retryLink.hasAttribute('data-retry-image')) { - event.preventDefault(); - - const imageURL = retryLink.getAttribute('data-retry-image'); - const imageID = retryLink.getAttribute('data-image-id'); - const container = retryLink.closest('.dead-embed-container'); - - if (container) { - const img = document.createElement('img'); - img.setAttribute('alt', ''); - container.parentNode.replaceChild(img, container); - - // Retry loading with timeout detection - Awful.loadImageWithProgressDetection(imageURL, img, imageID, enableGhost); - } - } - }, { once: false }); -}; + // Disconnect previous observer if it exists (prevents memory leak on re-render) + if (Awful.imageLazyLoadObserver) { + Awful.imageLazyLoadObserver.disconnect(); + } -// Lazy loads post content images using IntersectionObserver (for images 11+) -Awful.setupImageLazyLoading = function() { - const enableGhost = Awful.renderGhostTweets || false; + // Initialize sequential counter for lazy image IDs + if (typeof Awful.lazyImageIDCounter === 'undefined') { + Awful.lazyImageIDCounter = 0; + } // IntersectionObserver with lookahead (same as tweets) - const imageObserver = new IntersectionObserver(function(entries) { + Awful.imageLazyLoadObserver = new IntersectionObserver(function(entries) { entries.forEach(entry => { if (entry.isIntersecting) { const img = entry.target; @@ -486,17 +526,19 @@ Awful.setupImageLazyLoading = function() { if (lazySrc.includes('attachment.php')) { delete img.dataset.lazySrc; img.src = lazySrc; // Load normally without timeout - imageObserver.unobserve(img); + Awful.imageLazyLoadObserver.unobserve(img); return; } - const imageID = `lazy-${Date.now()}-${Math.random().toString(36).substring(2, 11)}`; + // Use sequential counter for guaranteed unique IDs + const imageID = `lazy-${Awful.lazyImageIDCounter++}`; delete img.dataset.lazySrc; // Load with smart timeout detection - Awful.loadImageWithProgressDetection(lazySrc, img, imageID, enableGhost); + // trackProgress = false because lazy images don't count toward progress + Awful.loadImageWithProgressDetection(lazySrc, img, imageID, enableGhost, false); - imageObserver.unobserve(img); + Awful.imageLazyLoadObserver.unobserve(img); } } }); @@ -507,11 +549,14 @@ Awful.setupImageLazyLoading = function() { // Observe all lazy-loadable images document.querySelectorAll('img[data-lazy-src]').forEach(img => { - imageObserver.observe(img); + Awful.imageLazyLoadObserver.observe(img); }); }; -// Apply timeout detection to images that are loading normally (first 10) +/** + * Apply timeout detection to images that are loading normally (first 10). + * Monitors initial image loading and tracks progress for the loading view. + */ Awful.applyTimeoutToLoadingImages = function() { const enableGhost = Awful.renderGhostTweets || false; @@ -527,8 +572,13 @@ Awful.applyTimeoutToLoadingImages = function() { // Initialize progress tracker (only tracks first 10 images, not lazy-loaded ones) Awful.imageLoadTracker.initialize(totalImages); + // Store interval timers for cleanup (prevents memory leaks) + if (!Awful.imageTimeoutCheckers) { + Awful.imageTimeoutCheckers = []; + } + initialImages.forEach((img, index) => { - const imageID = `img-${Date.now()}-${index}`; + const imageID = `img-init-${index}`; const imageURL = img.src; // Skip data URLs (placeholders) @@ -567,24 +617,17 @@ Awful.applyTimeoutToLoadingImages = function() { div.innerHTML = Awful.deadImageBadgeHTML(imageURL, imageID); img.parentNode.replaceChild(div, img); - const player = div.querySelector("lottie-player"); - if (player) { - player.addEventListener("rendered", () => { - const ghostData = document.getElementById("ghost-json-data"); - if (ghostData) { - player.load(ghostData.innerText); - } - }); - } + // Use helper function to set up Lottie player (fixes code duplication) + Awful.setupGhostLottiePlayer(div); } Awful.imageLoadTracker.incrementLoaded(); }; - // Set up timeout checker + // Set up timeout checker using config constants let checkCount = 0; - const maxChecks = 3; // Check 3 times total - const checkInterval = IMAGE_LOAD_TIMEOUT_CONFIG.connectionTimeout; // Check every second + const maxChecks = IMAGE_LOAD_TIMEOUT_CONFIG.maxImageChecks; + const checkInterval = IMAGE_LOAD_TIMEOUT_CONFIG.connectionTimeout; const timeoutChecker = setInterval(() => { checkCount++; @@ -599,17 +642,20 @@ Awful.applyTimeoutToLoadingImages = function() { // If image failed to load (error state) if (img.complete && img.naturalHeight === 0) { clearInterval(timeoutChecker); - handleFailure('failed'); + handleFailure(); return; } // If we've checked enough times and it's still not loaded, timeout if (checkCount >= maxChecks) { clearInterval(timeoutChecker); - handleFailure(`timed out after ${checkCount * checkInterval}ms`); + handleFailure(); } }, checkInterval); + // Store timer for potential cleanup + Awful.imageTimeoutCheckers.push(timeoutChecker); + // Also listen for load/error events to handle immediately img.addEventListener('load', () => { clearInterval(timeoutChecker); @@ -618,12 +664,15 @@ Awful.applyTimeoutToLoadingImages = function() { img.addEventListener('error', () => { clearInterval(timeoutChecker); - handleFailure('error event'); + handleFailure(); }, { once: true }); }); }; -// Setup retry click handler (using event delegation) - call once on page load +/** + * Setup retry click handler (using event delegation) - call once on page load. + * Allows users to retry loading failed images. + */ Awful.setupRetryHandler = function() { // Prevent duplicate event listener registration (memory leak fix) if (Awful.retryHandlerInitialized) { @@ -669,6 +718,12 @@ Awful.setupRetryHandler = function() { } }; + // Handle decode failures on retry + successImg.onerror = () => { + URL.revokeObjectURL(objectURL); + throw new Error("Image decode failed on retry"); + }; + } catch (error) { // FAILED - restore retry button with "Failed" feedback console.error(`Retry failed: ${error.message} - ${imageURL}`); @@ -681,12 +736,12 @@ Awful.setupRetryHandler = function() { img.parentNode.removeChild(img); } - // Reset to just "Retry" after 3 seconds + // Reset to just "Retry" after configured delay setTimeout(() => { if (retryLink.textContent === 'Retry Failed - Try Again') { retryLink.textContent = 'Retry'; } - }, 3000); + }, IMAGE_LOAD_TIMEOUT_CONFIG.retryResetDelay); } }; @@ -1183,16 +1238,30 @@ Awful.setAnnouncementHTML = function(html) { Awful.deadTweetBadgeHTML = function(url, tweetID){ - // get twitter username from url - var tweeter = url.match(/(?:https?:\/\/)?(?:www\.)?twitter\.com\/(?:#!\/)?@?([^\/\?\s]*)/)[1]; + // Sanitize URL to prevent XSS attacks + const safeURL = Awful.sanitizeURL(url); + + // get twitter username from url (with fallback for malformed URLs) + let tweeter = 'unknown'; + try { + const match = url.match(/(?:https?:\/\/)?(?:www\.)?twitter\.com\/(?:#!\/)?@?([^\/\?\s]*)/); + if (match && match[1]) { + tweeter = Awful.escapeHTML(match[1]); + } + } catch (e) { + console.error('Error parsing tweet URL:', e); + } + + // Escape tweetID for use in HTML attributes + const safeTweetID = Awful.escapeHTML(tweetID); var html = `
- +
DEAD TWEET - @${tweeter} + @${tweeter} `; return html; @@ -1200,17 +1269,31 @@ Awful.deadTweetBadgeHTML = function(url, tweetID){ // Dead Image Badge (similar to dead tweet) Awful.deadImageBadgeHTML = function(url, imageID) { - // Extract filename from URL - var filename = url.split('/').pop().split('?')[0] || 'unknown'; + // Sanitize URL to prevent XSS attacks + const safeURL = Awful.sanitizeURL(url); + + // Extract filename from URL and escape it + let filename = 'unknown'; + try { + const urlParts = url.split('/').pop().split('?')[0]; + if (urlParts) { + filename = Awful.escapeHTML(urlParts); + } + } catch (e) { + console.error('Error parsing image URL:', e); + } + + // Escape imageID for use in HTML attributes + const safeImageID = Awful.escapeHTML(imageID); var html = `
- +
DEAD IMAGE - ${filename} - Retry + ${filename} + Retry `; return html; diff --git a/App/Views/LoadingView.swift b/App/Views/LoadingView.swift index 195acd8a5..b929fb2d1 100644 --- a/App/Views/LoadingView.swift +++ b/App/Views/LoadingView.swift @@ -205,7 +205,12 @@ private class DefaultLoadingView: LoadingView { } private func showStatusElements() { + // Check that view is still in hierarchy before animating (prevents race condition) + guard superview != nil else { return } + UIView.animate(withDuration: 0.3) { [weak self] in + // Double-check during animation block + guard self?.superview != nil else { return } self?.statusLabel.alpha = 1.0 self?.showNowButton.alpha = 1.0 } From 9547576e5a776414ad3c29886fd45c75ccea19fb Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Sat, 29 Nov 2025 14:04:28 +1100 Subject: [PATCH 05/13] One more review --- App/Resources/RenderView.js | 117 +++++++++++++++++++++++++----------- App/Views/LoadingView.swift | 3 +- 2 files changed, 83 insertions(+), 37 deletions(-) diff --git a/App/Resources/RenderView.js b/App/Resources/RenderView.js index fa3659e87..1fd2f7ee7 100644 --- a/App/Resources/RenderView.js +++ b/App/Resources/RenderView.js @@ -18,21 +18,34 @@ const IMMEDIATELY_LOADED_IMAGE_COUNT = 10; /// How far ahead (in pixels) to start loading lazy content before it enters viewport const LAZY_LOAD_LOOKAHEAD_DISTANCE = '600px'; +/// CSS selectors used throughout the code +const SELECTORS = { + LAZY_IMAGE: 'img[data-lazy-src]', + LOADING_IMAGES: 'section.postbody img[src]:not(.awful-smile):not(.awful-avatar):not([data-lazy-src])', + POST_ELEMENTS: 'post', + LOTTIE_PLAYERS: 'lottie-player' +}; + /// Timeout configuration for image loading with progress detection const IMAGE_LOAD_TIMEOUT_CONFIG = { /// Milliseconds to wait for initial connection before aborting (no bytes received) + /// 1000ms gives slow connections a chance while quickly failing on unreachable hosts connectionTimeout: 1000, /// Milliseconds to wait during download if progress stalls + /// 2500ms accommodates network hiccups without excessive waiting on stalled downloads downloadStallTimeout: 2500, /// Milliseconds between progress checks during download + /// 500ms balances responsiveness with minimal performance overhead progressCheckInterval: 500, /// Maximum number of checks for initial image loading timeout detection + /// 3 checks × 1000ms = 3 seconds max wait for images to start loading maxImageChecks: 3, /// Milliseconds to wait before resetting retry button text after failed retry + /// 3000ms gives user time to read the error message before it resets retryResetDelay: 3000 }; @@ -45,7 +58,7 @@ const IMAGE_LOAD_TIMEOUT_CONFIG = { * @param {HTMLElement} container - The container element containing lottie-player elements */ Awful.setupGhostLottiePlayer = function(container) { - const players = container.querySelectorAll("lottie-player"); + const players = container.querySelectorAll(SELECTORS.LOTTIE_PLAYERS); players.forEach((lottiePlayer) => { lottiePlayer.addEventListener("rendered", () => { const ghostData = document.getElementById("ghost-json-data"); @@ -70,11 +83,16 @@ Awful.sanitizeURL = function(url) { // Block dangerous protocols if (urlLower.startsWith('javascript:') || - urlLower.startsWith('data:text/html') || urlLower.startsWith('vbscript:')) { return '#'; } + // Block all data: URLs except safe image formats + if (urlLower.startsWith('data:') && + !urlLower.startsWith('data:image/')) { + return '#'; + } + return url; }; @@ -103,9 +121,11 @@ Awful.fetchWithTimeout = async function(url, config = IMAGE_LOAD_TIMEOUT_CONFIG) const connectionTimeout = config.connectionTimeout; const downloadStallTimeout = config.downloadStallTimeout; const progressCheckInterval = config.progressCheckInterval; + const MAX_TOTAL_TIMEOUT = 30000; // 30 seconds absolute maximum const controller = new AbortController(); let lastProgressTime = Date.now(); + const startTime = Date.now(); let totalBytesReceived = 0; let progressCheckTimer = null; @@ -135,6 +155,11 @@ Awful.fetchWithTimeout = async function(url, config = IMAGE_LOAD_TIMEOUT_CONFIG) const chunks = []; while (true) { + // Check absolute timeout to prevent indefinite hangs + if (Date.now() - startTime > MAX_TOTAL_TIMEOUT) { + throw new Error('Total timeout exceeded'); + } + const { done, value } = await reader.read(); if (done) break; @@ -143,12 +168,11 @@ Awful.fetchWithTimeout = async function(url, config = IMAGE_LOAD_TIMEOUT_CONFIG) lastProgressTime = Date.now(); } - clearInterval(progressCheckTimer); return new Blob(chunks); - } catch (error) { + } finally { + // Always clean up timer, even if an error occurs clearInterval(progressCheckTimer); - throw error; } }; @@ -348,8 +372,15 @@ Awful.embedBlueskyPosts = function() { * Also sets up Lottie animation play/pause for ghost tweets in the viewport. */ Awful.embedTweets = function() { - Awful.loadTwitterWidgets(); - const enableGhost = (window.Awful.renderGhostTweets == true); + // Prevent concurrent setup (race condition protection) + if (Awful.embedTweetsInProgress) { + return; + } + Awful.embedTweetsInProgress = true; + + try { + Awful.loadTwitterWidgets(); + const enableGhost = (window.Awful.renderGhostTweets == true); // Set up IntersectionObserver for ghost Lottie animations (play/pause on scroll) if (enableGhost) { @@ -366,7 +397,7 @@ Awful.embedTweets = function() { Awful.ghostLottieObserver = new IntersectionObserver(function(posts) { posts.forEach((post) => { - const players = post.target.querySelectorAll("lottie-player"); + const players = post.target.querySelectorAll(SELECTORS.LOTTIE_PLAYERS); players.forEach((lottiePlayer) => { if (post.isIntersecting) { lottiePlayer.play(); @@ -377,7 +408,7 @@ Awful.embedTweets = function() { }); }, ghostConfig); - const postElements = document.querySelectorAll("post"); + const postElements = document.querySelectorAll(SELECTORS.POST_ELEMENTS); postElements.forEach((post) => { Awful.ghostLottieObserver.observe(post); }); @@ -415,7 +446,7 @@ Awful.embedTweets = function() { }, lazyLoadConfig); // Observe all post elements for lazy loading - const posts = document.querySelectorAll("post"); + const posts = document.querySelectorAll(SELECTORS.POST_ELEMENTS); posts.forEach((post) => { Awful.tweetLazyLoadObserver.observe(post); }); @@ -430,6 +461,11 @@ Awful.embedTweets = function() { } }); } + + } finally { + // Always reset flag, even if an error occurs + Awful.embedTweetsInProgress = false; + } }; // Image load progress tracker @@ -438,6 +474,12 @@ Awful.imageLoadTracker = { total: 0, initialize: function(totalCount) { + // Guard against zero-image case (prevents misleading progress reports) + if (totalCount === 0) { + this.loaded = 0; + this.total = 0; + return; // Don't report progress for pages with no images + } this.loaded = 0; this.total = totalCount; this.reportProgress(); @@ -509,10 +551,8 @@ Awful.setupImageLazyLoading = function() { Awful.imageLazyLoadObserver.disconnect(); } - // Initialize sequential counter for lazy image IDs - if (typeof Awful.lazyImageIDCounter === 'undefined') { - Awful.lazyImageIDCounter = 0; - } + // Reset sequential counter for lazy image IDs (prevents ID collisions on page reload) + Awful.lazyImageIDCounter = 0; // IntersectionObserver with lookahead (same as tweets) Awful.imageLazyLoadObserver = new IntersectionObserver(function(entries) { @@ -548,7 +588,7 @@ Awful.setupImageLazyLoading = function() { }); // Observe all lazy-loadable images - document.querySelectorAll('img[data-lazy-src]').forEach(img => { + document.querySelectorAll(SELECTORS.LAZY_IMAGE).forEach(img => { Awful.imageLazyLoadObserver.observe(img); }); }; @@ -561,7 +601,7 @@ Awful.applyTimeoutToLoadingImages = function() { const enableGhost = Awful.renderGhostTweets || false; // Find images with real src (not data-lazy-src) - these are the first 10 images - const loadingImages = document.querySelectorAll('section.postbody img[src]:not(.awful-smile):not(.awful-avatar):not([data-lazy-src])'); + const loadingImages = document.querySelectorAll(SELECTORS.LOADING_IMAGES); // Count only the initially loading images (first 10), excluding attachment.php and data URLs const initialImages = Array.from(loadingImages).filter(img => @@ -573,25 +613,18 @@ Awful.applyTimeoutToLoadingImages = function() { Awful.imageLoadTracker.initialize(totalImages); // Store interval timers for cleanup (prevents memory leaks) - if (!Awful.imageTimeoutCheckers) { - Awful.imageTimeoutCheckers = []; - } + // Reset array on each call to prevent memory leak across page reloads + Awful.imageTimeoutCheckers = []; initialImages.forEach((img, index) => { const imageID = `img-init-${index}`; const imageURL = img.src; - // Skip data URLs (placeholders) - if (imageURL.startsWith('data:')) { - return; - } - - // Skip attachment.php files (require auth, handled elsewhere) - if (imageURL.includes('attachment.php')) { - return; - } + // Note: attachment.php and data: URLs already filtered out above (line 565-567) // Skip if already loaded (but count it) + // Note: img.complete is true for both successfully loaded AND failed images + // We discriminate using naturalHeight: >0 means success, ===0 means failure if (img.complete && img.naturalHeight !== 0) { Awful.imageLoadTracker.incrementLoaded(); return; @@ -633,6 +666,8 @@ Awful.applyTimeoutToLoadingImages = function() { checkCount++; // If image loaded successfully + // Note: img.complete is true for both success and failure + // naturalHeight > 0 indicates successful load if (img.complete && img.naturalHeight !== 0) { clearInterval(timeoutChecker); handleSuccess(); @@ -640,6 +675,7 @@ Awful.applyTimeoutToLoadingImages = function() { } // If image failed to load (error state) + // img.complete true + naturalHeight === 0 indicates load failure if (img.complete && img.naturalHeight === 0) { clearInterval(timeoutChecker); handleFailure(); @@ -674,13 +710,13 @@ Awful.applyTimeoutToLoadingImages = function() { * Allows users to retry loading failed images. */ Awful.setupRetryHandler = function() { - // Prevent duplicate event listener registration (memory leak fix) - if (Awful.retryHandlerInitialized) { - return; // Already set up + // Remove old event listener if it exists (prevents memory leak on page re-render) + if (Awful.retryClickHandler) { + document.removeEventListener('click', Awful.retryClickHandler); } - Awful.retryHandlerInitialized = true; - document.addEventListener('click', function(event) { + // Define handler function and store reference for cleanup + Awful.retryClickHandler = function(event) { const retryLink = event.target; if (retryLink.hasAttribute('data-retry-image')) { event.preventDefault(); @@ -700,9 +736,10 @@ Awful.setupRetryHandler = function() { // Retry loading with feedback const retryWithFeedback = async function() { + let objectURL = null; try { const blob = await Awful.fetchWithTimeout(imageURL); - const objectURL = URL.createObjectURL(blob); + objectURL = URL.createObjectURL(blob); const successImg = document.createElement('img'); successImg.setAttribute('alt', ''); @@ -710,6 +747,7 @@ Awful.setupRetryHandler = function() { successImg.onload = () => { URL.revokeObjectURL(objectURL); + objectURL = null; // Mark as revoked // Replace the container with the successful image container.parentNode.replaceChild(successImg, container); // Remove the hidden test image @@ -721,10 +759,16 @@ Awful.setupRetryHandler = function() { // Handle decode failures on retry successImg.onerror = () => { URL.revokeObjectURL(objectURL); + objectURL = null; // Mark as revoked throw new Error("Image decode failed on retry"); }; } catch (error) { + // Ensure objectURL is revoked even on error + if (objectURL) { + URL.revokeObjectURL(objectURL); + } + // FAILED - restore retry button with "Failed" feedback console.error(`Retry failed: ${error.message} - ${imageURL}`); @@ -748,7 +792,10 @@ Awful.setupRetryHandler = function() { retryWithFeedback(); } } - }, { once: false }); + }; + + // Register the event listener with stored reference + document.addEventListener('click', Awful.retryClickHandler, { once: false }); }; Awful.tweetTheme = function() { diff --git a/App/Views/LoadingView.swift b/App/Views/LoadingView.swift index b929fb2d1..f721c60a4 100644 --- a/App/Views/LoadingView.swift +++ b/App/Views/LoadingView.swift @@ -147,9 +147,8 @@ private class DefaultLoadingView: LoadingView { if finished { // first animation complete! start second one and loop self?.animationView.play(fromFrame: 25, toFrame: .infinity, loopMode: .loop, completion: nil) - } else { - // animation cancelled } + // If not finished, animation was cancelled - no action needed }) } From 95a37ef784486e57ab6e3eb67750ebfff72061a1 Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Sat, 29 Nov 2025 14:16:06 +1100 Subject: [PATCH 06/13] Final code review? --- App/Misc/HTMLRenderingHelpers.swift | 1 + App/Resources/RenderView.js | 76 +++++++++++++++++++++++++---- App/Views/LoadingView.swift | 1 + 3 files changed, 68 insertions(+), 10 deletions(-) diff --git a/App/Misc/HTMLRenderingHelpers.swift b/App/Misc/HTMLRenderingHelpers.swift index e6aa38545..f7325e5bb 100644 --- a/App/Misc/HTMLRenderingHelpers.swift +++ b/App/Misc/HTMLRenderingHelpers.swift @@ -10,6 +10,7 @@ extension HTMLDocument { /// Number of post images to load immediately before deferring to lazy loading. /// First 10 images load right away for better perceived performance. + /// IMPORTANT: This value must match IMMEDIATELY_LOADED_IMAGE_COUNT in RenderView.js (line 17) private static let immediatelyLoadedImageCount = 10 /// 1x1 transparent GIF used as placeholder for lazy-loaded images diff --git a/App/Resources/RenderView.js b/App/Resources/RenderView.js index 1fd2f7ee7..4b0857ed9 100644 --- a/App/Resources/RenderView.js +++ b/App/Resources/RenderView.js @@ -13,6 +13,7 @@ if (!window.Awful) { // MARK: - Configuration Constants /// Number of post images to load immediately before lazy loading kicks in +/// IMPORTANT: This value must match immediatelyLoadedImageCount in HTMLRenderingHelpers.swift (line 13) const IMMEDIATELY_LOADED_IMAGE_COUNT = 10; /// How far ahead (in pixels) to start loading lazy content before it enters viewport @@ -54,18 +55,27 @@ const IMAGE_LOAD_TIMEOUT_CONFIG = { /** * Sets up a Lottie player to load ghost animation data. * Helper to avoid code duplication for dead tweet/image badge initialization. + * Properly removes existing listeners before adding new ones to prevent accumulation. * * @param {HTMLElement} container - The container element containing lottie-player elements */ Awful.setupGhostLottiePlayer = function(container) { const players = container.querySelectorAll(SELECTORS.LOTTIE_PLAYERS); players.forEach((lottiePlayer) => { - lottiePlayer.addEventListener("rendered", () => { + // Remove any existing listener before adding new one to prevent accumulation + if (lottiePlayer._ghostLoadHandler) { + lottiePlayer.removeEventListener("rendered", lottiePlayer._ghostLoadHandler); + } + + // Store handler reference for cleanup + lottiePlayer._ghostLoadHandler = () => { const ghostData = document.getElementById("ghost-json-data"); if (ghostData) { lottiePlayer.load(ghostData.innerText); } - }); + }; + + lottiePlayer.addEventListener("rendered", lottiePlayer._ghostLoadHandler); }); }; @@ -249,8 +259,8 @@ Awful.embedTweetNow = function(thisPostElement) { // Group tweet links by ID for deduplication const tweetIDsToLinks = {}; Array.prototype.forEach.call(tweetLinks, function(a) { - // Skip tweets with NWS content - if (a.parentElement.querySelector('img.awful-smile[title=":nws:"]')) { + // Skip tweets with NWS content (use optional chaining to avoid null reference errors) + if (a.parentElement?.querySelector('img.awful-smile[title=":nws:"]')) { return; } const tweetID = a.dataset.tweetId; @@ -263,10 +273,13 @@ Awful.embedTweetNow = function(thisPostElement) { // Fetch and embed each unique tweet Object.keys(tweetIDsToLinks).forEach(function(tweetID) { const callback = `jsonp_callback_${tweetID}`; + // Validate theme is an expected value to prevent URL injection const tweetTheme = Awful.tweetTheme(); + const validThemes = ['light', 'dark']; + const safeTheme = validThemes.includes(tweetTheme) ? tweetTheme : 'light'; const script = document.createElement('script'); - script.src = `https://api.twitter.com/1/statuses/oembed.json?id=${tweetID}&omit_script=true&dnt=true&theme=${tweetTheme}&callback=${callback}`; + script.src = `https://api.twitter.com/1/statuses/oembed.json?id=${tweetID}&omit_script=true&dnt=true&theme=${safeTheme}&callback=${callback}`; window[callback] = function(data) { cleanUp(script); @@ -372,7 +385,9 @@ Awful.embedBlueskyPosts = function() { * Also sets up Lottie animation play/pause for ghost tweets in the viewport. */ Awful.embedTweets = function() { - // Prevent concurrent setup (race condition protection) + // Prevent concurrent setup to avoid race conditions where multiple calls could + // create duplicate observers and listeners. The flag is reset in the finally block + // to ensure it's always cleared even if errors occur. if (Awful.embedTweetsInProgress) { return; } @@ -562,7 +577,9 @@ Awful.setupImageLazyLoading = function() { const lazySrc = img.dataset.lazySrc; if (lazySrc) { - // Skip attachment.php files (defensive check, shouldn't be lazy-loaded) + // Skip attachment.php files - these require authentication and are handled by the native + // ImageURLProtocol. Loading them through fetchWithTimeout would fail auth checks. + // This is a defensive check; attachment.php images shouldn't be lazy-loaded in the first place. if (lazySrc.includes('attachment.php')) { delete img.dataset.lazySrc; img.src = lazySrc; // Load normally without timeout @@ -613,7 +630,10 @@ Awful.applyTimeoutToLoadingImages = function() { Awful.imageLoadTracker.initialize(totalImages); // Store interval timers for cleanup (prevents memory leaks) - // Reset array on each call to prevent memory leak across page reloads + // Clear all existing timers before resetting array to prevent orphaned intervals + if (Awful.imageTimeoutCheckers) { + Awful.imageTimeoutCheckers.forEach(timer => clearInterval(timer)); + } Awful.imageTimeoutCheckers = []; initialImages.forEach((img, index) => { @@ -635,13 +655,19 @@ Awful.applyTimeoutToLoadingImages = function() { let handled = false; const handleSuccess = () => { - if (handled) return; + if (handled) { + console.warn(`[Image Load] Duplicate success event for ${imageID} (already handled)`); + return; + } handled = true; Awful.imageLoadTracker.incrementLoaded(); }; const handleFailure = () => { - if (handled) return; + if (handled) { + console.warn(`[Image Load] Duplicate failure event for ${imageID} (already handled)`); + return; + } handled = true; if (enableGhost && img.parentNode) { @@ -798,6 +824,36 @@ Awful.setupRetryHandler = function() { document.addEventListener('click', Awful.retryClickHandler, { once: false }); }; +/** + * Cleanup function to remove retry click handler and prevent memory leaks. + * Should be called when the view is destroyed or navigating away from the page. + */ +Awful.cleanupRetryHandler = function() { + if (Awful.retryClickHandler) { + document.removeEventListener('click', Awful.retryClickHandler); + Awful.retryClickHandler = null; + } +}; + +/** + * Cleanup function to disconnect all IntersectionObservers and prevent memory leaks. + * Should be called when the view is destroyed or navigating away from the page. + */ +Awful.cleanupObservers = function() { + if (Awful.ghostLottieObserver) { + Awful.ghostLottieObserver.disconnect(); + Awful.ghostLottieObserver = null; + } + if (Awful.tweetLazyLoadObserver) { + Awful.tweetLazyLoadObserver.disconnect(); + Awful.tweetLazyLoadObserver = null; + } + if (Awful.imageLazyLoadObserver) { + Awful.imageLazyLoadObserver.disconnect(); + Awful.imageLazyLoadObserver = null; + } +}; + Awful.tweetTheme = function() { return document.body.dataset.tweetTheme; } diff --git a/App/Views/LoadingView.swift b/App/Views/LoadingView.swift index f721c60a4..ec506bdb9 100644 --- a/App/Views/LoadingView.swift +++ b/App/Views/LoadingView.swift @@ -128,6 +128,7 @@ private class DefaultLoadingView: LoadingView { animationView.widthAnchor.constraint(equalToConstant: 90), animationView.heightAnchor.constraint(equalToConstant: 90), animationView.centerXAnchor.constraint(equalTo: centerXAnchor), + // Center animation slightly above true center for better visual balance with status text below animationView.centerYAnchor.constraint(equalTo: centerYAnchor, constant: -40), // Status label below animation From b8e16b00077106f0079b3cf4e27dd3b9bbc096d1 Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Sat, 29 Nov 2025 14:35:02 +1100 Subject: [PATCH 07/13] Last code review completed. --- App/Misc/HTMLRenderingHelpers.swift | 2 +- App/Resources/RenderView.js | 24 ++++++++++++++++-------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/App/Misc/HTMLRenderingHelpers.swift b/App/Misc/HTMLRenderingHelpers.swift index f7325e5bb..c6f389695 100644 --- a/App/Misc/HTMLRenderingHelpers.swift +++ b/App/Misc/HTMLRenderingHelpers.swift @@ -10,7 +10,7 @@ extension HTMLDocument { /// Number of post images to load immediately before deferring to lazy loading. /// First 10 images load right away for better perceived performance. - /// IMPORTANT: This value must match IMMEDIATELY_LOADED_IMAGE_COUNT in RenderView.js (line 17) + /// IMPORTANT: This value must match IMMEDIATELY_LOADED_IMAGE_COUNT constant in RenderView.js private static let immediatelyLoadedImageCount = 10 /// 1x1 transparent GIF used as placeholder for lazy-loaded images diff --git a/App/Resources/RenderView.js b/App/Resources/RenderView.js index 4b0857ed9..5a31067eb 100644 --- a/App/Resources/RenderView.js +++ b/App/Resources/RenderView.js @@ -489,15 +489,9 @@ Awful.imageLoadTracker = { total: 0, initialize: function(totalCount) { - // Guard against zero-image case (prevents misleading progress reports) - if (totalCount === 0) { - this.loaded = 0; - this.total = 0; - return; // Don't report progress for pages with no images - } this.loaded = 0; this.total = totalCount; - this.reportProgress(); + this.reportProgress(); // Always report - Swift side handles zero case correctly }, incrementLoaded: function() { @@ -640,7 +634,7 @@ Awful.applyTimeoutToLoadingImages = function() { const imageID = `img-init-${index}`; const imageURL = img.src; - // Note: attachment.php and data: URLs already filtered out above (line 565-567) + // Note: attachment.php and data: URLs already filtered out in initialImages filter above // Skip if already loaded (but count it) // Note: img.complete is true for both successfully loaded AND failed images @@ -835,6 +829,17 @@ Awful.cleanupRetryHandler = function() { } }; +/** + * Cleanup function to clear all image timeout interval timers. + * Prevents timers from running after page navigation or view destruction. + */ +Awful.cleanupImageTimers = function() { + if (Awful.imageTimeoutCheckers) { + Awful.imageTimeoutCheckers.forEach(timer => clearInterval(timer)); + Awful.imageTimeoutCheckers = []; + } +}; + /** * Cleanup function to disconnect all IntersectionObservers and prevent memory leaks. * Should be called when the view is destroyed or navigating away from the page. @@ -852,6 +857,9 @@ Awful.cleanupObservers = function() { Awful.imageLazyLoadObserver.disconnect(); Awful.imageLazyLoadObserver = null; } + + // Also cleanup image timers + Awful.cleanupImageTimers(); }; Awful.tweetTheme = function() { From 3df184abaf76379e82cb12c9b349ffbefcf6d44b Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Sat, 29 Nov 2025 14:44:14 +1100 Subject: [PATCH 08/13] Cleaned up silly comments --- App/Misc/HTMLRenderingHelpers.swift | 1 - App/Resources/RenderView.js | 16 +--------------- .../Posts/PostsPageViewController.swift | 2 +- App/Views/LoadingView.swift | 15 --------------- 4 files changed, 2 insertions(+), 32 deletions(-) diff --git a/App/Misc/HTMLRenderingHelpers.swift b/App/Misc/HTMLRenderingHelpers.swift index c6f389695..e7023b975 100644 --- a/App/Misc/HTMLRenderingHelpers.swift +++ b/App/Misc/HTMLRenderingHelpers.swift @@ -9,7 +9,6 @@ extension HTMLDocument { // MARK: - Constants /// Number of post images to load immediately before deferring to lazy loading. - /// First 10 images load right away for better perceived performance. /// IMPORTANT: This value must match IMMEDIATELY_LOADED_IMAGE_COUNT constant in RenderView.js private static let immediatelyLoadedImageCount = 10 diff --git a/App/Resources/RenderView.js b/App/Resources/RenderView.js index 5a31067eb..0a20a5319 100644 --- a/App/Resources/RenderView.js +++ b/App/Resources/RenderView.js @@ -62,12 +62,10 @@ const IMAGE_LOAD_TIMEOUT_CONFIG = { Awful.setupGhostLottiePlayer = function(container) { const players = container.querySelectorAll(SELECTORS.LOTTIE_PLAYERS); players.forEach((lottiePlayer) => { - // Remove any existing listener before adding new one to prevent accumulation if (lottiePlayer._ghostLoadHandler) { lottiePlayer.removeEventListener("rendered", lottiePlayer._ghostLoadHandler); } - // Store handler reference for cleanup lottiePlayer._ghostLoadHandler = () => { const ghostData = document.getElementById("ghost-json-data"); if (ghostData) { @@ -430,13 +428,8 @@ Awful.embedTweets = function() { } // Image lazy loading (works regardless of ghost feature being enabled) - // Apply timeout detection to initial images (first 10) Awful.applyTimeoutToLoadingImages(); - - // Setup lazy loading for deferred images (11+) Awful.setupImageLazyLoading(); - - // Setup retry handler Awful.setupRetryHandler(); // Set up lazy-loading IntersectionObserver for tweet embeds @@ -620,10 +613,8 @@ Awful.applyTimeoutToLoadingImages = function() { ); const totalImages = initialImages.length; - // Initialize progress tracker (only tracks first 10 images, not lazy-loaded ones) Awful.imageLoadTracker.initialize(totalImages); - // Store interval timers for cleanup (prevents memory leaks) // Clear all existing timers before resetting array to prevent orphaned intervals if (Awful.imageTimeoutCheckers) { Awful.imageTimeoutCheckers.forEach(timer => clearInterval(timer)); @@ -634,17 +625,13 @@ Awful.applyTimeoutToLoadingImages = function() { const imageID = `img-init-${index}`; const imageURL = img.src; - // Note: attachment.php and data: URLs already filtered out in initialImages filter above - - // Skip if already loaded (but count it) - // Note: img.complete is true for both successfully loaded AND failed images + // img.complete is true for both successfully loaded AND failed images // We discriminate using naturalHeight: >0 means success, ===0 means failure if (img.complete && img.naturalHeight !== 0) { Awful.imageLoadTracker.incrementLoaded(); return; } - // Let browser load the image naturally, but monitor for timeout // Track if we've already handled this image to prevent double-counting let handled = false; @@ -858,7 +845,6 @@ Awful.cleanupObservers = function() { Awful.imageLazyLoadObserver = null; } - // Also cleanup image timers Awful.cleanupImageTimers(); }; diff --git a/App/View Controllers/Posts/PostsPageViewController.swift b/App/View Controllers/Posts/PostsPageViewController.swift index 54dbde6c8..25098c6cb 100644 --- a/App/View Controllers/Posts/PostsPageViewController.swift +++ b/App/View Controllers/Posts/PostsPageViewController.swift @@ -1781,7 +1781,7 @@ extension PostsPageViewController: RenderViewDelegate { } // Note: Loading view is now dismissed when image loading completes (via ImageLoadProgress message) - // or when user taps "Show Now" button + // or when user taps (X) button } func didReceive(message: RenderViewMessage, in view: RenderView) { diff --git a/App/Views/LoadingView.swift b/App/Views/LoadingView.swift index ec506bdb9..f299af48f 100644 --- a/App/Views/LoadingView.swift +++ b/App/Views/LoadingView.swift @@ -46,23 +46,8 @@ class LoadingView: UIView { } } - /// Callback invoked when user dismisses the loading view via the X button. - /// - /// Use weak/unowned captures to avoid retain cycles: - /// ```swift - /// loadingView.onDismiss = { [weak self] in - /// self?.handleDismissal() - /// } - /// ``` var onDismiss: (() -> Void)? - /// Updates the status text displayed in the loading view. - /// - /// This method should be overridden in subclasses to implement status updates. - /// The default implementation does nothing. Only the default loading view theme - /// currently supports status updates. - /// - /// - Parameter text: The status message to display func updateStatus(_ text: String) { // Override in subclasses } From 0d2c7eddff90d6c388a476c6541edf455987c0e8 Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Sat, 29 Nov 2025 14:56:17 +1100 Subject: [PATCH 09/13] Fix excessive logging for imageLoadProgress --- App/Views/RenderView.swift | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/App/Views/RenderView.swift b/App/Views/RenderView.swift index 7bcc8cffd..b075df08c 100644 --- a/App/Views/RenderView.swift +++ b/App/Views/RenderView.swift @@ -199,7 +199,10 @@ extension RenderView: WKScriptMessageHandler { } func userContentController(_ userContentController: WKUserContentController, didReceive rawMessage: WKScriptMessage) { - logger.debug("received message from JavaScript: \(rawMessage.name)") + // Skip logging high-frequency progress updates to reduce console noise + if rawMessage.name != "imageLoadProgress" { + logger.debug("received message from JavaScript: \(rawMessage.name)") + } guard let messageType = registeredMessages[rawMessage.name] else { logger.warning("ignoring unexpected message from JavaScript: \(rawMessage.name). Did you forget to register a message type with the RenderView?") From 651b62fe241d706699c5cd736bfd0c7d4c9b7c43 Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Sat, 29 Nov 2025 16:23:46 +1100 Subject: [PATCH 10/13] Removing IntersectionObserver method for manually lazy loading images. This can be done by the browser as iOS15.4+ now accepts the loading="lazy" attribute. Tweets continue to use intersection method. If an image fails to download during a lazyload, replace with Dead Image ghost badge. --- App/Misc/HTMLRenderingHelpers.swift | 6 +- App/Resources/RenderView.js | 303 ++++++---------------------- 2 files changed, 63 insertions(+), 246 deletions(-) diff --git a/App/Misc/HTMLRenderingHelpers.swift b/App/Misc/HTMLRenderingHelpers.swift index e7023b975..dc25ef8c6 100644 --- a/App/Misc/HTMLRenderingHelpers.swift +++ b/App/Misc/HTMLRenderingHelpers.swift @@ -185,9 +185,9 @@ extension HTMLDocument { postContentImageCount += 1 if postContentImageCount > Self.immediatelyLoadedImageCount { - // Defer loading for images beyond the immediately loaded count - img["data-lazy-src"] = finalURL - img["src"] = Self.transparentPixelPlaceholder + // Defer loading for images beyond the immediately loaded count (browser handles lazy loading) + img["loading"] = "lazy" + img["src"] = finalURL } else { // Load immediately img["src"] = finalURL diff --git a/App/Resources/RenderView.js b/App/Resources/RenderView.js index 0a20a5319..3b151c29f 100644 --- a/App/Resources/RenderView.js +++ b/App/Resources/RenderView.js @@ -13,7 +13,7 @@ if (!window.Awful) { // MARK: - Configuration Constants /// Number of post images to load immediately before lazy loading kicks in -/// IMPORTANT: This value must match immediatelyLoadedImageCount in HTMLRenderingHelpers.swift (line 13) +/// IMPORTANT: This value must match immediatelyLoadedImageCount constant in HTMLRenderingHelpers.swift const IMMEDIATELY_LOADED_IMAGE_COUNT = 10; /// How far ahead (in pixels) to start loading lazy content before it enters viewport @@ -21,26 +21,13 @@ const LAZY_LOAD_LOOKAHEAD_DISTANCE = '600px'; /// CSS selectors used throughout the code const SELECTORS = { - LAZY_IMAGE: 'img[data-lazy-src]', - LOADING_IMAGES: 'section.postbody img[src]:not(.awful-smile):not(.awful-avatar):not([data-lazy-src])', + LOADING_IMAGES: 'section.postbody img[src]:not(.awful-smile):not(.awful-avatar):not([loading="lazy"])', POST_ELEMENTS: 'post', LOTTIE_PLAYERS: 'lottie-player' }; -/// Timeout configuration for image loading with progress detection +/// Timeout configuration for image loading const IMAGE_LOAD_TIMEOUT_CONFIG = { - /// Milliseconds to wait for initial connection before aborting (no bytes received) - /// 1000ms gives slow connections a chance while quickly failing on unreachable hosts - connectionTimeout: 1000, - - /// Milliseconds to wait during download if progress stalls - /// 2500ms accommodates network hiccups without excessive waiting on stalled downloads - downloadStallTimeout: 2500, - - /// Milliseconds between progress checks during download - /// 500ms balances responsiveness with minimal performance overhead - progressCheckInterval: 500, - /// Maximum number of checks for initial image loading timeout detection /// 3 checks × 1000ms = 3 seconds max wait for images to start loading maxImageChecks: 3, @@ -116,74 +103,6 @@ Awful.escapeHTML = function(str) { return div.innerHTML; }; -/** - * Fetches a URL with smart timeout detection that monitors download progress. - * Times out only when connection fails to start or download stalls. - * - * @param {string} url - The URL to fetch - * @param {Object} config - Optional timeout configuration (uses IMAGE_LOAD_TIMEOUT_CONFIG by default) - * @returns {Promise} The fetched content as a Blob - * @throws {Error} If connection times out, download stalls, or HTTP error occurs - */ -Awful.fetchWithTimeout = async function(url, config = IMAGE_LOAD_TIMEOUT_CONFIG) { - const connectionTimeout = config.connectionTimeout; - const downloadStallTimeout = config.downloadStallTimeout; - const progressCheckInterval = config.progressCheckInterval; - const MAX_TOTAL_TIMEOUT = 30000; // 30 seconds absolute maximum - - const controller = new AbortController(); - let lastProgressTime = Date.now(); - const startTime = Date.now(); - let totalBytesReceived = 0; - let progressCheckTimer = null; - - // Monitor download progress and abort if stalled - progressCheckTimer = setInterval(() => { - const timeSinceLastProgress = Date.now() - lastProgressTime; - - if (totalBytesReceived === 0 && timeSinceLastProgress > connectionTimeout) { - // No connection established within timeout - clearInterval(progressCheckTimer); - controller.abort(); - } else if (totalBytesReceived > 0 && timeSinceLastProgress > downloadStallTimeout) { - // Download started but has stalled - clearInterval(progressCheckTimer); - controller.abort(); - } - }, progressCheckInterval); - - try { - const response = await fetch(url, { signal: controller.signal }); - - if (!response.ok) { - throw new Error(`HTTP ${response.status}`); - } - - const reader = response.body.getReader(); - const chunks = []; - - while (true) { - // Check absolute timeout to prevent indefinite hangs - if (Date.now() - startTime > MAX_TOTAL_TIMEOUT) { - throw new Error('Total timeout exceeded'); - } - - const { done, value } = await reader.read(); - if (done) break; - - chunks.push(value); - totalBytesReceived += value.length; - lastProgressTime = Date.now(); - } - - return new Blob(chunks); - - } finally { - // Always clean up timer, even if an error occurs - clearInterval(progressCheckTimer); - } -}; - /** * Helper for consistent error handling when images fail to load. * Replaces failed images with dead image badges and optionally updates progress tracker. @@ -427,10 +346,10 @@ Awful.embedTweets = function() { }); } - // Image lazy loading (works regardless of ghost feature being enabled) + // Image loading and retry handling (works regardless of ghost feature being enabled) Awful.applyTimeoutToLoadingImages(); - Awful.setupImageLazyLoading(); Awful.setupRetryHandler(); + Awful.setupLazyImageErrorHandling(); // Set up lazy-loading IntersectionObserver for tweet embeds // Tweets are loaded before entering the viewport based on LAZY_LOAD_LOOKAHEAD_DISTANCE @@ -503,100 +422,6 @@ Awful.imageLoadTracker = { } }; -/** - * Loads an image with smart timeout detection. - * Monitors download progress and only times out stalled connections. - * - * @param {string} url - The image URL to load - * @param {HTMLImageElement} img - The image element - * @param {string} imageID - Unique ID for this image - * @param {boolean} enableGhost - Whether to show dead image badge on failure - * @param {boolean} trackProgress - Whether to track progress (default: false, only for initial images) - */ -Awful.loadImageWithProgressDetection = async function(url, img, imageID, enableGhost, trackProgress = false) { - try { - const blob = await Awful.fetchWithTimeout(url); - const objectURL = URL.createObjectURL(blob); - img.src = objectURL; - - // Clean up object URL after image loads - img.onload = () => { - URL.revokeObjectURL(objectURL); - // Only track progress for initial images, not lazy-loaded ones - if (trackProgress) { - Awful.imageLoadTracker.incrementLoaded(); - } - }; - - // Handle image decode failures (corrupt image data) - img.onerror = () => { - URL.revokeObjectURL(objectURL); - const decodeError = new Error("Image decode failed"); - Awful.handleImageLoadError(decodeError, url, img, imageID, enableGhost, trackProgress); - }; - - } catch (error) { - // Handle fetch/network failures - Awful.handleImageLoadError(error, url, img, imageID, enableGhost, trackProgress); - } -}; - -/** - * Lazy loads post content images using IntersectionObserver (for images 11+). - * Images load as they approach the viewport with a lookahead distance. - */ -Awful.setupImageLazyLoading = function() { - const enableGhost = Awful.renderGhostTweets || false; - - // Disconnect previous observer if it exists (prevents memory leak on re-render) - if (Awful.imageLazyLoadObserver) { - Awful.imageLazyLoadObserver.disconnect(); - } - - // Reset sequential counter for lazy image IDs (prevents ID collisions on page reload) - Awful.lazyImageIDCounter = 0; - - // IntersectionObserver with lookahead (same as tweets) - Awful.imageLazyLoadObserver = new IntersectionObserver(function(entries) { - entries.forEach(entry => { - if (entry.isIntersecting) { - const img = entry.target; - const lazySrc = img.dataset.lazySrc; - - if (lazySrc) { - // Skip attachment.php files - these require authentication and are handled by the native - // ImageURLProtocol. Loading them through fetchWithTimeout would fail auth checks. - // This is a defensive check; attachment.php images shouldn't be lazy-loaded in the first place. - if (lazySrc.includes('attachment.php')) { - delete img.dataset.lazySrc; - img.src = lazySrc; // Load normally without timeout - Awful.imageLazyLoadObserver.unobserve(img); - return; - } - - // Use sequential counter for guaranteed unique IDs - const imageID = `lazy-${Awful.lazyImageIDCounter++}`; - delete img.dataset.lazySrc; - - // Load with smart timeout detection - // trackProgress = false because lazy images don't count toward progress - Awful.loadImageWithProgressDetection(lazySrc, img, imageID, enableGhost, false); - - Awful.imageLazyLoadObserver.unobserve(img); - } - } - }); - }, { - rootMargin: `${LAZY_LOAD_LOOKAHEAD_DISTANCE} 0px`, - threshold: 0.01 - }); - - // Observe all lazy-loadable images - document.querySelectorAll(SELECTORS.LAZY_IMAGE).forEach(img => { - Awful.imageLazyLoadObserver.observe(img); - }); -}; - /** * Apply timeout detection to images that are loading normally (first 10). * Monitors initial image loading and tracks progress for the loading view. @@ -604,7 +429,7 @@ Awful.setupImageLazyLoading = function() { Awful.applyTimeoutToLoadingImages = function() { const enableGhost = Awful.renderGhostTweets || false; - // Find images with real src (not data-lazy-src) - these are the first 10 images + // Find post content images (excluding smilies, avatars, and lazy-loaded images) - these are the first 10 images const loadingImages = document.querySelectorAll(SELECTORS.LOADING_IMAGES); // Count only the initially loading images (first 10), excluding attachment.php and data URLs @@ -736,67 +561,34 @@ Awful.setupRetryHandler = function() { retryLink.textContent = 'Retrying...'; retryLink.style.pointerEvents = 'none'; // Disable clicking during retry - // Create hidden image element to test loading - const img = document.createElement('img'); - img.style.display = 'none'; - container.parentNode.insertBefore(img, container); - - // Retry loading with feedback - const retryWithFeedback = async function() { - let objectURL = null; - try { - const blob = await Awful.fetchWithTimeout(imageURL); - objectURL = URL.createObjectURL(blob); - - const successImg = document.createElement('img'); - successImg.setAttribute('alt', ''); - successImg.src = objectURL; - - successImg.onload = () => { - URL.revokeObjectURL(objectURL); - objectURL = null; // Mark as revoked - // Replace the container with the successful image - container.parentNode.replaceChild(successImg, container); - // Remove the hidden test image - if (img.parentNode) { - img.parentNode.removeChild(img); - } - }; - - // Handle decode failures on retry - successImg.onerror = () => { - URL.revokeObjectURL(objectURL); - objectURL = null; // Mark as revoked - throw new Error("Image decode failed on retry"); - }; - - } catch (error) { - // Ensure objectURL is revoked even on error - if (objectURL) { - URL.revokeObjectURL(objectURL); - } + // Create new image element with native browser loading + const successImg = document.createElement('img'); + successImg.setAttribute('alt', ''); - // FAILED - restore retry button with "Failed" feedback - console.error(`Retry failed: ${error.message} - ${imageURL}`); + // Handle successful load + successImg.addEventListener('load', () => { + // Replace the dead badge container with the successful image + container.parentNode.replaceChild(successImg, container); + }, { once: true }); - retryLink.textContent = 'Retry Failed - Try Again'; - retryLink.style.pointerEvents = 'auto'; // Re-enable clicking + // Handle load failure + successImg.addEventListener('error', (error) => { + // FAILED - restore retry button with "Failed" feedback + console.error(`Retry failed: ${error.message || 'Unknown error'} - ${imageURL}`); - // Remove the hidden test image - if (img.parentNode) { - img.parentNode.removeChild(img); - } + retryLink.textContent = 'Retry Failed - Try Again'; + retryLink.style.pointerEvents = 'auto'; // Re-enable clicking - // Reset to just "Retry" after configured delay - setTimeout(() => { - if (retryLink.textContent === 'Retry Failed - Try Again') { - retryLink.textContent = 'Retry'; - } - }, IMAGE_LOAD_TIMEOUT_CONFIG.retryResetDelay); - } - }; + // Reset to just "Retry" after configured delay + setTimeout(() => { + if (retryLink.textContent === 'Retry Failed - Try Again') { + retryLink.textContent = 'Retry'; + } + }, IMAGE_LOAD_TIMEOUT_CONFIG.retryResetDelay); + }, { once: true }); - retryWithFeedback(); + // Start loading (native browser handles everything) + successImg.src = imageURL; } } }; @@ -816,6 +608,35 @@ Awful.cleanupRetryHandler = function() { } }; +/** + * Sets up error handling for lazy-loaded images (those with loading="lazy" attribute). + * Attaches error event listeners that display dead image badges when browser attempts + * to load the image and it fails (404, broken, etc.). Only triggers after browser + * attempts load - doesn't interfere with native lazy loading. + */ +Awful.setupLazyImageErrorHandling = function() { + const enableGhost = Awful.renderGhostTweets || false; + const lazyImages = document.querySelectorAll('section.postbody img[loading="lazy"]'); + + lazyImages.forEach((img, index) => { + const imageID = `lazy-error-${index}`; + + // Only attach error listener - don't interfere with lazy loading + img.addEventListener('error', function() { + // Browser attempted to load this image and it failed + const imageURL = img.src; + Awful.handleImageLoadError( + new Error("Lazy image load failed"), + imageURL, + img, + imageID, + enableGhost, + false // trackProgress = false, lazy images don't count toward progress + ); + }, { once: true }); + }); +}; + /** * Cleanup function to clear all image timeout interval timers. * Prevents timers from running after page navigation or view destruction. @@ -840,10 +661,6 @@ Awful.cleanupObservers = function() { Awful.tweetLazyLoadObserver.disconnect(); Awful.tweetLazyLoadObserver = null; } - if (Awful.imageLazyLoadObserver) { - Awful.imageLazyLoadObserver.disconnect(); - Awful.imageLazyLoadObserver = null; - } Awful.cleanupImageTimers(); }; @@ -1616,15 +1433,15 @@ Awful.embedGfycat(); if (Awful.domContentLoadedFired) { if (typeof Awful.applyTimeoutToLoadingImages === 'function') { Awful.applyTimeoutToLoadingImages(); - Awful.setupImageLazyLoading(); Awful.setupRetryHandler(); + Awful.setupLazyImageErrorHandling(); } } else { document.addEventListener('DOMContentLoaded', function() { if (typeof Awful.applyTimeoutToLoadingImages === 'function') { Awful.applyTimeoutToLoadingImages(); - Awful.setupImageLazyLoading(); Awful.setupRetryHandler(); + Awful.setupLazyImageErrorHandling(); } }); } From f7aa218b8a588cf7d5a190e233bff0a58aaaa92d Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Sun, 30 Nov 2025 11:19:23 +1100 Subject: [PATCH 11/13] PR cleanup commit --- App/Misc/HTMLRenderingHelpers.swift | 7 +++++-- App/Resources/RenderView.js | 2 +- App/Views/RenderView.swift | 9 --------- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/App/Misc/HTMLRenderingHelpers.swift b/App/Misc/HTMLRenderingHelpers.swift index dc25ef8c6..8446fa055 100644 --- a/App/Misc/HTMLRenderingHelpers.swift +++ b/App/Misc/HTMLRenderingHelpers.swift @@ -179,8 +179,11 @@ extension HTMLDocument { finalURL = waffleURL.absoluteString } + // Check if this is a data: URI (inline data that shouldn't be lazy-loaded) + let isDataURI = finalURL.starts(with: "data:") + // Determine whether to load immediately or defer based on image type and count - if !isAvatar && !isAttachment { + if !isAvatar && !isAttachment && !isDataURI { // This is a post content image (not avatar, not smilie, not attachment) postContentImageCount += 1 @@ -193,7 +196,7 @@ extension HTMLDocument { img["src"] = finalURL } } else { - // Avatars and attachments always load immediately + // Avatars, attachments, and data URIs always load immediately img["src"] = finalURL } } diff --git a/App/Resources/RenderView.js b/App/Resources/RenderView.js index 3b151c29f..507076e5b 100644 --- a/App/Resources/RenderView.js +++ b/App/Resources/RenderView.js @@ -221,7 +221,7 @@ Awful.embedTweetNow = function(thisPostElement) { cleanUp(this); console.error(`The embed markup for tweet ${tweetID} failed to load`); - // Replace failed tweets with ghost Lottie animation + // when a tweet errors out, insert a floating ghost lottie in somber rememberence of the tweet that used to be if (enableGhost) { tweetIDsToLinks[tweetID].forEach(function(a) { if (a.parentNode) { diff --git a/App/Views/RenderView.swift b/App/Views/RenderView.swift index b075df08c..a353f29a9 100644 --- a/App/Views/RenderView.swift +++ b/App/Views/RenderView.swift @@ -403,15 +403,6 @@ extension RenderView { evalAwfulFunction("applyTimeoutToLoadingImages") } - /// Sets up lazy loading for deferred images (images 11+). - /// - /// Uses IntersectionObserver to load images as they approach the viewport. - /// Images are loaded approximately 600px before they would be visible to the user - /// for a seamless scrolling experience. - func setupImageLazyLoading() { - evalAwfulFunction("setupImageLazyLoading") - } - /// Sets up click handler for retry links on failed images. /// /// Allows users to retry loading images that timed out or failed to load. From df52e0551145c1735056ae6d295a296e6ed88e6c Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Mon, 15 Dec 2025 19:29:38 +1100 Subject: [PATCH 12/13] Fixed issue where some tweets were not being embedded but quoted ones were (race condition between twitter.js widget being loaded and embed processing taking place). Added the retry option to Dead Tweet badge, similar to Dead Image badge --- App/Resources/RenderView.js | 374 ++++++++++++++++++++++++++++++------ 1 file changed, 310 insertions(+), 64 deletions(-) diff --git a/App/Resources/RenderView.js b/App/Resources/RenderView.js index 507076e5b..922444fd6 100644 --- a/App/Resources/RenderView.js +++ b/App/Resources/RenderView.js @@ -37,6 +37,10 @@ const IMAGE_LOAD_TIMEOUT_CONFIG = { retryResetDelay: 3000 }; +/// Timeout for tweet embedding via Twitter API +/// 5 seconds gives enough time for API response while preventing indefinite waiting +const TWEET_EMBED_TIMEOUT = 5000; + // MARK: - Utility Functions /** @@ -156,6 +160,138 @@ Awful.fetchOEmbed = async function(url) { }); }; +// MARK: - Tweet Embedding Helper Functions + +/** + * Shows a dead tweet badge for a failed tweet. + * Centralizes the logic for displaying dead tweet badges to avoid code duplication + * between main embedding and retry functionality. + * + * @param {string} tweetID - The tweet ID + * @param {string} tweetURL - The original tweet URL + * @param {Element} containerToReplace - The element to replace with dead badge + */ +Awful.showDeadTweetBadge = function(tweetID, tweetURL, containerToReplace) { + if (!window.Awful.renderGhostTweets || !containerToReplace || !containerToReplace.parentNode) { + return; + } + + const div = document.createElement('div'); + div.classList.add('dead-tweet-container'); + div.innerHTML = Awful.deadTweetBadgeHTML(tweetURL, tweetID); + containerToReplace.parentNode.replaceChild(div, containerToReplace); + Awful.setupGhostLottiePlayer(div); +}; + +/** + * Calls Twitter widgets.load() with proper race condition handling. + * Checks if widgets.js is fully loaded (has widgets property), otherwise queues the call + * via twttr.ready() to ensure it executes after widgets.js finishes loading. + */ +Awful.loadTwitterWidgetsForEmbeds = function() { + if (!window.twttr) { + return; + } + + if (window.twttr.widgets) { + // Real widgets.js is loaded (has widgets property), call immediately + twttr.widgets.load(); + } else { + // widgets.js still loading (only has stub), queue the call + twttr.ready(function() { + twttr.widgets.load(); + }); + } +}; + +/** + * Fetches a single tweet via JSONP with timeout protection and comprehensive error handling. + * Centralizes all tweet fetching logic to ensure consistent behavior between main embedding + * and retry functionality. + * + * @param {string} tweetID - The tweet ID to fetch + * @param {Function} onSuccess - Called with (data, tweetID) on successful fetch + * @param {Function} onFailure - Called with (reason, tweetID, data) on failure + * reason can be: 'timeout', 'api_error', 'network' + * @returns {object} - Object with cleanup() function to manually abort the request + */ +Awful.fetchTweetOEmbed = function(tweetID, onSuccess, onFailure) { + // Create unique callback name to prevent collisions when same tweet appears in multiple posts + let callback = `jsonp_callback_${tweetID}`; + let counter = 0; + while (window[callback]) { + callback = `jsonp_callback_${tweetID}_${++counter}`; + } + + const script = document.createElement('script'); + const tweetTheme = Awful.tweetTheme(); + const validThemes = ['light', 'dark']; + const safeTheme = validThemes.includes(tweetTheme) ? tweetTheme : 'light'; + script.src = `https://api.twitter.com/1/statuses/oembed.json?id=${tweetID}&omit_script=true&dnt=true&theme=${safeTheme}&callback=${callback}`; + + let timedOut = false; + + let timeoutId = setTimeout(function() { + timedOut = true; + cleanUp(script); + console.error(`Tweet ${tweetID} embedding timed out after ${TWEET_EMBED_TIMEOUT}ms`); + if (onFailure) { + onFailure('timeout', tweetID); + } + }, TWEET_EMBED_TIMEOUT); + + // Track timeout for global cleanup on page unload + if (!Awful.tweetEmbedTimeouts) { + Awful.tweetEmbedTimeouts = []; + } + Awful.tweetEmbedTimeouts.push(timeoutId); + + window[callback] = function(data) { + if (timedOut) { + console.warn(`Ignoring late response for tweet ${tweetID}`); + return; + } + + clearTimeout(timeoutId); + cleanUp(script); + + // Validate response - check for data existence but don't inspect HTML content (iframe issues) + if (!data || !data.html || data.error) { + console.error(`Tweet ${tweetID} API returned error:`, data ? data.error : 'No data'); + if (onFailure) { + onFailure('api_error', tweetID, data); + } + return; + } + + // Success + if (onSuccess) { + onSuccess(data, tweetID); + } + }; + + script.onerror = function() { + if (timedOut) return; + cleanUp(this); + console.error(`Tweet ${tweetID} network error`); + if (onFailure) { + onFailure('network', tweetID); + } + }; + + function cleanUp(script) { + clearTimeout(timeoutId); + delete window[callback]; + if (script.parentNode) { + script.parentNode.removeChild(script); + } + } + + document.body.appendChild(script); + + return { cleanup: function() { cleanUp(script); } }; +}; + /** * Embeds tweets within a specific post element using Twitter's OEmbed API. * Called by IntersectionObserver when a post enters the viewport. @@ -163,45 +299,64 @@ Awful.fetchOEmbed = async function(url) { * @param {Element} thisPostElement - The post element to process for tweet embeds */ Awful.embedTweetNow = function(thisPostElement) { - if (!thisPostElement.classList.contains("embed-processed")) { - thisPostElement.classList.add("embed-processed"); + // Check if already processing or processed + if (thisPostElement.classList.contains("embed-processed") || + thisPostElement.classList.contains("embed-processing")) { + return; + } - const enableGhost = (window.Awful.renderGhostTweets == true); - const tweetLinks = thisPostElement.querySelectorAll('a[data-tweet-id]'); + // Mark as processing to prevent duplicate IntersectionObserver calls during embedding + thisPostElement.classList.add("embed-processing"); + + const enableGhost = (window.Awful.renderGhostTweets == true); + const tweetLinks = thisPostElement.querySelectorAll('a[data-tweet-id]'); - if (tweetLinks.length == 0) { + if (tweetLinks.length == 0) { + // No tweets to embed, mark as processed immediately + thisPostElement.classList.remove("embed-processing"); + thisPostElement.classList.add("embed-processed"); + return; + } + + // Group tweet links by ID for deduplication + const tweetIDsToLinks = {}; + Array.prototype.forEach.call(tweetLinks, function(a) { + // Skip tweets with NWS content (use optional chaining to avoid null reference errors) + if (a.parentElement?.querySelector('img.awful-smile[title=":nws:"]')) { return; } + const tweetID = a.dataset.tweetId; + if (!(tweetID in tweetIDsToLinks)) { + tweetIDsToLinks[tweetID] = []; + } + tweetIDsToLinks[tweetID].push(a); + }); - // Group tweet links by ID for deduplication - const tweetIDsToLinks = {}; - Array.prototype.forEach.call(tweetLinks, function(a) { - // Skip tweets with NWS content (use optional chaining to avoid null reference errors) - if (a.parentElement?.querySelector('img.awful-smile[title=":nws:"]')) { - return; - } - const tweetID = a.dataset.tweetId; - if (!(tweetID in tweetIDsToLinks)) { - tweetIDsToLinks[tweetID] = []; - } - tweetIDsToLinks[tweetID].push(a); - }); + // Track completion of tweets in this post - only mark as processed when ALL complete + let pendingTweets = Object.keys(tweetIDsToLinks).length; - // Fetch and embed each unique tweet - Object.keys(tweetIDsToLinks).forEach(function(tweetID) { - const callback = `jsonp_callback_${tweetID}`; - // Validate theme is an expected value to prevent URL injection - const tweetTheme = Awful.tweetTheme(); - const validThemes = ['light', 'dark']; - const safeTheme = validThemes.includes(tweetTheme) ? tweetTheme : 'light'; + function markTweetComplete() { + pendingTweets--; + if (pendingTweets === 0) { + // All tweets done (success or failure) - now safe to mark as processed + thisPostElement.classList.remove("embed-processing"); + thisPostElement.classList.add("embed-processed"); + } + } - const script = document.createElement('script'); - script.src = `https://api.twitter.com/1/statuses/oembed.json?id=${tweetID}&omit_script=true&dnt=true&theme=${safeTheme}&callback=${callback}`; + // Fetch and embed each unique tweet using shared helper function + Object.keys(tweetIDsToLinks).forEach(function(tweetID) { + const tweetLinks = tweetIDsToLinks[tweetID]; - window[callback] = function(data) { - cleanUp(script); + // Get first link's URL for error messages + const firstLink = tweetLinks[0]; + const tweetURL = firstLink ? firstLink.href : ''; - // Replace all links for this tweet with the embedded HTML + Awful.fetchTweetOEmbed( + tweetID, + // onSuccess callback + function(data, tweetID) { + // Replace all links for this tweet with embedded HTML tweetIDsToLinks[tweetID].forEach(function(a) { if (a.parentNode) { const div = document.createElement('div'); @@ -211,42 +366,26 @@ Awful.embedTweetNow = function(thisPostElement) { } }); - // Load Twitter widgets to render the embedded tweets - if (window.twttr) { - twttr.widgets.load(); - } - }; - - script.onerror = function() { - cleanUp(this); - console.error(`The embed markup for tweet ${tweetID} failed to load`); - - // when a tweet errors out, insert a floating ghost lottie in somber rememberence of the tweet that used to be - if (enableGhost) { - tweetIDsToLinks[tweetID].forEach(function(a) { - if (a.parentNode) { - const div = document.createElement('div'); - div.classList.add('dead-tweet-container'); - div.innerHTML = Awful.deadTweetBadgeHTML(a.href.toString(), `${tweetID}`); - a.parentNode.replaceChild(div, a); - - // Use helper function to set up Lottie player (fixes code duplication and null check) - Awful.setupGhostLottiePlayer(div); - } - }); - } - }; + // Load Twitter widgets (with race condition fix) + Awful.loadTwitterWidgetsForEmbeds(); - function cleanUp(script) { - delete window[callback]; - if (script.parentNode) { - script.parentNode.removeChild(script); - } - } + // Mark this tweet as complete + markTweetComplete(); + }, + // onFailure callback + function(reason, tweetID) { + // Show dead tweet badge for all links with this tweet ID + tweetIDsToLinks[tweetID].forEach(function(a) { + if (a.parentNode) { + Awful.showDeadTweetBadge(tweetID, tweetURL, a); + } + }); - document.body.appendChild(script); - }); - } + // Mark this tweet as complete (even though it failed) + markTweetComplete(); + } + ); + }); }; /** @@ -311,6 +450,10 @@ Awful.embedTweets = function() { Awful.embedTweetsInProgress = true; try { + // Clean up any existing observers/timers before setting up new ones + // This handles the case where embedTweets() is called multiple times on the same page + Awful.cleanupObservers(); + Awful.loadTwitterWidgets(); const enableGhost = (window.Awful.renderGhostTweets == true); @@ -351,6 +494,9 @@ Awful.embedTweets = function() { Awful.setupRetryHandler(); Awful.setupLazyImageErrorHandling(); + // Tweet retry handling + Awful.setupTweetRetryHandler(); + // Set up lazy-loading IntersectionObserver for tweet embeds // Tweets are loaded before entering the viewport based on LAZY_LOAD_LOOKAHEAD_DISTANCE // Disconnect previous observer if it exists (prevents memory leak on re-render) @@ -597,6 +743,76 @@ Awful.setupRetryHandler = function() { document.addEventListener('click', Awful.retryClickHandler, { once: false }); }; +/** + * Sets up a click event listener for retrying failed tweet embeds. + * Uses shared fetchTweetOEmbed helper for consistent timeout and error handling. + */ +Awful.setupTweetRetryHandler = function() { + // Remove old event listener if it exists (prevents memory leak on page re-render) + if (Awful.tweetRetryClickHandler) { + document.removeEventListener('click', Awful.tweetRetryClickHandler); + } + + // Define handler function and store reference for cleanup + Awful.tweetRetryClickHandler = function(event) { + const button = event.target; + if (button.hasAttribute('data-retry-tweet')) { + event.preventDefault(); + + const tweetID = button.getAttribute('data-retry-tweet'); + const tweetURL = button.getAttribute('data-tweet-url'); + const deadContainer = button.closest('.dead-tweet-container'); + + if (!deadContainer || !deadContainer.parentNode) { + return; + } + + // Validate URL is actually a Twitter/X URL (security check) + if (!tweetURL.match(/^https?:\/\/(www\.)?(twitter\.com|x\.com)\//)) { + console.error('Invalid tweet URL for retry:', tweetURL); + return; + } + + // Disable button during retry + button.disabled = true; + button.textContent = 'Retrying...'; + + // Create loading indicator + const loadingDiv = document.createElement('div'); + loadingDiv.className = 'tweet-loading'; + loadingDiv.textContent = 'Loading tweet...'; + deadContainer.parentNode.replaceChild(loadingDiv, deadContainer); + + // Use shared fetch function + Awful.fetchTweetOEmbed( + tweetID, + // onSuccess + function(data, tweetID) { + if (loadingDiv.parentNode) { + const div = document.createElement('div'); + div.classList.add('tweet'); + div.innerHTML = data.html; + loadingDiv.parentNode.replaceChild(div, loadingDiv); + + // Load Twitter widgets (using shared function) + Awful.loadTwitterWidgetsForEmbeds(); + } + }, + // onFailure + function(reason, tweetID) { + // Show dead badge again using shared function + if (loadingDiv.parentNode) { + Awful.showDeadTweetBadge(tweetID, tweetURL, loadingDiv); + } + } + ); + } + }; + + // Register the event listener with stored reference + document.addEventListener('click', Awful.tweetRetryClickHandler, { once: false }); +}; + /** * Cleanup function to remove retry click handler and prevent memory leaks. * Should be called when the view is destroyed or navigating away from the page. @@ -648,6 +864,19 @@ Awful.cleanupImageTimers = function() { } }; +/** + * Cleanup function to clear all tweet embedding timeout timers. + * Prevents timers from running after page navigation or view destruction. + */ +Awful.cleanupTweetTimers = function() { + if (Awful.tweetEmbedTimeouts) { + Awful.tweetEmbedTimeouts.forEach(function(timeoutId) { + clearTimeout(timeoutId); + }); + Awful.tweetEmbedTimeouts = []; + } +}; + /** * Cleanup function to disconnect all IntersectionObservers and prevent memory leaks. * Should be called when the view is destroyed or navigating away from the page. @@ -663,6 +892,7 @@ Awful.cleanupObservers = function() { } Awful.cleanupImageTimers(); + Awful.cleanupTweetTimers(); }; Awful.tweetTheme = function() { @@ -691,11 +921,26 @@ Awful.loadTwitterWidgets = function() { var script = document.createElement('script'); script.id = 'twitter-wjs'; script.src = "https://platform.twitter.com/widgets.js"; + + // Add error handler for widgets.js load failure + script.onerror = function() { + console.error('Failed to load Twitter widgets.js'); + // Set flag to prevent queuing more callbacks + if (window.twttr) { + window.twttr._failed = true; + } + }; + document.body.appendChild(script); window.twttr = { _e: [], + _failed: false, // Track load failure ready: function(f) { + if (window.twttr._failed) { + console.warn('Twitter widgets.js failed to load, skipping callback'); + return; + } twttr._e.push(f); } }; @@ -1176,6 +1421,7 @@ Awful.deadTweetBadgeHTML = function(url, tweetID){
DEAD TWEET @${tweeter} + Retry `; return html; From 9b5eba35b918064b44178863f95425d12d1d6eff Mon Sep 17 00:00:00 2001 From: commiekong <30882689+dfsm@users.noreply.github.com> Date: Sun, 21 Dec 2025 12:11:24 +1100 Subject: [PATCH 13/13] Final code quality pass (added a new named constant for connectionTimeout in RenderView.js). Added optional show/hide configuration option for the newly introduced LoadingView status message and exit button. This will now display for loading threads but not for loading while previewing new posts or private messages. --- App/Resources/RenderView.js | 14 ++++-- .../Messages/MessageViewController.swift | 2 +- .../Posts/PostPreviewViewController.swift | 4 +- .../Posts/PostsPageViewController.swift | 4 +- .../Threads/ThreadPreviewViewController.swift | 4 +- App/Views/LoadingView.swift | 50 ++++++++++++------- 6 files changed, 50 insertions(+), 28 deletions(-) diff --git a/App/Resources/RenderView.js b/App/Resources/RenderView.js index 922444fd6..6cd4ab4f7 100644 --- a/App/Resources/RenderView.js +++ b/App/Resources/RenderView.js @@ -34,13 +34,21 @@ const IMAGE_LOAD_TIMEOUT_CONFIG = { /// Milliseconds to wait before resetting retry button text after failed retry /// 3000ms gives user time to read the error message before it resets - retryResetDelay: 3000 + retryResetDelay: 3000, + + /// Milliseconds between timeout checks for image loading + /// 1000ms = 1 second interval for checking if images have started loading + connectionTimeout: 1000 }; /// Timeout for tweet embedding via Twitter API /// 5 seconds gives enough time for API response while preventing indefinite waiting const TWEET_EMBED_TIMEOUT = 5000; +/// Threshold for IntersectionObserver - triggers when even tiny fraction is visible +/// This minimal threshold ensures the observer fires as soon as element enters viewport +const INTERSECTION_THRESHOLD_MIN = 0.000001; + // MARK: - Utility Functions /** @@ -467,7 +475,7 @@ Awful.embedTweets = function() { const ghostConfig = { root: document.body.posts, rootMargin: '0px', - threshold: 0.000001, + threshold: INTERSECTION_THRESHOLD_MIN, }; Awful.ghostLottieObserver = new IntersectionObserver(function(posts) { @@ -507,7 +515,7 @@ Awful.embedTweets = function() { const lazyLoadConfig = { root: null, rootMargin: `${LAZY_LOAD_LOOKAHEAD_DISTANCE} 0px`, - threshold: 0.000001, + threshold: INTERSECTION_THRESHOLD_MIN, }; Awful.tweetLazyLoadObserver = new IntersectionObserver(function(entries) { diff --git a/App/View Controllers/Messages/MessageViewController.swift b/App/View Controllers/Messages/MessageViewController.swift index 2a3e889f9..2422a6826 100644 --- a/App/View Controllers/Messages/MessageViewController.swift +++ b/App/View Controllers/Messages/MessageViewController.swift @@ -251,7 +251,7 @@ final class MessageViewController: ViewController { .store(in: &cancellables) if privateMessage.innerHTML == nil || privateMessage.innerHTML?.isEmpty == true || privateMessage.from == nil { - let loadingView = LoadingView.loadingViewWithTheme(theme) + let loadingView = LoadingView.loadingViewWithTheme(theme, configuration: .hideStatusElements) self.loadingView = loadingView view.addSubview(loadingView) diff --git a/App/View Controllers/Posts/PostPreviewViewController.swift b/App/View Controllers/Posts/PostPreviewViewController.swift index 16bf03863..ca5809f98 100644 --- a/App/View Controllers/Posts/PostPreviewViewController.swift +++ b/App/View Controllers/Posts/PostPreviewViewController.swift @@ -179,8 +179,8 @@ final class PostPreviewViewController: ViewController { renderView.frame = CGRect(origin: .zero, size: view.bounds.size) renderView.autoresizingMask = [.flexibleWidth, .flexibleHeight] view.insertSubview(renderView, at: 0) - - let loadingView = LoadingView.loadingViewWithTheme(theme) + + let loadingView = LoadingView.loadingViewWithTheme(theme, configuration: .hideStatusElements) self.loadingView = loadingView view.addSubview(loadingView) diff --git a/App/View Controllers/Posts/PostsPageViewController.swift b/App/View Controllers/Posts/PostsPageViewController.swift index 25098c6cb..e29dc6971 100644 --- a/App/View Controllers/Posts/PostsPageViewController.swift +++ b/App/View Controllers/Posts/PostsPageViewController.swift @@ -670,7 +670,7 @@ final class PostsPageViewController: ViewController { private func showLoadingView() { guard postsView.loadingView == nil else { return } - let loadingView = LoadingView.loadingViewWithTheme(theme) + let loadingView = LoadingView.loadingViewWithTheme(theme, configuration: .showStatusElements) loadingView.updateStatus("Loading...") loadingView.onDismiss = { [weak self] in self?.clearLoadingMessage() @@ -1507,7 +1507,7 @@ final class PostsPageViewController: ViewController { if postsView.loadingView != nil { - postsView.loadingView = LoadingView.loadingViewWithTheme(theme) + postsView.loadingView = LoadingView.loadingViewWithTheme(theme, configuration: .showStatusElements) } let appearance = UIToolbarAppearance() diff --git a/App/View Controllers/Threads/ThreadPreviewViewController.swift b/App/View Controllers/Threads/ThreadPreviewViewController.swift index 0e09dabf8..5b8c8034a 100644 --- a/App/View Controllers/Threads/ThreadPreviewViewController.swift +++ b/App/View Controllers/Threads/ThreadPreviewViewController.swift @@ -187,8 +187,8 @@ final class ThreadPreviewViewController: ViewController { threadCell.autoresizingMask = .flexibleWidth renderView.scrollView.addSubview(threadCell) - - let loadingView = LoadingView.loadingViewWithTheme(theme) + + let loadingView = LoadingView.loadingViewWithTheme(theme, configuration: .hideStatusElements) self.loadingView = loadingView view.addSubview(loadingView) diff --git a/App/Views/LoadingView.swift b/App/Views/LoadingView.swift index f299af48f..4efc3d2e0 100644 --- a/App/Views/LoadingView.swift +++ b/App/Views/LoadingView.swift @@ -7,6 +7,15 @@ import FLAnimatedImage import UIKit import Lottie +/// Configuration for LoadingView behavior +enum LoadingViewConfiguration { + /// Shows status text and exit button after delay (for thread pages) + case showStatusElements + + /// Never shows status text or exit button (for previews and messages) + case hideStatusElements +} + /// A view that covers its superview with an indeterminate progress indicator. class LoadingView: UIView { @@ -19,30 +28,32 @@ class LoadingView: UIView { // MARK: - Properties fileprivate let theme: Theme? + let configuration: LoadingViewConfiguration - fileprivate init(theme: Theme?) { + fileprivate init(theme: Theme?, configuration: LoadingViewConfiguration) { self.theme = theme + self.configuration = configuration super.init(frame: .zero) } convenience init() { - self.init(theme: nil) + self.init(theme: nil, configuration: .showStatusElements) } required init?(coder: NSCoder) { fatalError("init(coder:) has not been implemented") } - class func loadingViewWithTheme(_ theme: Theme) -> LoadingView { + class func loadingViewWithTheme(_ theme: Theme, configuration: LoadingViewConfiguration = .showStatusElements) -> LoadingView { switch theme[string: "postsLoadingViewType"] { case "Macinyos"?: - return MacinyosLoadingView(theme: theme) + return MacinyosLoadingView(theme: theme, configuration: configuration) case "Winpos95"?: - return Winpos95LoadingView(theme: theme) + return Winpos95LoadingView(theme: theme, configuration: configuration) case "YOSPOS"?: - return YOSPOSLoadingView(theme: theme) + return YOSPOSLoadingView(theme: theme, configuration: configuration) default: - return DefaultLoadingView(theme: theme) + return DefaultLoadingView(theme: theme, configuration: configuration) } } @@ -71,7 +82,7 @@ private class DefaultLoadingView: LoadingView { private let showNowButton: UIButton private var visibilityTimer: Timer? - override init(theme: Theme?) { + override init(theme: Theme?, configuration: LoadingViewConfiguration) { animationView = LottieAnimationView( animation: LottieAnimation.named("mainthrobber60"), configuration: LottieConfiguration(renderingEngine: .mainThread)) @@ -79,7 +90,7 @@ private class DefaultLoadingView: LoadingView { statusLabel = UILabel() showNowButton = UIButton(type: .system) - super.init(theme: theme) + super.init(theme: theme, configuration: configuration) // Setup animation view animationView.currentFrame = 0 @@ -176,6 +187,9 @@ private class DefaultLoadingView: LoadingView { super.willMove(toSuperview: newSuperview) if newSuperview != nil { + // Only start timer if configuration allows status elements + guard configuration == .showStatusElements else { return } + // Invalidate any existing timer first to prevent race conditions visibilityTimer?.invalidate() // Start timer to show status and button after delay @@ -205,9 +219,9 @@ private class DefaultLoadingView: LoadingView { private class YOSPOSLoadingView: LoadingView { let label = UILabel() fileprivate var timer: Timer? - - override init(theme: Theme?) { - super.init(theme: theme) + + override init(theme: Theme?, configuration: LoadingViewConfiguration) { + super.init(theme: theme, configuration: configuration) backgroundColor = .black @@ -271,9 +285,9 @@ private class YOSPOSLoadingView: LoadingView { private class MacinyosLoadingView: LoadingView { let imageView = UIImageView() - - override init(theme: Theme?) { - super.init(theme: theme) + + override init(theme: Theme?, configuration: LoadingViewConfiguration) { + super.init(theme: theme, configuration: configuration) if let wallpaper = UIImage(named: "macinyos-wallpaper") { backgroundColor = UIColor(patternImage: wallpaper) @@ -302,9 +316,9 @@ private class Winpos95LoadingView: LoadingView { let imageView = FLAnimatedImageView() var centerXConstraint: NSLayoutConstraint! var centerYConstraint: NSLayoutConstraint! - - override init(theme: Theme?) { - super.init(theme: theme) + + override init(theme: Theme?, configuration: LoadingViewConfiguration) { + super.init(theme: theme, configuration: configuration) backgroundColor = UIColor(red: 0.067, green: 0.502, blue: 0.502, alpha: 1)