From a607426a9c98cd4301645791d1d4ca7d91bb2eb8 Mon Sep 17 00:00:00 2001 From: Dillard Blom Date: Sun, 27 Sep 2026 00:24:47 +0200 Subject: [PATCH] Only load vendored jQuery on NC34+, verified against NC32/33/34 in testbak/ Core still provides a working jQuery on NC32/33, but via a read-only window.$ getter - loading our own vendored copy there too made its UMD wrapper throw trying to reassign window.$ (confirmed on real containers, harmless in practice since this app attaches via window.jQuery, but a real avoidable console exception). Gate the extra script() call on ServerVersion::getMajorVersion() >= 34 instead. Re-tested both sides of the boundary: NC32 no longer requests jquery.js and has zero console errors; NC34 still requests it and still works. --- README.md | 7 ++++--- appinfo/info.xml | 2 +- templates/index.php | 13 +++++++++++-- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 0179b4c..dfedf7a 100644 --- a/README.md +++ b/README.md @@ -149,11 +149,12 @@ was verified, so a rollback target is obvious if a newer major causes problems. | 0.4.0 | (untagged) | 34.0.4 | ✅ Verified (nc01, 2026-09-26) - app id renamed `occweb` → `extended_occweb` (App Store prep), no functional changes to the 0.3.5 fixes otherwise | | 0.4.1 | `v0.4.1-nc34` | 34.0.4 | ✅ Verified (nc01, 2026-09-26) - code review fixes: correct jQuery `.fail()` signature, added a missing `.fail()` handler on the initial terminal-load request | | 0.4.1 | `v0.4.1-nc34` | 33.0.8 | ⚠️ Same caveat as 0.3.5/0.4.0 - not re-verified, no NC33 test environment available. Also unresolved: `js/jquery.js` now loads unconditionally on every supported version, even NC30-33 where core still provides jQuery - flagged in code review, needs an NC33 environment to confirm it's harmless there. | -| 0.4.2 | (untagged) | 32.0.15 | ✅ Verified (`testbak/`, 2026-09-27) - terminal loads and runs commands correctly. Confirms the suspected jQuery issue above: core still exposes a read-only `window.$` getter here, so our vendored `js/jquery.js` throws `TypeError: Cannot set property $ of # which has only a getter` on load - a real, uncaught console exception, but harmless in practice since the app attaches via `window.jQuery` (still writable), not `window.$`. Not fixed yet - conditional jQuery loading is still deferred (see the code review notes below). | -| 0.4.2 | (untagged) | 33.0.9 | ✅ Verified (`testbak/`, 2026-09-27) - same result as 32.0.15: functional, same harmless-but-real `window.$` console exception. | +| 0.4.2 | (untagged) | 32.0.15 | ✅ Verified (`testbak/`, 2026-09-27) - terminal loads and runs commands correctly. Confirms the suspected jQuery issue above: core still exposes a read-only `window.$` getter here, so our vendored `js/jquery.js` throws `TypeError: Cannot set property $ of # which has only a getter` on load - a real, uncaught console exception, but harmless in practice since the app attaches via `window.jQuery` (still writable), not `window.$`. Fixed in 0.4.3. | +| 0.4.2 | (untagged) | 33.0.9 | ✅ Verified (`testbak/`, 2026-09-27) - same result as 32.0.15: functional, same harmless-but-real `window.$` console exception. Fixed in 0.4.3. | | 0.4.2 | (untagged) | 35.0.1 | ✅ Verified (`testbak/`, 2026-09-27) after a real fix: the bootstrap request 500'd with a PHP fatal, `Declaration of OccOutput::setErrorOutput() must be compatible with ConsoleOutputInterface::setErrorOutput(): void` - NC35 bundles a Symfony Console version whose interface now declares that return type. Fixed by adding `: void` to `OccOutput::setErrorOutput()` (`lib/Controller/OccOutput.php`), which is compatible with every earlier Symfony/NC version too. Confirmed clean afterwards: no console errors, no CSRF issues, terminal fully functional. | +| 0.4.3 | (untagged) | 32.0.15, 34.0.4 | ✅ Verified (`testbak/`, 2026-09-27) - `templates/index.php` now only loads `js/jquery.js` when `\OCP\ServerVersion::getMajorVersion() >= 34`, since core still provides a working jQuery on 32/33. Re-tested both sides of the boundary: NC32 loads no `jquery.js` request at all and has zero console errors; NC34 still loads it and still works, confirming no regression. | -`min-version`/`max-version` in `appinfo/info.xml` were updated from `30`/`34` to `32`/`35` on 2026-09-27 based on the above: 30/31 have never actually been tested (not even before the app-id rename) so claiming them was never justified once the `testbak/`-based verification policy existed, and 35 is now a genuinely verified (and fixed) target rather than a guess. +`min-version`/`max-version` in `appinfo/info.xml` were updated from `30`/`34` to `32`/`35` on 2026-09-27 based on the above: 30/31 have never actually been tested (not even before the app-id rename) so claiming them was never justified once the `testbak/`-based verification policy existed, and 35 is now a genuinely verified (and fixed) target rather than a guess. 30/31 are also past Nextcloud's own currently-supported boundary as of 2026-09-18 (32-35 supported), which independently confirms 32 as the right floor. If a future major breaks something, `v0.3.4` is the last version confirmed working on NC33 and older - but a plain `git checkout v0.3.4` is **not** a working rollback by itself. That diff --git a/appinfo/info.xml b/appinfo/info.xml index d401cdc..d0aa006 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -5,7 +5,7 @@ Extended OCC Web OCC Commands in a web terminal - 0.4.2 + 0.4.3 agpl Dillard Blom OCCWeb diff --git a/templates/index.php b/templates/index.php index ad31f1b..a8df7c6 100644 --- a/templates/index.php +++ b/templates/index.php @@ -2,8 +2,17 @@ // Nextcloud's global jQuery has been deprecated since NC19 and is no longer // reliably present on NC34+ (see e.g. serverinfo needing to bundle its own) - // term.js (jquery.terminal) and index.js both require jQuery to be loaded -// first, so ship and load our own copy rather than relying on core. -script('extended_occweb', 'jquery'); +// first, so ship and load our own copy where core no longer provides one. +// +// Only do this on NC34+: on NC32/33 core still exposes a working jQuery, but +// via a read-only `window.$` getter - loading our vendored copy there too +// makes its UMD wrapper throw trying to reassign `window.$` (confirmed via +// testbak/, see the README compat matrix). Harmless in practice (this app +// attaches via window.jQuery, not window.$) but a real, avoidable console +// exception, so skip the extra load where core's own jQuery already works. +if (\OCP\Server::get(\OCP\ServerVersion::class)->getMajorVersion() >= 34) { + script('extended_occweb', 'jquery'); +} script('extended_occweb', 'index'); script('extended_occweb', 'term'); script('extended_occweb', 'unix_formatting');