Opened 6 years ago

Closed 6 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)

10131.html (6.0 KB) - added by Piotrek Koszuliński 6 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 6 years ago by dggtydnk

Version: 4.04.0.1

Changed 6 years ago by Piotrek Koszuliński

Attachment: 10131.html added

comment:2 Changed 6 years ago by Piotrek Koszuliński

Status: newconfirmed

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:

  • lock - called from sample
  • lock - called before fixDom triggered after sample called insertElement
  • unlock - called after fixDom

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.

Last edited 6 years ago by Piotrek Koszuliński (previous) (diff)

comment:3 Changed 6 years ago by Jakub Ś

Keywords: undo toolbar state removed
Version: 4.0.14.0

Events lockSnapshot and unlockSnapshot were introduced in CKEditor 4.0 so this is the starting point.

comment:4 Changed 6 years ago by Alfonso Martínez de Lizarrondo

A minor note: the documentation for those events lack the @since to state that 4.0 version.

comment:5 Changed 6 years ago by Piotrek Koszuliński

Thanks Alfonso, fixed docs with git:0b97fc7.

comment:6 Changed 6 years ago by Piotrek Koszuliński

Owner: set to Piotrek Koszuliński
Status: confirmedassigned

comment:7 Changed 6 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.1.1
Status: assignedreview

Pushed t/10131 on dev and tests.

comment:8 Changed 6 years ago by Frederico Caldeira Knabben

Status: reviewreview_passed

Let's move this ahead so we can see the effects of it in practice.

comment:9 Changed 6 years ago by Piotrek Koszuliński

Resolution: fixed
Status: review_passedclosed

Fixed on master with git:25d174d and on tests with 312b9b3.

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy