Opened 8 years ago

Closed 8 years ago

#3912 closed Bug (fixed)

UI color plugin fails in IE with 3+ editors

Reported by: highjinx_53 Owned by: garry.yao
Priority: Normal Milestone: CKEditor 3.1
Component: General Version: SVN (CKEditor) - OLD
Keywords: Confirmed IE Review+ Cc: pomu@…


With three or more editors in the same page in IE, trying to change the UI color for all editors causes a JS exception when changing the color for the third editor.

The specific problem is in skins/kama/skin.js, line 153: uiStyle.$.styleSheet is null.

Attachments (2)

3912.patch (423 bytes) - added by pomu0325 8 years ago.
3912_2.patch (2.3 KB) - added by garry.yao 8 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 8 years ago by arczi

  • Keywords WorksForMe Pending added

I was unable to reproduce this bug (IE7 and 8). Could you provide more information?

comment:2 Changed 8 years ago by fredck

You may attach a test page for it, including the precise steps to be taken to reproduce it.

comment:3 Changed 8 years ago by pomu0325

  • Cc pomu@… added

Try with html which has a lot of <style> elements. kama skin seems to add <style> element during initialization, when it reaches 30, the 'uiStyle.$.styleSheet is null' error occurs.

refer: All style tags after the first 30 style tags on an HTML page are not applied in Internet Explorer

comment:4 Changed 8 years ago by pomu0325

Here is the way to reproduce this bug with CKEditor 3.0.1 sample.

  • Enable uiColor in default config.js
    config.uiColor = '#AADC6E';
  • Open CKEditor samples page with IE, and go to 'Create and destroy editor instances for Ajax applications'
  • Repeat 'Create' and 'Remove' editor for 30 times
  • After 30 times of clicking, '$.styleSheet is null.' error will occur

If you inspect DOM tree at this time, you can see redundant of <style id="cke_ui_color"> elements. I'll attach a possible patch to remove these <style> tags on editor destroy.

Changed 8 years ago by pomu0325

comment:5 Changed 8 years ago by fredck

  • Keywords Confirmed HasPatch added; WorksForMe Pending removed

comment:6 Changed 8 years ago by fredck

  • Keywords IE added
  • Milestone set to CKEditor 3.1

@pomu0325, thanks for the patch. It's wonderful to have simple solutions for issues like that.

We could go ahead with this patch idea for now, but I was wondering if we would not be able to instead reuse a single <style> element for all editors. In this way we can also make it work with a page with more than 30 editors on it (crazy thing, I know, but just in case).

comment:7 Changed 8 years ago by garry.yao

  • Keywords Review? added; HasPatch removed
  • Owner set to garry.yao
  • Status changed from new to assigned

Manual TCs added at : http://ckeditor.t/tt/3912/1.html

Changed 8 years ago by garry.yao

comment:8 Changed 8 years ago by garry.yao

Attaching a new patch following Fred's idea inspired by pomu0325.

P.S. You'll be regret about not seeing this TC.

comment:9 Changed 8 years ago by fredck

  • Keywords Review+ added; Review? removed

comment:10 Changed 8 years ago by garry.yao

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

Fixed with [4519].

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