Modernize extension; merge and reconcile PRs #5, #6, #7 - #8
Merged
Merged
Conversation
Brings the extension up to current phpBB/PHP support and reconciles three long-open, overlapping PRs into one coherent change: Version support: - composer.json: PHP >=5.3.3 -> >=7.1, phpbb soft-require bumped to <3.3.*@dev, GPL-2.0 -> GPL-2.0-only (current SPDX identifier), added version-check block so phpBB's ACP can detect new releases, version bumped to 2.1.0. - config/services.yml: quoted service references and added `_defaults: public: true`, required for the newer Symfony versions phpBB 3.3.x ships. Feature: Christmas theme (from PR #7, the more complete and more recent of the two competing jQuery-image-display PRs; #6 is an earlier, superseded revision of the same idea and also had an unrelated .gitattributes regression -- `text=auto` accidentally changed to `text=false` -- that is not adopted here): - Corner-hat banner and a new "mini hat" icon on the forum list are now injected via jQuery instead of being hardcoded into one style's template, so they work across every style the extension ships events for (prosilver, Green-Style-Slim, pro_ubuntu_lucid, proflat) instead of prosilver only. Feature: Valentine's Day theme (from PR #5), reworked: - Config, ACP field, event listener, and template events renamed from the single enable_hohohatcorner flag to enable_xmas / enable_valentine so the two themes are independently selectable (matching PR #5's intent), applied consistently across all four styles' event files, not just prosilver. - New migrations/xmas_valentine_split.php (depends_on config_data) adds enable_valentine and renames an existing install's enable_hohohatcorner value to enable_xmas, preserving the setting instead of silently losing it on upgrade -- neither open PR included a migration for this rename. - Dropped PR #5's unfinished styles/prosilver/theme/*.forumbg,.forabg background-image override and unused .hat class (no template references either), and its committed changes.md scratch file. - Added en/fr/ar strings for the renamed and new theme toggles (existing languages this repo already ships); kept PR #5's new nl translation. Dead tooling removed, not migrated: .travis.yml, travis/ helper script, phpunit.xml.dist (configured a ./tests/ suite that doesn't exist in this repo), composer.lock (stale, minimal real dependency tree to lock), and the committed composer.phar binary. Replaced with .github/workflows/ci.yml: PHP syntax lint across PHP 7.1/7.4/8.0/8.1 and composer.json validation -- meaningfully checkable without a full phpBB integration-test harness, which the old Travis config also only partially provided (several of its own referenced scripts came from a phpBB-core checkout it cloned at CI time, not from this repo). README updated: CI badge, features list, ACP settings names. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
[valdeny] Inline JavaScript in template event files -> extracted to
real .js files (styles/prosilver/theme/js/xmas.js, valentine.js),
included via <!-- INCLUDEJS --> per the validation policy
("Include JavaScript files with the template function: <!--
INCLUDEJS -->"). One shared script per theme, referenced by all four
styles the same way the CSS already is -- this also gives
Green-Style-Slim, pro_ubuntu_lucid, and proflat the full banner+mini-icon
treatment for Christmas and the header banner for Valentine, which
they were previously missing (jQuery .insertBefore() on a selector
that doesn't exist in a given style is a harmless no-op, so this is
lower-risk than it sounds).
[valinfo] composer.json: added extension-level homepage and William's
author email/homepage (already used consistently in every file's
header comment, not new information); bumped composer/installers to
^1.0 matching the current Extension Skeleton. Left Matt (VSE)'s
email/homepage out -- no verified real value for them anywhere in
this codebase, and an empty placeholder field is a known composer.json
schema-validation trap.
[valinfo] license.txt: trimmed to end at "END OF TERMS AND CONDITIONS",
matching the Extension Skeleton's license.txt.twig exactly (confirmed
via diff, ignoring trailing whitespace). Dropped the GPLv2 boilerplate's
"How to Apply These Terms to Your New Programs" section, which the
skeleton doesn't include -- inert instructional text, not a change to
the actual license terms.
[valinfo] .github/workflows/ci.yml: re-indented from 2-space to
4-space, matching the coding guidelines' YAML exception to the tabs
rule.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
William's homepage -> the phpbbmodders community site, not a GitHub org URL. Matt's name -> Matt Friedman (VSE) -- VSE was his old phpBB username, not his name; matches how he's already credited elsewhere in this repo (language/fr/holidayflare_acp.php's copyright line). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Confirmed via his real phpBB.com Customisation Database author profile (https://www.phpbb.com/customise/db/author/mattf): MattF is his current username there, VSE is a former one ("Formerly known as VSE" per his own profile signature). Added his author profile as his composer.json homepage, now that a real verified URL exists for it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…install) The ../theme/ prefix in every INCLUDECSS/INCLUDEJS reference across all four styles was wrong -- phpBB registers an extension's theme/ directory as its own root within the Twig namespace (alongside template/), so referencing it as ../theme/xmas.css tries to escape outside the configured directories entirely and throws Twig\Error\LoaderError, a fatal 500 on every page load once the extension is enabled. This predates this branch: the original master's overall_header_head_append.html used the identical ../theme/hohohatcorner.css pattern, and PR #7 copied it unchanged into the three additional styles. Nobody had apparently ever actually run this extension in a live phpBB install -- caught by enabling it against a real phpBB 3.3.18-dev + SQLite install and watching it 500 on the very first page load. Fix: drop the ../theme/ prefix entirely (@phpbbmodders_holidayflare/xmas.css, @phpbbmodders_holidayflare/js/xmas.js). Re-verified live: Christmas and Valentine both render correctly (header-corner icon + Christmas's forum-list mini icon), migration round-trip (fresh install and simulated upgrade from the old enable_hohohatcorner config) both confirmed against a real SQLite-backed install, no PHP errors or warnings in phpBB's own log. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Real screenshots (headless Chromium against the actual running phpBB 3.3.18-dev harness install, not mockups) showing each theme rendered: the Christmas Santa hat in the header corner plus the smaller matching icon on the forum list, and the Valentine's Day heart in the header corner. Excluded from the packaged extension archive (docs export-ignore), matching .github. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Filled in the gaps against this project's standard README shape: no Requirements section (PHP/phpBB version support was only in composer.json), no Contributing section, no Acknowledgments, no License section despite a real license.txt already in the repo. Support section reworked to route bug reports to Issues (enabled on this repo) and everything else to the existing phpBB.com support thread, since Discussions isn't enabled here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Explains what the two screenshots are and how they were captured. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…edman's Verified against PR #5's actual commit author, not just guessed from context. Matt Friedman (MattF/VSE) is still credited correctly elsewhere (composer.json, as one of the extension's two original authors) -- he just wasn't the one who added Valentine's Day. Also linked rmcgirr83 and Galixte's GitHub profiles for consistency. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
Author
|
@kaileymsnay this is ready for review whenever you have time -- reconciles the three long-open PRs (#5, #6, #7) into one modernization pass, verified against a real phpBB 3.3.18-dev install (screenshots in the PR description). Investigated and written by Claude on behalf of William Jacoby (bonelifer). |
Use the standard phpBB Modders header in every PHP file while keeping the original author, copyright and translator lines. composer.json: phpBB Modders as Extension Developer, earlier contributors as Past Developer, phpbbmodders.com homepage, and keywords. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Sep 27, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the three long-open PRs by reconciling them into one coherent change, plus general modernization to current phpBB/PHP support.
Version support
composer.json: PHP>=5.3.3->>=7.1, phpBB soft-require bumped to<3.3.*@dev,GPL-2.0->GPL-2.0-only(current SPDX id), addedversion-checkblock, version bumped to2.1.0. Author details corrected: William's homepage points to phpbbmodders.com; Matt's identifier corrected from his old username VSE to his current phpBB.com username MattF (confirmed via his real author profile).config/services.yml: quoted service refs +_defaults: public: true, needed for the Symfony version phpBB 3.3.x ships.Christmas theme (from #7, the more complete and more recent of the two competing jQuery-image-display PRs -- #6 is an earlier revision of the same idea and also had an unrelated
.gitattributesregression,text=autoaccidentally changed totext=false, not adopted here):Valentine's Day theme (from #5, reworked):
enable_hohohatcornerflag toenable_xmas/enable_valentineso both are independently selectable, applied consistently across all four styles (Added Valentine #5 only did prosilver).migrations/xmas_valentine_split.phpaddsenable_valentineand migrates an existing install'senable_hohohatcornervalue toenable_xmasinstead of silently losing the setting on upgrade -- neither original PR included this..forumbg/.forabgbackground-image override and unused.hatclass (no template ever referenced it), and its committedchanges.mdscratch file.nltranslation.Dead tooling removed, not migrated:
.travis.yml, itstravis/helper script,phpunit.xml.dist(configured a./tests/suite that doesn't exist in this repo),composer.lock, and the committedcomposer.pharbinary. Replaced with.github/workflows/ci.yml: PHP syntax lint across PHP 7.1/7.4/8.0/8.1 +composer.jsonvalidation.Validation pass (phpBB.com-style, per this project's own validation prompt): found and fixed one real policy violation -- inline
<script>in template event files instead of<!-- INCLUDEJS -->-- plus several composer.json/license.txt/CI-formatting advisories. See the commit history for the full findings and fixes.Runtime bug found and fixed: every
INCLUDECSS/INCLUDEJSreference used a../theme/path prefix that's actually invalid -- phpBB registers an extension'stheme/directory as its own lookup root, so../theme/xmas.csstries to escape outside the configured directories and throws a fatalTwig\Error\LoaderErroron the very first page load once the extension is enabled. This predates this branch (the original master had the identical bug, and PR #7 copied it into the new files unchanged) -- nobody had apparently ever actually run this extension against a live phpBB install. Caught by doing exactly that, fixed, and re-verified.Header and metadata modernization (org-wide pass):
@author,@copyrightand translator-credit lines are kept word for word, with a(c) 2026, phpBB Moddersline added.composer.json: homepage → phpbbmodders.com, phpBB Modders added asExtension Developer, William and Matt →Past Developer, and addskeywords.Screenshots
Real screenshots from a live phpBB 3.3.18-dev + SQLite install running this branch:
Christmas theme:
Valentine's Day theme:
Test plan
php -lon every.phpfile in the repo: cleancomposer validate: valid (one expected warning --versionfield present -- which phpBB's own extension-manager convention requires, so kept deliberately)enable_hohohatcorner/hohohatcornerreferences outside the two migration files that are supposed to still reference it (the old, unmodified migration, and the new rename migration): none foundurl()/INCLUDECSS/INCLUDEJSreferences match the actual filenames and pathsdepends_on()matches the real namespaced class nameenable_hohohatcornersetting, both themes render correctly (screenshots above), no errors in phpBB's own logInvestigated and written by Claude on behalf of William Jacoby (bonelifer).