Opened 5 years ago

Closed 5 years ago

Last modified 5 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 5 years ago.
7811_2.patch (787 bytes) - added by garry.yao 5 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 5 years ago by garry.yao

  • Description modified (diff)
  • Milestone CKEditor 3.6 deleted
  • Status changed from new to confirmed
  • Version changed from 3.6 (SVN - 3.6.x) to 3.5.1

Changed 5 years ago by garry.yao

comment:2 Changed 5 years ago by garry.yao

  • Owner set to garry.yao
  • Status changed from confirmed to review

comment:3 Changed 5 years ago by satya

  • Cc satya_minnekanti@… added

comment:4 Changed 5 years ago by fredck

  • Status changed from review to review_failed

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

Changed 5 years ago by garry.yao

comment:5 Changed 5 years ago by garry.yao

  • Status changed from review_failed to review

comment:6 Changed 5 years ago by fredck

  • Status changed from review to review_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 5 years ago by garry.yao

  • Resolution set to fixed
  • Status changed from review_passed to closed

Fixed with [6874].

comment:8 Changed 5 years ago by wwalc

  • Milestone set to CKEditor 3.6.1
  • Resolution fixed deleted
  • Status changed from closed to reopened

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 5 years ago by wwalc

Reverted with [6877].

comment:10 Changed 5 years ago by fredck

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 5 years ago by satya

  • Keywords IBM added

comment:12 Changed 5 years ago by garry.yao

  • Resolution set to fixed
  • Status changed from reopened to closed

Fixed with [6913].

comment:13 Changed 5 years ago by fredck

#7889 has been marked as DUP.

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