Opened 8 years ago

Closed 8 years ago

#3912 closed Bug (fixed)

UI color plugin fails in IE with 3+ editors

Reported by: Josh Nisly Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.1
Component: General Version: SVN (CKEditor) - OLD
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 (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 Artur Formella

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 Frederico Caldeira Knabben

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

Attachment: 3912.patch added

comment:5 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Confirmed HasPatch added; WorksForMe Pending removed

comment:6 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: IE added
Milestone: 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: newassigned

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

Changed 8 years ago by Garry Yao

Attachment: 3912_2.patch added

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 Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

comment:10 Changed 8 years ago by Garry Yao

Resolution: fixed
Status: assignedclosed

Fixed with [4519].

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