Ticket #3717 (closed Bug: fixed)

Opened 5 years ago

Last modified 5 years ago

config.dialog_backgroundCoverColor does not work in IE7 or IE8

Reported by: mrceolla2 Owned by: fredck
Priority: Normal Milestone: CKEditor 3.1
Component: UI : Dialogs Version: SVN (CKEditor) - OLD
Keywords: Confirmed Review+ Cc: mrceolla@…

Description

This is my first bug report for this project. Please let me know if I am doing anything incorrectly.

I have set a custom dialog_backgroundCoverColor in the editor host page before calling CKEDITOR.replace. It works correctly in FF3 but it does not work on IE7 or IE8 with compatibility mode on and off. Here is my script block inside an ASP page:

<script type="text/javascript">

CKEDITOR.config.toolbarLocation = 'Out:xToolbar'; CKEDITOR.config.height = '800px'; CKEDITOR.config.dialog_backgroundCoverColor = 'rgb(255, 0, 0)'; CKEDITOR.config.contentsCss = '../<%=folder%>style.css'; CKEDITOR.config.skin = 'office2003'; CKEDITOR.config.toolbar = [

['Source','-','Save','NewPage','Preview','-','Templates'], ['Cut','Copy','Paste','PasteText','PasteFromWord','-','Print', 'SpellChecker','Scayt'], ['Undo','Redo','-','Find','Replace','-','SelectAll','RemoveFormat'],

['Form', 'Checkbox', 'Radio', 'TextField', 'Textarea', 'Select', 'Button', 'ImageButton', 'HiddenField'],

'/', ['Bold','Italic','Underline','Strike','-','Subscript','Superscript'], ['NumberedList','BulletedList','-','Outdent','Indent','Blockquote'], ['JustifyLeft','JustifyCenter','JustifyRight','JustifyBlock'], ['Link','Unlink','Anchor'], ['Image','Flash','Table','HorizontalRule','Smiley','SpecialChar','PageBreak'], '/', ['Styles','Format','Font','FontSize'], ['TextColor','BGColor'], ['Maximize','ShowBlocks','-','About']

];

CKEDITOR.replace( 'editleftcol', {

filebrowser_browse_url : '/editor/ckfinder/ckfinder.html?action=js&func=SetUrl&thumbFunc=SetUrl', filebrowser_image_browse_url : '/editor/ckfinder/ckfinder.html?action=js&func=SetUrl&thumbFunc=SetUrl&Type=Images', filebrowser_flash_browse_url : '/editor/ckfinder/ckfinder.html?action=js&func=SetUrl&thumbFunc=SetUrl&Type=Flash', filebrowser_upload_url : '/editor/ckfinder/core/connector/php/connector.php?command=QuickUpload&type=Files', filebrowser_image_upload_url : '/editor/ckfinder/core/connector/php/connector.php?command=QuickUpload&type=Images', filebrowser_flash_upload_url : '/editor/ckfinder/core/connector/php/connector.php?command=QuickUpload&type=Flash'

});

CKEDITOR.replace( 'editrightcol');

</script>

Attachments

3717.patch (30.9 KB) - added by arczi 5 years ago.
3717_2.patch (2.5 KB) - added by arczi 5 years ago.
3717_3.patch (2.2 KB) - added by fredck 5 years ago.

Change History

comment:1 Changed 5 years ago by mrceolla2

  • Cc mrceolla@… added

comment:2 Changed 5 years ago by arczi

  • Status changed from new to assigned
  • Keywords Confirmed added
  • Owner set to arczi

Hmm. There is a small difference between config set in config.js and in plugins. The last method cannot be overwritten before replace() because we have

CKEDITOR.config.dialog_backgroundCoverColor = 'white';

So we need to replace it with

CKEDITOR.config.dialog_backgroundCoverColor = CKEDITOR.config.dialog_backgroundCoverColor || 'white';

(just like it is done in WSC plugin)

Changed 5 years ago by arczi

comment:3 Changed 5 years ago by arczi

  • Keywords Review? added

comment:4 Changed 5 years ago by garry.yao

  • Keywords Review- added; Review? removed

Are you sure this is the correct way to go? I think instead we should instead suggest user to always dedicate configurations to editor instance instead.

Changed 5 years ago by arczi

comment:5 Changed 5 years ago by arczi

  • Keywords Review? added; Review- removed

comment:6 Changed 5 years ago by fredck

  • Milestone changed from CKEditor 3.0 to CKEditor 3.1

I don't see how this patch can help with this ticket. Actually, using CKEDITOR.tools.extend may be a good option for us to fix this ticket. I mean, having all plugin related settings being defined with it.

In any case, we don't need to work on it right now. The packaged distribution contains all plugins into the ckeditor.js file, so it would be definitely possible to override this setting for all instances with CKEDITOR.config.dialog_backgroundCoverColor.

Other than that, it's always possible to define these settings by instance, avoiding any issues, so we can delay it for the 3.1.

comment:7 Changed 5 years ago by arczi

  • Owner arczi deleted
  • Status changed from assigned to new

comment:8 Changed 5 years ago by fredck

  • Keywords Review- added; Review? removed

My suggestion here is to instead not having the default color being initialized in the plugin file. We could instead simply have this setting documented, and use it in the code only if defined, failing down into its default value.

comment:9 Changed 5 years ago by fredck

  • Status changed from new to assigned
  • Owner set to fredck

Changed 5 years ago by fredck

comment:10 Changed 5 years ago by fredck

  • Keywords Review? added; Review- removed

This patch introduces a pattern we should consider using for our settings. There is no effective need to have them initialized in the CKEDITOR.config object. In this way we are able to set the CKEDITOR.config object in-page with the default settings we would like to have.

I'm proposing these changes for the dialog setting only to simply match this ticket needs, but we should consider adopting the same approach for all plugin settings.

comment:11 Changed 5 years ago by garry.yao

  • Keywords Review+ added; Review? removed

It's a good move, while it's noticed that cover background-color is not working on IE6, I'll open a new ticket for it.

comment:12 Changed 5 years ago by fredck

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

Fixed with [4364].

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