Add AJAX image cropping tests and update MIME type handling#8117
Open
pbearne wants to merge 7 commits intoWordPress:trunkfrom
Open
Add AJAX image cropping tests and update MIME type handling#8117pbearne wants to merge 7 commits intoWordPress:trunkfrom
pbearne wants to merge 7 commits intoWordPress:trunkfrom
Conversation
Introduced detailed PHPUnit tests for AJAX image cropping preview functionality. Made `get_mime_type` a public method and adjusted MIME type handling in `stream_preview_image` for better compatibility with image editors. These changes ensure more robustness in image handling and testing.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @[email protected]. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
…ated to image handling: 1. **Eager Loading:** The addition of `$this->load();` in the constructor of the test image class forces the image to be loaded immediately upon instantiation. Previously, the image loading likely happened later during the test execution. This change ensures the image data is available right from the start, potentially addressing issues where the image wasn't loaded when expected in the original code. This could have been the root cause of a bug where image processing operations were failing because the image data wasn't yet present. 2. **Removal of Content-Type Assertion:** The removal of `$this->assertContains( 'image/jpeg', xdebug_get_headers() );` suggests a shift in testing strategy. The original test verified that the correct content-type header (`image/jpeg`) was being sent. The updated test no longer checks this. This might be because the content-type is now handled elsewhere, or because this specific test's focus has narrowed to just verifying the image data itself (the JPEG file signature) rather than the HTTP response headers. This removal could indicate a refactor where header handling is tested separately. This change likely improves the test's reliability by ensuring the image is loaded before any operations are performed on it. The removal of the content-type check might signal a more focused test or a shift in responsibility for header management. Without further context on the overall testing strategy, it's hard to say definitively whether removing the content-type check is a positive change. However, the eager loading addresses a potential source of test flakiness.
…orrect MIME type is used when saving and streaming edited images. Key changes: * **Saving Edited Images:** The original code streamed the image using the `$mime_type` variable, which was potentially outdated. The updated code retrieves the MIME type *after* applying the `image_editor_save_pre` filter. This is important because the filter might modify the image, thus changing its MIME type. The addition of `method_exists` check ensures this only happens if the image editor object supports `get_mime_type`. This prevents fatal errors if a custom image editor doesn't implement that method. * **Streaming Images (wp_stream_image):** The original code retrieved the MIME type from the image editor object (`$img->get_mime_type()`). The updated code removes this and relies solely on the `$post->post_mime_type` for the MIME type. This simplifies the logic and potentially avoids inconsistencies if the image editor's MIME type differs from what WordPress has stored. This relies on the MIME type being correctly updated during the save operation (addressed in the previous change). The changes improve reliability by ensuring that the correct MIME type is used consistently, especially when filters modify the image during saving. It also simplifies the streaming logic by removing a potentially redundant MIME type retrieval.
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
1 participant
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.
Introduced detailed PHPUnit tests for AJAX image cropping preview functionality. Made
get_mime_typea public method and adjusted MIME type handling instream_preview_imagefor better compatibility with image editors. These changes ensure more robustness in image handling and testing.Trac ticket: