Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#7811 closed Bug (fixed)

[IE] Delete row throws JS error

Reported by: Garry Yao Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.6.1
Component: General Version: 3.5.1
Keywords: IE IBM Cc: satya_minnekanti@…

Description (last modified by Garry Yao)

Applies to IE<=8, delete table row now throws error, even though cursor position looks right, regression of [6328].

Attachments (2)

7811.patch (753 bytes) - added by Garry Yao 6 years ago.
7811_2.patch (787 bytes) - added by Garry Yao 6 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 6 years ago by Garry Yao

Description: modified (diff)
Milestone: CKEditor 3.6
Status: newconfirmed
Version: 3.6 (SVN - 3.6.x)3.5.1

Changed 6 years ago by Garry Yao

Attachment: 7811.patch added

comment:2 Changed 6 years ago by Garry Yao

Owner: set to Garry Yao
Status: confirmedreview

comment:3 Changed 6 years ago by Satya Minnekanti

Cc: satya_minnekanti@… added

comment:4 Changed 6 years ago by Frederico Caldeira Knabben

Status: reviewreview_failed

After patch, there is a buggy effect if you delete the last available row in a table. A cell-less table remains.

Changed 6 years ago by Garry Yao

Attachment: 7811_2.patch added

comment:5 Changed 6 years ago by Garry Yao

Status: review_failedreview

comment:6 Changed 6 years ago by Frederico Caldeira Knabben

Status: reviewreview_passed

I feel that the proper fix would be something in between those two patches.

When deleting the row, I expect the cursor to jump to the *next* row. That happened with the first patch. With the second one, it's now jumping to the *previous* row, which is wrong.

Please review the behavior on commit.

comment:7 Changed 6 years ago by Garry Yao

Resolution: fixed
Status: review_passedclosed

Fixed with [6874].

comment:8 Changed 6 years ago by Wiktor Walc

Milestone: CKEditor 3.6.1
Resolution: fixed
Status: closedreopened

Let's avoid introducing last minute changes when the second testing phase is about to finish. Even if it's a simple change it needs to be tested well.

comment:9 Changed 6 years ago by Wiktor Walc

Reverted with [6877].

comment:10 Changed 6 years ago by Frederico Caldeira Knabben

I'm sorry for having targeted it wrongly. I really though it was a 3.6 issue, since the beginning. Only after commit I saw it is an older issue. Well done, wwalc.

comment:11 Changed 6 years ago by Satya Minnekanti

Keywords: IBM added

comment:12 Changed 6 years ago by Garry Yao

Resolution: fixed
Status: reopenedclosed

Fixed with [6913].

comment:13 Changed 6 years ago by Frederico Caldeira Knabben

#7889 has been marked as DUP.

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