Opened 7 years ago

Closed 7 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 7 years ago.
table-err2.png (17.9 KB) - added by Lynne Kues 7 years ago.
9112.patch (9.2 KB) - added by Garry Yao 7 years ago.

Download all attachments as: .zip

Change History (8)

Changed 7 years ago by Lynne Kues

Attachment: table-err1.png added

Changed 7 years ago by Lynne Kues

Attachment: table-err2.png added

comment:1 Changed 7 years ago by Lynne Kues

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

comment:2 Changed 7 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 7 years ago by Garry Yao

Attachment: 9112.patch added

comment:3 Changed 7 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 7 years ago by Garry Yao

#9303 has been marked as DUP.

comment:5 Changed 7 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 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy