Opened 7 years ago

Closed 7 years ago

#3983 closed Bug (fixed)

Extra comma in toolbar definition causes JavaScript error in IE

Reported by: kschneider Owned by: fredck
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 7 years ago.
config file for reproducing
3983.patch (1.9 KB) - added by fredck 7 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 7 years ago by fredck

  • Milestone set to CKEditor 3.0

comment:2 Changed 7 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 7 years ago by garry.yao

config file for reproducing

comment:3 Changed 7 years ago by garry.yao

  • Component changed from General to UI : Toolbar
  • Keywords Confirmed added; Pending removed
  • Owner set to garry.yao
  • Status changed from new to assigned

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

comment:4 Changed 7 years ago by garry.yao

  • Resolution set to invalid
  • Status changed from assigned to closed

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 7 years ago by alfonsoml

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 7 years ago by kschneider

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 7 years ago by fredck

  • Keywords IE added
  • Resolution invalid deleted
  • Status changed from closed to reopened
  • Summary changed from Calling CKEDITOR.replace in ie8 causes javascript error to Extra 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 7 years ago by fredck

comment:8 Changed 7 years ago by fredck

  • Keywords Review? added
  • Owner changed from garry.yao to fredck
  • Status changed from reopened to new

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

comment:9 Changed 7 years ago by fredck

  • Status changed from new to assigned

comment:10 Changed 7 years ago by garry.yao

  • Keywords Review+ added; Review? removed

comment:11 Changed 7 years ago by fredck

  • Resolution set to fixed
  • Status changed from assigned to closed

Fixed with [3939].

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