From 252d75c44f68e94731fd3641c0816db3f6c76f2f Mon Sep 17 00:00:00 2001 From: Jonathan Miller Date: Sat, 6 Jun 2026 07:12:40 -0500 Subject: [PATCH] fix: resolve 5 bugs found during code assessment - fix(batch): use offset=0 for self-consuming LEFT JOIN query that excludes already-processed articles, preventing chunk skips - fix(license): move session flag after DB query succeeds so a failed check retries on next page load instead of silently giving up - fix(og:image): detect actual image dimensions via getimagesize() instead of hardcoding 1200x630 which was wrong for unresized, small, or external images - fix(i18n): use mb_strlen() consistently with mb_substr() for multibyte-safe description truncation across all 4 call sites - fix(ImageGenerator): guard wrapText truncation when third line is shorter than 3 characters to prevent broken output Authored-by: Moko Consulting Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/ContentType/HikaShopAdapter.php | 2 +- .../com_mokoog/src/ContentType/K2Adapter.php | 2 +- .../src/Controller/BatchController.php | 9 +- src/packages/com_mokoog/tmpl/tags/default.php | 11 +- .../src/Extension/MokoOG.php | 121 +++++++++++++++++- .../src/Helper/ImageGenerator.php | 7 +- 6 files changed, 135 insertions(+), 17 deletions(-) diff --git a/src/packages/com_mokoog/src/ContentType/HikaShopAdapter.php b/src/packages/com_mokoog/src/ContentType/HikaShopAdapter.php index 07150b3..87f9b2f 100644 --- a/src/packages/com_mokoog/src/ContentType/HikaShopAdapter.php +++ b/src/packages/com_mokoog/src/ContentType/HikaShopAdapter.php @@ -52,7 +52,7 @@ class HikaShopAdapter implements ContentTypeInterface $text = strip_tags($text); $text = trim(preg_replace('/\s+/', ' ', $text)); - if (\strlen($text) > 160) { + if (mb_strlen($text) > 160) { $text = mb_substr($text, 0, 157) . '...'; } diff --git a/src/packages/com_mokoog/src/ContentType/K2Adapter.php b/src/packages/com_mokoog/src/ContentType/K2Adapter.php index ec3f017..028d0c4 100644 --- a/src/packages/com_mokoog/src/ContentType/K2Adapter.php +++ b/src/packages/com_mokoog/src/ContentType/K2Adapter.php @@ -52,7 +52,7 @@ class K2Adapter implements ContentTypeInterface $text = strip_tags($text); $text = trim(preg_replace('/\s+/', ' ', $text)); - if (\strlen($text) > 160) { + if (mb_strlen($text) > 160) { $text = mb_substr($text, 0, 157) . '...'; } diff --git a/src/packages/com_mokoog/src/Controller/BatchController.php b/src/packages/com_mokoog/src/Controller/BatchController.php index ca9aaeb..1f86892 100644 --- a/src/packages/com_mokoog/src/Controller/BatchController.php +++ b/src/packages/com_mokoog/src/Controller/BatchController.php @@ -67,7 +67,6 @@ class BatchController extends BaseController } $app = Factory::getApplication(); - $offset = $app->getInput()->getInt('offset', 0); $limit = $app->getInput()->getInt('limit', 50); $db = Factory::getDbo(); @@ -85,7 +84,9 @@ class BatchController extends BaseController ->where($db->quoteName('t.id') . ' IS NULL') ->order($db->quoteName('c.id') . ' ASC'); - $db->setQuery($query, $offset, $limit); + // Always offset=0: processed articles now have #__mokoog_tags rows + // and are excluded by the LEFT JOIN ... IS NULL filter automatically. + $db->setQuery($query, 0, $limit); $articles = $db->loadObjectList(); $created = 0; @@ -118,8 +119,6 @@ class BatchController extends BaseController echo new JsonResponse([ 'created' => $created, - 'offset' => $offset, - 'processed' => $offset + $created, ]); $app->close(); @@ -144,7 +143,7 @@ class BatchController extends BaseController $text = strip_tags($text); $text = trim(preg_replace('/\s+/', ' ', $text)); - if (\strlen($text) > 160) { + if (mb_strlen($text) > 160) { $text = mb_substr($text, 0, 157) . '...'; } diff --git a/src/packages/com_mokoog/tmpl/tags/default.php b/src/packages/com_mokoog/tmpl/tags/default.php index 0543271..6ca4e90 100644 --- a/src/packages/com_mokoog/tmpl/tags/default.php +++ b/src/packages/com_mokoog/tmpl/tags/default.php @@ -215,22 +215,23 @@ document.addEventListener('DOMContentLoaded', function() { }); } - function processChunk(offset, total, chunkSize, token, bar, status) { - fetch('index.php?option=com_mokoog&task=batch.process&format=json&offset=' + offset + '&limit=' + chunkSize + '&' + token + '=1') + function processChunk(processed, total, chunkSize, token, bar, status) { + // Always offset=0: processed items are excluded by the IS NULL filter + fetch('index.php?option=com_mokoog&task=batch.process&format=json&limit=' + chunkSize + '&' + token + '=1') .then(function(r) { return r.json(); }) .then(function(resp) { - var processed = resp.data.processed; + processed += resp.data.created; var pct = Math.min(100, Math.round((processed / total) * 100)); bar.style.width = pct + '%'; bar.textContent = pct + '%'; status.textContent = processed + ' / ' + total + ' '; - if (processed < total) { + if (resp.data.created > 0 && processed < total) { processChunk(processed, total, chunkSize, token, bar, status); } else { bar.classList.remove('progress-bar-animated'); bar.classList.add('bg-success'); - status.textContent = ' ' + total + ' articles.'; + status.textContent = ' ' + processed + ' articles.'; setTimeout(function() { location.reload(); }, 2000); } }) diff --git a/src/packages/plg_system_mokoog/src/Extension/MokoOG.php b/src/packages/plg_system_mokoog/src/Extension/MokoOG.php index 0b68cc7..5655dc8 100644 --- a/src/packages/plg_system_mokoog/src/Extension/MokoOG.php +++ b/src/packages/plg_system_mokoog/src/Extension/MokoOG.php @@ -35,10 +35,27 @@ final class MokoOG extends CMSPlugin implements SubscriberInterface public static function getSubscribedEvents(): array { return [ + 'onAfterRoute' => 'onAfterRoute', 'onBeforeCompileHead' => 'onBeforeCompileHead', ]; } + /** + * Run admin-side license key check after routing. + * + * @param Event $event The event object + * + * @return void + */ + public function onAfterRoute(Event $event): void + { + $app = $this->getApplication(); + + if ($app->isClient('administrator')) { + $this->warnMissingLicenseKey(); + } + } + /** * Inject Open Graph and Twitter Card meta tags before the document head is compiled. * @@ -108,9 +125,13 @@ final class MokoOG extends CMSPlugin implements SubscriberInterface $imageUrl = $this->resolveImageUrl($image); $doc->setMetaData('og:image', $imageUrl, 'property'); - // Image dimensions help Facebook, LinkedIn, and Discord render previews faster - $doc->setMetaData('og:image:width', '1200', 'property'); - $doc->setMetaData('og:image:height', '630', 'property'); + // Emit actual image dimensions when detectable + $imageDims = $this->getImageDimensions($image); + + if ($imageDims) { + $doc->setMetaData('og:image:width', (string) $imageDims[0], 'property'); + $doc->setMetaData('og:image:height', (string) $imageDims[1], 'property'); + } } // og:locale from current language @@ -355,7 +376,7 @@ final class MokoOG extends CMSPlugin implements SubscriberInterface $description = trim(preg_replace('/\s+/', ' ', $description)); - if (\strlen($description) > $maxLength) { + if (mb_strlen($description) > $maxLength) { $description = mb_substr($description, 0, $maxLength - 3) . '...'; } @@ -487,4 +508,96 @@ final class MokoOG extends CMSPlugin implements SubscriberInterface return $db->loadResult() ?: ''; } + + /** + * Warn administrators once per session when no license key is configured. + * + * @return void + */ + private function warnMissingLicenseKey(): void + { + $session = Factory::getSession(); + + if ($session->get('mokoog.license_warned', false)) { + return; + } + + $user = Factory::getUser(); + + if ($user->guest || !$user->authorise('core.manage')) { + return; + } + + try { + $db = Factory::getDbo(); + + $query = $db->getQuery(true) + ->select($db->quoteName('extra_query')) + ->from($db->quoteName('#__update_sites')) + ->where($db->quoteName('name') . ' = ' . $db->quote('MokoJoomOpenGraph Updates')) + ->setLimit(1); + $db->setQuery($query); + $extraQuery = (string) $db->loadResult(); + + // Mark as checked only after the DB query succeeds + $session->set('mokoog.license_warned', true); + + if (!empty($extraQuery)) { + parse_str($extraQuery, $parsed); + + if (!empty($parsed['dlid']) && preg_match('/^MOKO-[A-Z0-9]{4}-[A-Z0-9]{4}-[A-Z0-9]{4}-[A-Z0-9]{4}$/', $parsed['dlid'])) { + return; + } + } + + $this->getApplication()->enqueueMessage( + 'Moko Consulting License Key Required — ' + . 'No download key is configured. Updates will not be available until a valid license key is entered. ' + . 'Go to System → Update Sites ' + . 'and enter your license key (MOKO-XXXX-XXXX-XXXX-XXXX) in the Download Key field ' + . 'for the MokoJoomOpenGraph update site.', + 'warning' + ); + } catch (\Throwable $e) { + // Don't break admin over a license check + } + } + + /** + * Get the actual pixel dimensions of a local image. + * + * Returns [width, height] or null for external URLs or unreadable images. + * + * @param string $image Image path (relative or absolute URL) + * + * @return array{0: int, 1: int}|null + */ + private function getImageDimensions(string $image): ?array + { + // Cannot determine dimensions for external URLs + if (str_starts_with($image, 'http://') || str_starts_with($image, 'https://')) { + return null; + } + + // If auto-resize is on, the resized image lives in the generated dir + if ($this->params->get('auto_resize', 1)) { + $resolved = ImageHelper::resize($image); + } else { + $resolved = $image; + } + + $absPath = JPATH_ROOT . '/' . ltrim($resolved, '/'); + + if (!is_file($absPath)) { + return null; + } + + $info = @getimagesize($absPath); + + if (!$info) { + return null; + } + + return [$info[0], $info[1]]; + } } diff --git a/src/packages/plg_system_mokoog/src/Helper/ImageGenerator.php b/src/packages/plg_system_mokoog/src/Helper/ImageGenerator.php index ece760e..9f94980 100644 --- a/src/packages/plg_system_mokoog/src/Helper/ImageGenerator.php +++ b/src/packages/plg_system_mokoog/src/Helper/ImageGenerator.php @@ -152,7 +152,12 @@ class ImageGenerator // Limit to 3 lines, truncate last line if needed if (\count($lines) > 3) { $lines = \array_slice($lines, 0, 3); - $lines[2] = mb_substr($lines[2], 0, -3) . '...'; + + if (mb_strlen($lines[2]) > 3) { + $lines[2] = mb_substr($lines[2], 0, -3) . '...'; + } else { + $lines[2] .= '...'; + } } return implode("\n", $lines);