Conversation
|
[Claude Opus 5 (1M context) from Hatton's machine during preflight] Consulted Devin on 2026-09-16 up to commit Across three rounds it raised four distinct bugs, all in the new Ctrl+Z handling and all now fixed, each with its own resolved thread: a form field losing its own undo to the picture's, the same for an open dialog, standing aside for a toolbox undo that nothing then performed, and only recognising the React dialogs and not the older jQuery ones. The final round re-reported three of those against the very commit that fixes them — its own CI ( |
After "Edit with AI..." put a new picture on the page, Undo could not put back what was there: Ctrl+Z did nothing at all, and the Undo button brought the old picture file back without its cropping. Nothing listened for Ctrl+Z over a picture. Text undo belongs to the CKEditor instance that has focus and origami binds its own handler in layout mode, but an image change had neither, so only the Undo button in the top bar reached the image undo. Right after the AI editor closes the keystroke arrives in the top window, where the overlay lives, so both frames now handle it, and only when there is an image change to undo and the keystroke did not come from text. A crop is two boxes, not one: the img grows past its canvas element and slides under it, and the canvas element takes the shape of the part that shows. The undo record captured only the img's box, and put it back only on the background-image path, so undo restored an img sized for a canvas element that had meanwhile been reshaped to fit the picture being undone; the background fitter then rescaled the img to match, which is what erased the crop. The record now carries both boxes, and the restore tells the sizing code this is not a new image, so it keeps the cropping instead of fitting to the whole picture. Verified in a running Bloom over the real commit path, on a background image and on an ordinary picture canvas element, with a real Ctrl+Z and with the Undo button: src and both boxes come back to the values they had before the swap. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The new image-undo handlers took Ctrl+Z whenever there was an image change to undo. Origami binds its own Ctrl+Z to the page frame's html element while layout mode is on, so one keystroke there would have run both undos: the layout change and the image change. Give origami first refusal, exactly as the Undo button's handleUndo already does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…16868) Two gaps in the guards on the new image-undo handlers. The page frame's handler skipped only a rich-text editable, so with an ordinary form field focused in that frame Ctrl+Z replaced a picture instead of undoing what the user had just typed. Both frames now use one selector for the fields whose own undo we must not take. Neither handler noticed a modal. A dialog, and the AI Image Editor's own overlay, are elements of the top window, so with one of them open and focus on a button or the overlay itself, Ctrl+Z reached the page behind and replaced a picture the user could not even see. Both handlers now stop while either is up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Passing "not a new image" on the undo path does two things, not one: it keeps the cropping, and it switches off the wait for the restored src to load. The img goes on reporting the outgoing picture's natural dimensions until the restored one decodes, and the sizing code reads those whenever there is no crop to work from, so an undo back to a picture that had never been cropped was sized from the picture being undone away. There is no cropping to keep in that case, so treat it as a new image, which is what it did before and gets it the real dimensions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Undo button's handleUndo asks origami, then the active tool, then the image undo. The new Ctrl+Z handlers asked origami but not the tool, so with a reader tools undo and an image undo both pending on a page, the button undid the tool's change while the keystroke undid the picture, and the tool's undo could not be reached from the keyboard at all until the image stack emptied. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two things the last round got half right. Standing aside for a tool that has something to undo left Ctrl+Z doing nothing at all, because no tool listens for the keystroke itself; the Undo button is what performs a tool's undo. Perform it here as well, the way the button does. And "is a dialog open" only recognised the React dialogs. Bloom's older jQuery UI ones, Reader Setup among them, put up a .ui-dialog instead, so with one of those open Ctrl+Z still reached the picture on the page behind it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
6954c0b to
782d1ea
Compare
The top window's handler stood aside for origami the way the page frame's does, but the two are not alike: origami binds its handler to the page frame's html element, so when the keystroke arrives in the top window nothing else sees it and the layout change went un-undone. Perform it here, as the handler a few lines below already does for the toolbox and for the same reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
[Claude Opus 5 (1M context) from Hatton's machine during preflight] Consulted Devin on 2026-09-16 up to commit The rebase made Devin re-review the whole diff from scratch ( Five distinct bugs across all rounds now, every one in the new Ctrl+Z handling and every one fixed, each with its own resolved thread. The final round re-reports all five against the commit that fixes them; each was checked against |
JohnThomson
left a comment
JohnThomson
left a comment
There was a problem hiding this comment.
@JohnThomson reviewed 8 files and all commit messages, and resolved 4 discussions.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on hatton).
JohnThomson
left a comment
JohnThomson
left a comment
There was a problem hiding this comment.
@JohnThomson made 1 comment.
Reviewable status:
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2 participants
Problem
After Edit with AI... puts a new picture on the page, Undo cannot put back what was there. Ctrl+Z does nothing at all, and the Undo button in the top bar brings the old picture file back but uncropped — a picture the user had cropped returns showing the whole photo, at the wrong shape. Found by working through this card's own test ideas after PR #8356 merged.
Cause
Two independent gaps.
Nothing listened for Ctrl+Z over a picture: text undo belongs to the CKEditor instance that has focus, and origami binds its own handler in layout mode, but an image change had neither — only the Undo button reached the image undo. And right after the AI editor closes, the keystroke arrives in the top window, where the editor's overlay lives, not in the page frame.
Separately, a crop is two boxes: the
img, which grows past its canvas element and slides under it, and the.bloom-canvas-elementthat frames it and takes the shape of the part that shows. The undo record captured only the img's box, and restored it only on the background-image path. The canvas element had meanwhile been reshaped to fit the new picture, and the background fitter then rescaled the img to match — which is what erased the crop.Fix
IImageCropInfo.canvasElement).Targets master (6.6), not 6.5, although the card is marked [6.5]: this half of the card touches Bloom's undo mechanism, where a bug destroys a user's work rather than merely failing, and we are late in the 6.5 cycle. The other half of the card, sending the AI editor the cropped view of a picture, shipped to 6.5 in #8356 and is unaffected.
Deliberately out of scope: a background picture set to Fill (scaled up and cropped to cover the page) can still come back at the wrong scale, because the Fill arithmetic reads the outgoing picture's natural dimensions before the restored file has decoded. Fixing that means separating the two meanings currently carried by one flag, which several other callers share; it is filed as BL-16880 for 6.6.
Verified in a running Bloom over the real commit path (
applyAiImageEditorReplacements), on a background image and on an ordinary picture canvas element, with a real Ctrl+Z and with the Undo button's own handler: the src and both boxes come back to the values they had before the swap. Reverting the fix reproduced both reported symptoms exactly.Ref: https://issues.bloomlibrary.org/youtrack/issue/BL-16868
🤖 Generated with Claude Code
Devin review
This change is 