Skip to content

fix(manager): restore template preview image when creating document#16889

Open
Ibochkarev wants to merge 3 commits into
modxcms:3.xfrom
Ibochkarev:fix/16256-template-preview-phpthumb
Open

fix(manager): restore template preview image when creating document#16889
Ibochkarev wants to merge 3 commits into
modxcms:3.xfrom
Ibochkarev:fix/16256-template-preview-phpthumb

Conversation

@Ibochkarev

Copy link
Copy Markdown
Collaborator

What does it do?

  • Adds getPreviewPath() and getPreviewSourceId() to modElement so the template list can pass a filesystem path and media source id to phpThumb instead of a URL.
  • Updates Template/GetList to build the template preview phpThumb URL using path + source; phpThumb’s default source (Flysystem) expects a path, so passing a URL caused blank output.
  • In modMediaSource::prepareSrcForThumb(), converts local HTTP URLs (matching the site base URL) to filesystem paths before the fileExists() check, so callers that still pass URLs (e.g. subdirectory installs) continue to work.
  • In the PhpThumb processor: fixes PHP 8.1 deprecation when pathinfo(..., PATHINFO_EXTENSION) returns null (guard with is_string($ext) before strtolower($ext)), and removes error_reporting(E_ALL) so deprecations do not corrupt the image response.

Why is it needed?

Template preview images did not render when creating a new document (Create document → template picker): the preview URL pointed at phpThumb with src set to a full URL. The default media source uses Flysystem, which expects a path; fileExists($url) failed and the processor returned empty output. In subdirectory installs the image path was also wrong. PHP 8.1+ triggered a deprecation for strtolower(null) and error_reporting(E_ALL) caused that to appear in the image body.

How to test

  1. Create a template with a preview image (Template → Preview Image, e.g. under assets/images/ or a media source).
  2. Open Create document; in the template picker, select a template that has a preview.
  3. Confirm the preview image loads (no white/blank area).
  4. With MODX in a subdirectory (base_url e.g. /subdir/), repeat and confirm the preview still loads.
  5. Call phpThumb with a file that has no extension (or empty extension) and confirm no PHP deprecation in the response.

Related issue(s)/PR(s)

Resolves #16256

- Add getPreviewPath() and getPreviewSourceId() to modElement so phpThumb
  receives a filesystem path and source id instead of a URL
- Template/GetList now passes path and source to phpthumb.php; fixes blank
  preview when base_url is not / and when Flysystem received URLs
- In modMediaSource::prepareSrcForThumb(), convert local URLs to paths
  before fileExists() so legacy callers passing URLs still work
- In PhpThumb processor: guard pathinfo extension for PHP 8.1 (strtolower
  null deprecation) and remove error_reporting(E_ALL) so deprecations
  do not corrupt image output

Fixes modxcms#16256
@Ibochkarev Ibochkarev marked this pull request as ready for review February 24, 2026 18:06
@biz87

biz87 commented Feb 25, 2026

Copy link
Copy Markdown

Code Review

Summary

Fixes template preview images when creating documents. Adds getPreviewPath() and getPreviewSourceId() to modElement to provide filesystem paths (instead of URLs) to phpThumb. Updates Template/GetList to use these new methods. Adds URL-to-path conversion in modMediaSource::prepareSrcForThumb() for subdirectory installs. Removes error_reporting(E_ALL) from the PhpThumb processor and fixes a PHP 8.1 deprecation (pathinfo returning null).

Suggestions

  • modElement.php: getPreviewPath() and getPreviewSourceId() share significant logic (fetching source, initializing, checking basePath + previewfile). Consider extracting the shared source resolution into a private helper to reduce duplication.
  • modMediaSource.php:1585-1607: The URL-to-filesystem conversion in prepareSrcForThumb() does file_exists() checks — ensure this doesn't introduce a performance concern when called frequently (e.g. in a large template list), since file_exists() hits the filesystem on each call.

Assessment

Solid fix addressing a real issue with phpThumb expecting filesystem paths rather than URLs. The PhpThumb processor cleanup (removing error_reporting(E_ALL), guarding pathinfo return) is a good improvement. The is_string($ext) guard before strtolower() properly handles PHP 8.1 deprecations. Multi-file change is well-coordinated across the stack.

Verdict: Approve

- Rename getPreviewPath() to resolvePreviewSource() for clarity and encapsulate logic for resolving the preview file source.
- Update getPreviewPath() to utilize the new method, returning an empty string if no preview file is set.
- Modify getPreviewSourceId() to return the source ID from the resolved preview source.
- Enhance documentation for clarity on return types and functionality.

@smg6511 smg6511 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For others testing, I found that the preview problem solved here only occurs when MODX is installed in a subdirectory.

Comment thread core/src/Revolution/Processors/Element/Template/GetList.php Outdated
Comment thread core/src/Revolution/Processors/Element/Template/GetList.php Outdated
@smg6511

smg6511 commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

@Ibochkarev - Hi Ivan, just a bump to take a look at the suggestions here. I'll review again once you have ;-)

Pass preview_file path and source id directly to phpThumb in Template/GetList
instead of resolving URLs via new modElement helpers. Reverts the broader
modMediaSource and PhpThumb changes from earlier iterations.

Fixes modxcms#16256
@Ibochkarev

Copy link
Copy Markdown
Collaborator Author

@Ibochkarev - Hi Ivan, just a bump to take a look at the suggestions here. I'll review again once you have ;-)

Done

@Ibochkarev Ibochkarev requested a review from smg6511 June 19, 2026 17:07

@smg6511 smg6511 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create document does not render template preview

3 participants