Ticket #3912 (closed Bug: fixed)

Opened 8 months ago

Last modified 4 months ago

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)
Keywords: Confirmed IE Review+ Cc: pomu@…

Description

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

3912.patch Download (423 bytes) - added by pomu0325 5 months ago.
3912_2.patch Download (2.3 KB) - added by garry.yao 4 months ago.

Change History

Changed 8 months ago by arczi

  • keywords WorksForMe Pending added

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

Changed 8 months ago by fredck

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

Changed 5 months 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

Changed 5 months 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 5 months ago by pomu0325

Changed 5 months ago by fredck

  • keywords Confirmed HasPatch added; WorksForMe Pending removed

Changed 5 months 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).

Changed 4 months 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 4 months ago by garry.yao

Changed 4 months 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.

Changed 4 months ago by fredck

  • keywords Review+ added; Review? removed

Changed 4 months ago by garry.yao

  • status changed from assigned to closed
  • resolution set to fixed

Fixed with [4519].

Note: See TracTickets for help on using tickets.