Opened 3 years ago

Closed 3 years ago

#13388 closed Bug (fixed)

Tableresize plugin is not integrated with undo manager

Reported by: Prem Owned by: Szymon Cofalik
Priority: Normal Milestone: CKEditor 4.5.3
Component: General Version:
Keywords: Cc:

Description (last modified by Piotrek Koszuliński)

When table cells are re-sized with the mouse, cells are re-sized but when we save the CkEditor data is not updated with the new cell widths. Tableresize plugin not firing change event.

we need to call editor.fire('change') for this to work properly in the init function


Edit: The goal is that resizing was undoable and that the #change event was fired once user _finishes_ resizing.

Change History (13)

comment:1 Changed 3 years ago by Jakub Ś

Status: newpending

Could you tell me which table resize plugins you are talking about? Is it native table resize (available in IE and FF) or CKSource resize which allows resizing table columns?

In case of native resize this is a duplicate of #10979. In case of CKEditor resize this is valid ticket.

Please tell us which feature you are talking about.

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

comment:2 Changed 3 years ago by Prem

Hi, It is a native resize that is available in http://ckeditor.com/addon/tableresize.

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

Description: modified (diff)
Status: pendingconfirmed
Summary: Tableresize not changing model i.e not firing onchangeTableresize plugin is not integrated with undo manager

By "native" we mean features/behaviours implemented by browsers or OS. The tableresize plugin is a JS plugin, so it is not a native behaviour.

Anyway, I confirm that the tableresize plugin does not fire snapshots so it is not integrated with undo manager at all. And it is undo manager who fires the #change event.

comment:4 Changed 3 years ago by Szymon Cofalik

Owner: set to Szymon Cofalik
Status: confirmedassigned

comment:5 Changed 3 years ago by Szymon Cofalik

Status: assignedreview

Fixed pushed to branch:t/13388

comment:6 Changed 3 years ago by Szymon Cofalik

Rebased branch:t/13388 with major.

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

Milestone: CKEditor 4.5.3
Status: reviewreview_failed

I pushed two more commits to the branch.

In one of them you will find a manual test which didn't pass (typing was part of resizing's undo step). In the second commit there's how I think the code should look like. If you agree, please write an automated test for it. The way how we simulate typing is that we simply set innerHTML of some element (table cell in this case). It will be enough for this test.

comment:8 Changed 3 years ago by Szymon Cofalik

Status: review_failedreview

There was already a unit test, but I improved it.

Rebased with master and pushed branch:t/13388

comment:9 Changed 3 years ago by Olek Nowodziński

Status: reviewreview_failed

I pushed a commit to branch:t/13388 and rebased in on master.

The fix is OK. But it looks like the scenario of MT is wrong as it passes both on master and branch:t/13388. Only when I repeat 2. and 3. a couple of times, the bug is reproducible. You can extend the existing scenario or suggest something totally different.

comment:10 Changed 3 years ago by Szymon Cofalik

Pushed branch:t/13388

I expanded current test.

comment:11 Changed 3 years ago by Szymon Cofalik

Status: review_failedreview

comment:12 Changed 3 years ago by Olek Nowodziński

Status: reviewreview_passed

comment:13 Changed 3 years ago by Olek Nowodziński

Resolution: fixed
Status: review_passedclosed

Merged git:730e54b into master.

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