Opened 7 years ago

Closed 7 years ago

#3925 closed Bug (fixed)

FCKDialog.OpenDialog() - wrong variable mapping (parentWindow)

Reported by: ivantcholakov Owned by: martinkou
Priority: Normal Milestone:
Component: UI : Dialogs Version: FCKeditor 2.6.4
Keywords: Confirmed Review+ Cc:

Description

FCKEditor 2.6.4 This is not a "theoretical" note, I found this bug during development of something.

See .../_source/internals/fckdialog.js

The parameter *parentWindow* in the function OpenDialog() declaration and and the variable *topWindow* that sets a property of the dialogInfo object are the same thing actually. Only one should stay - parentWindow or topWindow.

Here is an example for a correction:

var FCKDialog = ( function()
{

  ...

	return {
		/**
		 * Opens a dialog window using the standard dialog template.
		 */
		OpenDialog : function( dialogName, dialogTitle, dialogPage, width, height, customValue, parentWindow, resizable )
		{

			...

      // Setup the dialog info to be passed to the dialog.
			var dialogInfo =
			{
				Title : dialogTitle,
				Page : dialogPage,
				Editor : window,
				CustomValue : customValue,		// Optional
				// -------------- A correction starts here ---------------------
				//TopWindow : topWindow       // Wrong
				TopWindow : parentWindow      // Correct
				// -------------------------------------------------------------
			}

			...

		},

  ...

Attachments (2)

3925.patch (4.6 KB) - added by martinkou 7 years ago.
3925_2.patch (5.0 KB) - added by martinkou 7 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 7 years ago by ivantcholakov

My Previous correction was wrong, topWindow and parentWindow are not the same. But anyway I need to access parrentWindow when I close a dialog. Here is another modification that works fine for me:

var FCKDialog = ( function()
{

  ...

	return {
		/**
		 * Opens a dialog window using the standard dialog template.
		 */
		OpenDialog : function( dialogName, dialogTitle, dialogPage, width, height, customValue, parentWindow, resizable )
		{

			...

      // Setup the dialog info to be passed to the dialog.
			var dialogInfo =
			{
				Title : dialogTitle,
				Page : dialogPage,
				Editor : window,
				CustomValue : customValue,		// Optional
				TopWindow : topWindow,
				ParentWindow : parentWindow		// <----------- Added property
			}

			...

		},

  ...

comment:2 Changed 7 years ago by martinkou

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

The parentWindow argument is actually useless - if you need access to the parent dialog's window, use

ParentDialog().contentWindow

instead.

Changed 7 years ago by martinkou

comment:3 Changed 7 years ago by martinkou

  • Keywords Review? added

Proposed patch to remove parentWindow reference to reduce code size and avoid confusion.

comment:4 Changed 7 years ago by ivantcholakov

Thank you, martinkou. Your suggestion works.

I had a look at your patch. I think, there is one place that was missed to be updated:

File: fckeditor/editor/_source/commandclasses/fck_othercommands.js
Line 189: FCKDialog.OpenDialog( 'FCKDialog_Source', FCKLang.Source, 'dialog/fck_source.html', iWidth, iHeight, null, null, true ) ;

Changed 7 years ago by martinkou

comment:5 Changed 7 years ago by martinkou

Thanks for the correction, updated the patch.

comment:6 Changed 7 years ago by fredck

  • Keywords Confirmed added
  • Milestone FCKeditor 2.6.5 deleted

comment:7 Changed 7 years ago by alfonsoml

  • Keywords Review+ added; Review? removed

comment:8 Changed 7 years ago by martinkou

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

Fixed with [3975].

Click here for more info about our SVN system.

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