Ticket #8916 (closed Bug: fixed)

Opened 3 years ago

Last modified 2 years ago

Safari: Press delete button in an empty cell deleting empty Rows below current row

Reported by: satya Owned by: garry.yao
Priority: Normal Milestone: CKEditor 4.0
Component: Core : Keystrokes Version:
Keywords: IBM Webkit Cc: damo, tmonahan

Description

To reproduce the defect:

  1. Open any CK Editor sample and insert a table with default values(3 Rows & 2 Columns).
  1. Don't enter any content in second or third rows.
  1. Enter the content in first cell in first row, leave second cell in first row empty.
  1. Keep cursor in the Second cell on first row & press Delete

Issue: Second row is deleted

  1. Press Delete again..

Issue: Next empty row gets deleted

Change History

comment:1 Changed 3 years ago by j.swiderski

  • Status changed from new to confirmed
  • Keywords Webkit added

Reproducible in Webkit from CKEditor 3.0

Notes:

  • There doesn't need to be any text in first row. Just put cursor in first row in right-most cell and press delete.
  • Similar to already closed #1722

comment:2 Changed 3 years ago by damo

What is the plan to resolve this ticket?

comment:3 Changed 2 years ago by j.swiderski

Just a guess but perhaps #8471 has the same cause for this behavior.

comment:4 Changed 2 years ago by j.swiderski

What is the plan to resolve this ticket?

As discussed with senior developers - This one will be aimed for v4 only, it's just complex for this release.

comment:5 Changed 2 years ago by garry.yao

  • Owner set to garry.yao
  • Status changed from confirmed to assigned
  • Component changed from General to Core : Keystrokes
  • Milestone set to CKEditor 4.0

comment:6 Changed 2 years ago by garry.yao

  • Status changed from assigned to review

Opened t/8916: Backspace/Del keystrokes are now handled at the boundary of table cells, to move cursor.

comment:7 Changed 2 years ago by Reinmar

I pushed rebased branch + small coding style fix.

Del/backspace behaviour is fixed on Webkits.

In the code I see that this fix is generic and I wonder if it shouldn't affect also IEs? I also think that this change may deserve a new TC.

comment:8 Changed 2 years ago by fredck

  • Status changed from review to review_passed

Making the change for all browsers makes sense, as it guarantees a unified behaviour among them, no matter their default behaviour. Considering that we're have that code for a single browser anyway, it is safe to just enlarge it to others.

I'm still not a big fan of making DEL a movement key. I prefer the IE behaviour for this TC, where nothing happens in this case.

Anyway, the proposed behaviour is "acceptable". Let's see if people will complain about it.

comment:9 Changed 2 years ago by garry.yao

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

There's a problem with having test coverage on keystroke handlers in v4, see #9300.

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