Opened 10 years ago

Closed 10 years ago

#9112 closed Bug (invalid)

Some table operations are causing javascript errors

Reported by: Lynne Kues Owned by: Garry Yao
Priority: Normal Milestone:
Component: General Version: 3.2
Keywords: IBM Cc:

Description

FF ESR 10.0.4

Insert a table. plugins/dialog/plugin.js, function changeFocus(offset) causes javascript error (see first attachment)

Press okay with default values to insert the table. Press the tab key twice. plugins/tab/plugin.js, function selectNextCellCommand (backward) causes javascript error (see second attachment)

In both cases the operations appear to work properly and maybe this is working as designed; however, it would be preferable to guard against the error condition so that the javascript error doesn't occur.

Attachments (3)

table-err1.png (25.3 KB) - added by Lynne Kues 10 years ago.
table-err2.png (17.9 KB) - added by Lynne Kues 10 years ago.
9112.patch (9.2 KB) - added by Garry Yao 10 years ago.

Download all attachments as: .zip

Change History (8)

Changed 10 years ago by Lynne Kues

Attachment: table-err1.png added

Changed 10 years ago by Lynne Kues

Attachment: table-err2.png added

comment:1 Changed 10 years ago by Lynne Kues

Is this, by any chance, cleaned up in 3.6.4?

comment:2 Changed 10 years ago by Jakub Ś

Status: newconfirmed
Version: 3.6.33.2

Image dialog has almost the same TC - #6205.


To reproduce:

  1. Open replacebycode smaple in Firefox
  2. Turn on Firebug and go to Console tab
  3. Press "Break On All Errors" button (Pause button located to the left. It's right under firebug icon)
  4. Open Table dialog. You will get:

Message: focusList[current] is undefined
Line: 377
URI: /3.6.3/ckeditor/_source/plugins/dialog/plugin.js
Reproducible from CKEditor 3.2

  1. Press okay with default values to insert the table. Press the tab key you will get:

Message: next is undefined
Line: 53
URI: /3.6.3/ckeditor/_source/plugins/tab/plugin.js
Reproducible from CKEditor 3.5.4 rev. [6698]

Changed 10 years ago by Garry Yao

Attachment: 9112.patch added

comment:3 Changed 10 years ago by Garry Yao

Component: Core : TablesGeneral
Milestone: CKEditor 3.6.5
Owner: set to Garry Yao
Status: confirmedreview

For now let's avoid using unnecessary try...catch block, that we'd used to simplify condition checking, considering it has become a plain for developers to work with the "tracking error" feature provided by browser dev tools (at least Fx always break on caught exceptions).

comment:4 Changed 10 years ago by Garry Yao

#9303 has been marked as DUP.

comment:5 Changed 10 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.6.5
Resolution: invalid
Status: reviewclosed

There is certainly a reason why to have those try/catch blocks in the code. Those are not bugs, but controlled errors. Only a developer who wants to listen to try{} errors will see them happening, not normal users. We can just assume that it is the developer choice to listen to these errors and so it's his option to be disturbed by them.

If any editor use case is impacted by these try/catch blocks, then we'll definitely think about changing them. Otherwise, it is too risky to make changes now.

In any case, the patch is there. Make use of it at your own risk.

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