Opened 8 years ago

Closed 8 years ago

#3983 closed Bug (fixed)

Extra comma in toolbar definition causes JavaScript error in IE

Reported by: Kirk schneider Owned by: Frederico Caldeira Knabben
Priority: Normal Milestone: CKEditor 3.0
Component: UI : Toolbar Version:
Keywords: Confirmed IE Review+ Cc:

Description

When calling CKEDITOR.replace on this sample page http://72.197.202.64:33112/entries/show/5 with ie8 on a vista machine, I get the following Javascript Error

'length' is null or not an object ckeditor.js, line 67 character 1800

I do not get this error using Chrome or Firefox 3.5

Attachments (2)

3983_TC.patch (1.1 KB) - added by Garry Yao 8 years ago.
config file for reproducing
3983.patch (1.9 KB) - added by Frederico Caldeira Knabben 8 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 8 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.0

comment:2 Changed 8 years ago by Garry Yao

Keywords: Pending added

The link is unreachable for me, could you please provide a reduced sample page file to help us locate the bug?

Changed 8 years ago by Garry Yao

Attachment: 3983_TC.patch added

config file for reproducing

comment:3 Changed 8 years ago by Garry Yao

Component: GeneralUI : Toolbar
Keywords: Confirmed added; Pending removed
Owner: set to Garry Yao
Status: newassigned

I'm able to catch to problem now simply with the configuration change.

comment:4 Changed 8 years ago by Garry Yao

Resolution: invalid
Status: assignedclosed

It's not a bug, the only problem is the config array CKEDITOR.editorConfig.config.toolbar is containing a extra comma end with weird result in IE. You should safely remove it.

Below from ECMAScript262:
Array elements may be elided at the beginning, middle or end of the element list. Whenever a comma in the element list is not preceded by an Assignment Expression (i.e., a comma at the beginning or after another comma), the missing array element contributes to the length of the Array and increases the index of subsequent elements. Elided array elements are not defined.

comment:5 Changed 8 years ago by Alfonso Martínez de Lizarrondo

Note than in 2.x there was some extra checks in the toolbar initialization to avoid this problem as people loves to remove the "about" button and forget to correct the previous line.

comment:6 Changed 8 years ago by Kirk schneider

thanks for getting on this so quickly. I spent some time trying to isolate this and provide a simpler example but I couldn't. I guess because in my simpler examples I was using the default config.

CKEditor is a great tool!!!

comment:7 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: IE added
Resolution: invalid
Status: closedreopened
Summary: Calling CKEDITOR.replace in ie8 causes javascript errorExtra comma in toolbar definition causes JavaScript error in IE

This is actually a quite common mistake out there. People simply remove the toolbar rows from the definition (especially the About button), leaving that extra comma there.

In V2, to avoid confusion, we simply added a small check into the code for this specific case. We could do the same here also.

Changed 8 years ago by Frederico Caldeira Knabben

Attachment: 3983.patch added

comment:8 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review? added
Owner: changed from Garry Yao to Frederico Caldeira Knabben
Status: reopenednew

It's not said that we need to do such kinds of checks everywhere though.

comment:9 Changed 8 years ago by Frederico Caldeira Knabben

Status: newassigned

comment:10 Changed 8 years ago by Garry Yao

Keywords: Review+ added; Review? removed

comment:11 Changed 8 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: assignedclosed

Fixed with [3939].

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