Opened 10 years ago

Closed 10 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 10 years ago.
config file for reproducing
3983.patch (1.9 KB) - added by Frederico Caldeira Knabben 10 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 10 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.0

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

Attachment: 3983_TC.patch added

config file for reproducing

comment:3 Changed 10 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 10 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 10 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 10 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 10 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 10 years ago by Frederico Caldeira Knabben

Attachment: 3983.patch added

comment:8 Changed 10 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 10 years ago by Frederico Caldeira Knabben

Status: newassigned

comment:10 Changed 10 years ago by Garry Yao

Keywords: Review+ added; Review? removed

comment:11 Changed 10 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: assignedclosed

Fixed with [3939].

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