Opened 12 years ago
Closed 12 years ago
#10131 closed Bug (fixed)
undoManager.update does not refresh command state
Reported by: | dggtydnk | Owned by: | Piotrek Koszuliński |
---|---|---|---|
Priority: | Normal | Milestone: | CKEditor 4.1.1 |
Component: | Core : Undo & Redo | Version: | 4.0 |
Keywords: | Cc: |
Description
Calls to the update() in undo Manager should also call undoManager.onChange() to refresh the state of the toolbar. I have a command that does something like:
editor.fire("saveSnapshot"); editor.fire("lockSnapshot"); var span = new CKEDITOR.dom.element("span"); span.setText("Loading..."); editor.insertElemenent(span); // load data into span via AJAX call here - code omitted function ajaxLoaded(contents) { span.setText(contents); editor.fire("unlockSnapshot") }
I lock the snapshot before setting the "Loading..." text so it does not appear in the undo history stack.
I am left with a toolbar that has both the undo and redo buttons on, even though undoManager.redoable() is false. I think if undoManager.fireChange() (or even just undoManager.onChange()) were called in undoManager.update() (since update() is called in undoManager.unlock()), the toolbar would be in the proper state.
Attachments (1)
Change History (10)
comment:1 Changed 12 years ago by
Version: | 4.0 → 4.0.1 |
---|
Changed 12 years ago by
Attachment: | 10131.html added |
---|
comment:2 Changed 12 years ago by
Status: | new → confirmed |
---|
comment:3 Changed 12 years ago by
Keywords: | undo toolbar state removed |
---|---|
Version: | 4.0.1 → 4.0 |
Events lockSnapshot and unlockSnapshot were introduced in CKEditor 4.0 so this is the starting point.
comment:4 Changed 12 years ago by
A minor note: the documentation for those events lack the @since to state that 4.0 version.
comment:6 Changed 12 years ago by
Owner: | set to Piotrek Koszuliński |
---|---|
Status: | confirmed → assigned |
comment:7 Changed 12 years ago by
Milestone: | → CKEditor 4.1.1 |
---|---|
Status: | assigned → review |
Pushed t/10131 on dev and tests.
comment:8 Changed 12 years ago by
Status: | review → review_passed |
---|
Let's move this ahead so we can see the effects of it in practice.
comment:9 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | review_passed → closed |
Fixed on master with git:25d174d and on tests with 312b9b3.
I attached sample. Open it, click both buttons and see that undo recorded all states. This is incorrect behaviour - undo shouldn't record changes done between lock and unlock.
I checked that this usage of lock/unlock is conflicting with fixDom which does the same - locks and unlocks undoM. So there is:
All changes done later are recorded.
Thus, undoM#locked should be an integer counting how deeply it's locked :). Two lock calls should require two unlock calls to unlock undoM.