Opened 13 years ago

Closed 10 years ago

#7975 closed Bug (fixed)

Errors when trying to select empty table cell on IE8

Reported by: Jakub Ś Owned by: Piotrek Koszuliński
Priority: Normal Milestone: CKEditor 4.4.2
Component: General Version: 3.5.3
Keywords: IBM IE Cc: Satya Minnekanti, Christophe Guillou, irinauru@…

Description (last modified by Jakub Ś)

The below description was copied from bug #7928 comment 3.

Problems using IE 7, 8 & 9 when using the elementspath ! Try the following :

  1. Create new table
  2. Put the cursor in a cell
  3. Click "td" in the elementspath : JAVASCRIPT ERROR

Message: 'undefined' is empty or not an Object
Line: 1578
URI: /ckeditor/_source/core/dom/range.js

This hasn't worked for IE6-10, in CKEditor 3.x, from CKEditor 3.5.3 rev [6559]. In CKEditor 4.x this doesn't work only for IE7 and IE8.

Attachments (1)

7975.patch (857 bytes) - added by Garry Yao 13 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 13 years ago by Jakub Ś

Status: newconfirmed

comment:2 Changed 13 years ago by Garry Yao

Owner: set to Garry Yao
Status: confirmedreview

Regression of [5463], can't reproduce anymore #5568 on trunk?

Changed 13 years ago by Garry Yao

Attachment: 7975.patch added

comment:3 Changed 12 years ago by Frederico Caldeira Knabben

Status: reviewreview_failed

With IE9+Compat:

  1. Load this HTML:
<table><tr><td>Test</td></tr></table>
  1. Click inside the cell.
  2. Click the "td" element in the elements path.

The first character will not get selected.

comment:4 Changed 11 years ago by Jakub Ś

#10122 was marked as duplicate.

comment:5 Changed 11 years ago by Jakub Ś

Description: modified (diff)

comment:6 Changed 10 years ago by Jakub Ś

Cc: Satya Minnekanti Christophe Guillou added
Keywords: IBM added
Milestone: CKEditor 4.4.2

#11991 was marked as duplicate.

comment:7 Changed 10 years ago by Irina

Cc: irinauru@… added

comment:8 Changed 10 years ago by Piotrek Koszuliński

Owner: changed from Garry Yao to Piotrek Koszuliński
Status: review_failedassigned

Pushed branch:t/7975. This solution needs tests (both manual and unit).

comment:9 Changed 10 years ago by Piotrek Koszuliński

Status: assignedreview

Pushed tests to branch:t/7975.

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

Summary: Errors when creating a table in IEErrors when trying to select empty table cell on IE8

comment:11 Changed 10 years ago by Piotr Jasiun

Status: reviewreview_failed

Are you sure that using cached collapsed and updating it makes sense at all? This collapsed is used twice (here and here) and when I replaced both with range.collapsed everything seems to work fine. Moreover we change the range before we use this cached collapsed for the second time. In this case it is not a problem, but caching part of the range when we manipulate it is a bad practice.

If there is a reason why collapsed is cached there should be a comment, otherwise we should use range.collapsed to avoid this problems in the future.

comment:12 Changed 10 years ago by Piotrek Koszuliński

Status: review_failedreview

You assume that you know what that value means. But none of us really do. There are operations made on range before the cached value is checked, so it's very probable that cached and property diverge often and perhaps that's an intended behaviour.

I don't want to spend a week on understanding this code. It's there for ages and it fixes many browser bugs, so expecting it to be straightforward is unwise.

Therefore, I focused on single case that has been reported in this ticket. My change affects only behaviour in tables and we'll get back to this code when next bug is reported.

comment:13 Changed 10 years ago by Piotr Jasiun

Status: reviewreview_passed

So beside this everything is fine. I rebased branch on the top of the master.

comment:14 Changed 10 years ago by Piotrek Koszuliński

Resolution: fixed
Status: review_passedclosed

Fixed on master with git:850b2e4.

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy