Opened 5 years ago

Closed 5 years ago

#8916 closed Bug (fixed)

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

Reported by: Satya Minnekanti Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 4.0
Component: Core : Keystrokes Version:
Keywords: IBM Webkit Cc: Damian, Teresa Monahan

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 (9)

comment:1 Changed 5 years ago by Jakub Ś

Keywords: Webkit added
Status: newconfirmed

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

What is the plan to resolve this ticket?

comment:3 Changed 5 years ago by Jakub Ś

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

comment:4 Changed 5 years ago by Jakub Ś

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

Component: GeneralCore : Keystrokes
Milestone: CKEditor 4.0
Owner: set to Garry Yao
Status: confirmedassigned

comment:6 Changed 5 years ago by Garry Yao

Status: assignedreview

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

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

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 5 years ago by Frederico Caldeira Knabben

Status: reviewreview_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 5 years ago by Garry Yao

Resolution: fixed
Status: review_passedclosed

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

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