fix: resolve inline image bugs with class, link, toggle, and indicators#658
fix: resolve inline image bugs with class, link, toggle, and indicators#658
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Summary of ChangesHello @CybotTM, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses five specific bugs related to inline image handling within CKEditor, focusing on improving class assignment, link management, image type toggling, and visual indicators. The changes ensure more robust and predictable behavior for images, preventing issues like empty classes, double links, and incorrect class persistence, ultimately enhancing the editor's stability and user experience. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request provides a comprehensive set of fixes for several bugs related to inline image handling in CKEditor. The changes address issues with default classes, duplicate link tags, class leakage between elements, and UI indicator consistency. The approach is solid, with fixes in the JavaScript plugin logic and corresponding CSS adjustments. The addition of new regression tests in PHP to verify the JavaScript source patterns is a great way to prevent these issues from recurring.
I have one suggestion for improvement in the JavaScript file to enhance consistency and maintainability regarding how alignment classes are handled.
There was a problem hiding this comment.
Pull request overview
This PR fixes multiple CKEditor inline image edge cases reported in #655, focusing on ensuring consistent class defaults, preventing double-link wrapping during upcast, keeping link-related classes clean when toggling inline/block, and avoiding duplicate link indicators in the editor UI.
Changes:
- Default newly inserted images to
image-block(orimage-inline) when the dialog returns an empty class string. - Adjust inline image upcast to prevent double
<a>creation and to strip alignment classes fromimageLinkClass. - Clean alignment classes from
imageLinkClasswhen toggling inline/block, and suppress CKEditor’s built-in link indicator to avoid duplicate badges.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Tests/Unit/JavaScript/Typo3ImagePluginTest.php | Adds regression tests for #655 and refactors JS loading into setUp(). |
| Resources/Public/JavaScript/Plugins/typo3image.js | Implements default class handling, link upcast consumption/filtering, and toggle cleanup of imageLinkClass. |
| Resources/Public/Css/editor-image-widget.css | Hides CKEditor’s built-in link ::after indicator to avoid duplicate link badges. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #658 +/- ##
=======================================
Coverage 93.04% 93.04%
=======================================
Files 27 27
Lines 2286 2286
=======================================
Hits 2127 2127
Misses 159 159 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
52bb44f to
5358e70
Compare
5358e70 to
353302e
Compare
353302e to
ae70e64
Compare
…rs (#655) Bug 1: New block images get empty class — default to 'image-block' (new only) Bug 2a: Inline upcast creates double <a> — consume parent link element Bug 2b: Alignment classes leak to imageLinkClass — filter in inline upcast Bug 3: Toggle inline→block keeps image-inline on <a> — clean imageLinkClass Bug 4: Double link indicators — suppress CKEditor's built-in ::after icon Bug 5: Backend preview styling docs — added backend.css guidance Closes #655 Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
ae70e64 to
655e089
Compare
Summary
Fixes 5 bugs reported in #655 involving inline image handling in CKEditor:
edit()now defaults toimage-block(orimage-inline) when the dialog returns an empty class string for newly inserted images<a>tags: Inline image upcast now consumes the parent<a>element to prevent CKEditor's link plugin from double-processing itimageLinkClass: Inline upcast filtersimage-inline,image-block, etc. from link class before storing asimageLinkClass(same pattern already used in block upcast)image-inlinepersists on<a>after toggle:ToggleImageTypeCommandnow strips alignment classes fromimageLinkClasswhen switching between inline/block::afterlink icon onfigure.image > asince we provide custom.ck-image-indicator--linkbadgesBug 2c (t3:// link not resolved on FE) is a downstream consequence of bug 2a — the double
<a>tags cause the outer link to bypass TYPO3's link resolution. Fixed by fixing 2a.Bug 5 (preview image docs) is an enhancement request tracked separately.
Test plan
image-blockclass applied<a>, no double wrappingimage-inlineremoved from<a>Closes #655