Use floating panels for dialogs

[Frederico Caldeira Knabben]

Users may have problems with popup blockers and FCKeditor dialogs. This is quite annoying for them, and we should propose a solution for it.

The most obvious one is implementing a new dialog mechanism which would use floating panes (FCKPanel) to load the dialogs pages.

An important detail in this feature is that the user must not be able to play with the editor area or toolbar while the dialog is opened. They must instead be able to interact with the rest of the page. Maybe it would be possible to cover the editor with a transparent thing (IFRAME) that would block all user actions.

[Frederico Caldeira Knabben]

Another note: responsiveness. It is important that the dialog is opened as soon as the user requests it. It could be also empty at startup with a nice "Loading..." message.

[Geir Helge Tjøstolvsen-Schmidt]

Replying to FredCK:

An important detail in this feature is that the user must not be able to play with the editor area or toolbar while the dialog is opened. They must instead be able to interact with the rest of the page. Maybe it would be possible to cover the editor with a transparent thing (IFRAME) that would block all user actions.

Just to clarify; Do want the transparent thing to cover the whole page behind the FCKeditor dialog or only the editor (including the toolbar) and not the rest of the page where the editor has been used? I'd prefer the first so that the user will have to close the FCKeditor dialog before she can do any other operation.

[Geir Helge Tjøstolvsen-Schmidt]

Replying to FredCK:

Another note: responsiveness. It is important that the dialog is opened as soon as the user requests it. It could be also empty at startup with a nice "Loading..." message.

Yes, it's important that the dialog is opened very quickly. I'm not convinced about the "Loading..." message. Maybe if it's possible to enable it for slow dialogs/plugins and drop it for others. The reason is that we've had some bad experience with such a "Loading" message that also had a nice progress bar while waiting. Users found it annoying because it was used to often and just flashed for a second before the page loaded.

[Frederico Caldeira Knabben]

Replying to geirhelge:

Just to clarify; Do want the transparent thing to cover the whole page behind the FCKeditor dialog or only the editor (including the toolbar) and not the rest of the page where the editor has been used? I'd prefer the first so that the user will have to close the FCKeditor dialog before she can do any other operation.

I'm still not sure about the preferred behavior. But, as dialogs usually block the access to the parent page, maybe a complete page lock would be better to avoid users being confused (like making changes to a dialog and submit the form without closing it).

[Frederico Caldeira Knabben]

Replying to geirhelge:

Yes, it's important that the dialog is opened very quickly. I'm not convinced about the "Loading..." message. Maybe if it's possible to enable it for slow dialogs/plugins and drop it for others. The reason is that we've had some bad experience with such a "Loading" message that also had a nice progress bar while waiting. Users found it annoying because it was used to often and just flashed for a second before the page loaded.

The thing is that the dialogs will still load files from the server when getting opened. We may think about a timer to display the loading message, or even not showing a textual message, but a simple graphical "thing" that shows you that the dialog is not simply locked.

[Alfonso Martínez de Lizarrondo]

#449 has been marked as dup

[Alfonso Martínez de Lizarrondo]

These bugs are about blocked popups: ​https://sourceforge.net/tracker/index.php?func=detail&aid=1450111&group_id=75348&atid=543653 ​https://sourceforge.net/tracker/index.php?func=detail&aid=1574269&group_id=75348&atid=543653

[Frederico Caldeira Knabben]

This will be one of the main new features in version 2.6.

[Frederico Caldeira Knabben]

#1510 has been marked as DUP

[Martin Kou]

The floating dialog branch at ​http://svn.fckeditor.net/FCKeditor/branches/features/floating_dialog/ is ready to be reviewed after [1177].

[Martin Kou]

After discussing with Alfonso, it was found that new floating dialog layout lacks the border space found in the old non-floating dialogs. The floating dialog code need to be amended to make them look the same as the old dialogs.

[Martin Kou]

I've added back the 10px border and added some logic to make the blocker iframes resize with the outer window in [1180].

[Frederico Caldeira Knabben]

Currently the branch is still not ready to commit. We'll be discussing about it, and ask for review later.

[Frederico Caldeira Knabben]

Martin, unfortunately, there are still some old things in the branch. I ask you to manually review the differences between the branch and the trunk, and update the branch so only related changes remain there. Please do not use the SVN to do this job this time, as it seems it is not giving you good results.

[Frederico Caldeira Knabben]

Using Fiddler, I've analyzed the HTTP requests made when opening a dialog. I tried it with the most simple of them: Special Chars. It fired 16 requests.

Consider that, FCKeditor itself makes 14 requests, including all toolbar images, configurations, language, XML files, etc. So, the dialog is making too many requests, which definitely impacts on performance. Actually, the number of requests is the most important thing for UI responsiveness.

The following requests have been made:

01. /skins/default/images/dialog.shadow.top.left.png
02. /skins/default/images/dialog.shadow.top.png
03. /skins/default/images/dialog.shadow.top.right.png
04. /skins/default/images/dialog.shadow.left.png
05. /skins/default/images/dialog.shadow.core.png
06. /skins/default/images/dialog.shadow.right.png
07. /skins/default/images/dialog.shadow.bottom.left.png
08. /skins/default/images/dialog.shadow.bottom.png
09. /skins/default/images/dialog.shadow.bottom.right.png
10. /skins/default/fck_background_blocker.html
11. /skins/default/fck_editor_blocker.html
12. /fckdialog.html?dialogId=121708304
13. /skins/default/fck_dialog.css
14. /skins/default/images/dialog.close.png
15. /dialog/fck_specialchar.html
16. /skins/default/images/dialog.loading.gif

Let's try to analyze it, looking for possible enhancements.

1. The first thing that comes to the eyes immediately is that we have 9 requests just for the dialog shadow. Other than that, many of those images are not visible: top.left, top, left and core. So as a first step, 01, 02, 04 and 05 could be removed, reducing the requests to 12.

2. Then, all shadow images could be merged into a single file, using CSS positioning, just like we do with the toolbar icons. It would reduce our requests to 8. (Note: at least on IE, this image would be loaded right after loading the editor, to be cached before use, bringing a small enhancement in that browser).

3. I have the impression that 10 and 11 could be avoided, by creating the IFRAMEs and filling them on the fly. The colors could come from the fck_editor.css skin file. It would reduce the requests to 6. The same thing should used to avoid using fck_dialog_blocker.html (not listed here). We could have a generic function that creates and fills and IFRAME.

4. For 12, we must still discuss about the dialogId, to check if it is really needed to avoid caching the file.

5. Then we have 13, 14 and 15, which are all needed. We could think about a preloading system for 13 and 14 (and 16), but not at this moment.

6. For 16 we could think about a timer to display it (500ms?) to avoid flashing it on speedy connections. Even better, we could use a textual "Loading..." style message, avoiding using an image for it. If an elegant solution is found for it, that would be better, reducing the requests to only 5.

Any thoughts on this?

[Frederico Caldeira Knabben]

To reduce the size of the core code, it would be better to have any kind of code that strictly depends on the dialog.html file, to be placed in that file. Maybe some things that are in the core code be moved, like shadows creation/handling, dialog resize/close and dragging.

Does it makes sense?

[Frederico Caldeira Knabben]

Debug must be "false" in fckconfig.js. You may also leave it "true" locally, but it should never be committed as "true".

[Frederico Caldeira Knabben]

Instead of:

function _FCKDialog_Drag_Handlers()
{
    ... do stuff
}
_FCKDialog_Drag_Handlers() ;

... there should be:

{function()
{
    ... do stuff
})() ;

Am I wrong?

[Frederico Caldeira Knabben]

There are some function parameters called "yes"... well, there is not much to say about it :)

[Frederico Caldeira Knabben]

Would it be nice to have "cursor:move" in the draggable part of the dialog on the top? I could make the dragging feature more evident, as we don't have a standard title bar.

[Frederico Caldeira Knabben]

With IE7, the "About" dialog has the close bottom moved to the left. In Firefox, I'm able to see the scrollbars in the dialog IFRAME, maybe this is the reason.

[Frederico Caldeira Knabben]

In FF, the "dialog construction" effect is quite visible and... well.. annoying and ugly :P

We must check it, making the dialog visible only when all its elements are available for visibility. The "About" dialog is a good test for it... just go ahead opening it many times to check the effect.

[Frederico Caldeira Knabben]

Regarding point 3 from one of my previous comments, not only the blockers colors, but also the opacity should be configurable in the fckconfig.js.

[Frederico Caldeira Knabben]

The BackgroundBlocker (black) is also over the editor. It should be behind it, shouldn't it?

Otherwise, it makes less sense having the specific EditorBlocker in a different color (I've tried to make the editor more visible than the rest of the page.

[Frederico Caldeira Knabben]

Is there any motivation on having SetDialogMode inside FCK, instead of FCKDialog?

[Scott McNaught]

Hey guys,

I tried to do something similar to fix this popup blocker problem... But my efforts failed because the editor lost its selection properties as soon as the user focussed on one of the edit boxes in the dialog window. Is there any way around this? I tried all sorts of hacks such as saving a bookmark... I still am yet to find an elegant solution to this problem.

PS: Is there really a need for you to use an iframe as the transparent veil to stop people from accessing the page behind?

PS 2: That clicky noise can be stopped if you remove all changes to the src property on the dialog iframes. If you need to change the source via script, you should also create the iframe via script at the same time.

Eg:

function LoadInnerDialog()
{
	if ( window.onresize )
		window.onresize() ;

	// First of all, translate the dialog box contents.
	window.dialogArguments.Editor.FCKLanguageManager.TranslatePage( document ) ;
	
	var oIFrame = document.createElement('iframe') ;
	oIFrame.id = 'frmMain' ;
	oIFrame.name = 'frmMain' ;
	oIFrame.src = window.dialogArguments.Page ;
	oIFrame.width = '100%' ;
	oIFrame.height = '100%' ;
	oIFrame.scrolling = 'auto' ;
	oIFrame.frameBorder = 0 ;
	document.getElementById('FrameCell').appendChild(oIFrame) ;
}

[Frederico Caldeira Knabben]

Dialog Frame Proposal

[Frederico Caldeira Knabben]

I've attached a proposal for the dialog structure. It uses pure CSS and absolute positioning to build the dialog.

Some notes about it:

• The dialog is build from inside the floating IFRAME, not from the main window.

• Shadows work on Safari, Opera, Firefox and IE7. The solutions is quite simply... they are part of the border image. It degrades on IE6 and IE5.5, where no shades are shown.

• It gives a lot of flexibility for the design, making it possible to have shadows, and rounded corners.

• It is customized for RTL UI.

• Almost all images used for the borders come from a single sprite image. It was not possible to put everything in the sprite image. Another image is necessary for the left and right borders only.

• The following files are loaded:

  dialog.html
  css/dialog.css
  css/sprites.png
  css/dlg-sides.png
  

• The close button (including mouse-over state) images have been placed in the sprite.png file too, so we can use CSS to display them too. Actually, we could think about using the sprites.png images for all images used by the skin, including toolbar assets.

• For rendering performance, as few elements as possible had been used in the page structure: eight divs for the corners and borders, and one for the content.

Should this one be the way to go for it?

PS: Hopefully some day IE6 will die.

[Scott McNaught]

But... how are you guys going to keep track of what was selected / the cursor position prior to someone opening a dialog? As soon as the user focusses on any thing in the dialog, you lose your selection in the editor.

[Alfonso Martínez de Lizarrondo]

Have you tested the branch with the code?

It seems to work fine for me, martinkou has worked out lots of little problems in order to reach this status.

[Scott McNaught]

Yeah I have. Sorry my bad... My test were only with Internet Explorer. I didn't think that any thought had been put into this yet. So I was trying to raise it as a red flag issue.

I tried it in firefox and it works fine.

Under IE however, you cannot edit an image, or a link. Which is why I thought this hadn't been thought through.

[Martin Kou]

Replying to Scott:

Yeah I have. Sorry my bad... My test were only with Internet Explorer. I didn't think that any thought had been put into this yet. So I was trying to raise it as a red flag issue.

I tried it in firefox and it works fine.

Under IE however, you cannot edit an image, or a link. Which is why I thought this hadn't been thought through.

Thanks to the bug report. I've fixed the issue with [1186] in the floating_dialog branch.

[Scott McNaught]

Thanks martinkou.

Hey just a suggestion for improving the user-friendlyness of this. After an image or a table has been inserted, can we give it control focus?

[Martin Kou]

Replying to fredck:

The 9 shadow images are separated because they are loaded with AlphaImageLoader on IE, which does not have any coordinate shifting feature for us to use sprites. If they were to be condensed into a sprite image, CSS background styles must be used, and the browser must support transparent PNGs (IE6 doesn't). So if a sprite image is used then the dialog shadow would have to be eliminated in IE6, which I see you have done in the proposed .zip file.

The blocker .htmls were implemented as HTML files for flexibility. Since they cover a large area I expected people might want to put other things into them other than a simple grayish or white color (e.g. borders?). They can be eliminated into purely JavaScript IFRAMES if needed (i.e. create iframe in JavaScript, document.open(), document.write(fixed_html_code), document.close()). It just depends on whether performance or feature is of higher priority.

The background blocker is going above the editor iframe because the editor iframe is static layout. I've already set the background blocker's z-index to 0 and placed it after the editor iframe but it's of no help. I've tried giving "position: relative" to the editor iframe but that caused serious problems when there are multiple editor instances (just try it on sample11.html, set both editor iframes to relative position via Firebug and open a dialog for each of them).

I guess the dialog construction is visible due to the large number of iframes used. Every time I open a floating dialog the very first thing that appears is the shadow - it is fairly complicated but it is not an iframe. After the shadow appears the blockers appear - they are iframes but they are simple. After that, the dialog contents appears. While the TCP delays created by the 16 loading files might be a contributing factor, the original popup dialogs contents can also be seen constructing themselves. If a check were to be implemented, then it should be checking on whether the dialog contents have been rendered completely. The dialog contents might have to be placed somewhere invisible (say, top=-10000px) while it's being rendered.

[Martin Kou]

Replying to fredck:

About the dialog proposal... Is there any reason the border images include a solid border in addition to the translucent shadow? I'm coding a cover iframe for the proposed dialog structure but I'm finding myself having to account for those +2/-2 border sizes in my size calculations (can't do it with pure CSS). But those +2/-2 sizes might be different in different skins.

[Martin Kou]

Proposed dialog structure with iframe and inner shadow cover.

[Frederico Caldeira Knabben]

I've attached yet another proposal. It contains the "inactive cover", which is essentially a styled div.

I've avoided always having the blocker iframe, as it is actually needed for IE6- only. Other browsers, including IE7, are ok with the div only, which is a much more lightweight solution. IFrames are resource expensive and slow on creation. We already have enough of them.

On IE6 instead, we create the blocker iframe on the fly. This iframe has no participation on the cover effect, which is still handled with the div, and acts as a pure blocker. It is completely transparent.

I think we must have the same approach in the dialog code, avoiding using iframes whenever possible for all covers, using then as pure blockers for IE6 only.

PS: I still hope IE6 will die someday :)

[Frederico Caldeira Knabben]

Replying to martinkou:

The blocker .htmls were implemented as HTML files for flexibility. Since they cover a large area I expected people might want to put other things into them other than a simple grayish or white color (e.g. borders?). They can be eliminated into purely JavaScript IFRAMES if needed (i.e. create iframe in JavaScript, document.open(), document.write(fixed_html_code), document.close()). It just depends on whether performance or feature is of higher priority.

Let's use CSS to give customization options instead. Be sure the covers have a well defined class that can be somehow used for customizations.

The last structure proposal actually changes the blocker/cover logic too.

The background blocker is going above the editor iframe because the editor iframe is static layout. I've already set the background blocker's z-index to 0 and placed it after the editor iframe but it's of no help. I've tried giving "position: relative" to the editor iframe but that caused serious problems when there are multiple editor instances (just try it on sample11.html, set both editor iframes to relative position via Firebug and open a dialog for each of them).

At this point, there is no sense having two covers. Let's use just the big one.

[Frederico Caldeira Knabben]

I've introduced several changes to the code with [1213]:

• Merged fck_dialog_floating.css in fck_dialog.css.
• Using the sprite image for the close button.
• Added hover effect in the close button.
• The cover for nested dialogs is also getting the color an opacity values from the config file.
• The "dialogId" is not needed anymore.
• Fixed the dialog centering algorithm for Firefox.
• The dialog is now RTL compatible.
• Using #ffffff for the main cover color. It makes it a bit lighter and cleaner. (still to check though).
• Code cleanup. Simplified some things. Removed a few unused functions.
• Unused images and files have been removed.
• Some images now have a generic name, as they could be used outside the dialog scope (sprites and loading).

Martin, please review it, to be sure that nothing has been broken and that the changes are ok (like a Review+ for the changeset only).

This was the first big/strict review step in the branch. The dialogs are fully working, but we are still far from having it ready for the trunk. There are several things to be changed in the code, and still a lot of clean up and simplification to be done.

Things that come to my mind right now:

• Remove all changes from fck.js. The code introduced there can certainly live on fckdialog.js, or even in fckdialog.html.

• Move most of the dialog logic from fckdialog.js to fckdialog.html. "Show", "Resize", "Close", "Enabled", "DnD" and "DialogStack" could probably be handled in the dialog itself.

• If dialog.html contains the code, the DialogStack thing could probably be avoided by using a simpler parent/child relationship.

• Some code commenting would be much appreciated.

To remember that, once we arrive to the final solution, we still need to change the office2003 and silver skins to the new system.

[Martin Kou]

I've tested the code changes and also the new features (e.g. dialog layout under RTL languages), and they seem to be working.

I've found some bugs with the throbber logic unrelated to the changes however, and they were fixed in [1214].

[Martin Kou]

The floating dialog code in [1214] has some very serious memory leaking bugs in IE6 (leaking away 10M+ for each dialog opened). Currently I'm trying to find out the leaking parts by disabling and commenting out parts of the dialog code, and so the code in the floating_dialog branch will be "broken" while I'm looking for the leaks.

I'm leaving a message here so that people won't be surprised when they find out the floating_dialog branch goes from "working quite well" to "completely broken" in a few changesets.

[Alfonso Martínez de Lizarrondo]

maybe you have already read it or done what is suggested, but this might be worth reading ​http://dojotoolkit.org/forum/general/general-discussion/using-dojo-iframes-solving-memory-leak

[Martin Kou]

I've fixed all floating dialog memory leak problems for IE6 and some other errors in r1265 or the floating dialog branch. Now you can literally open and close floating dialogs thousands of times in IE6 and IE6's memory usage would still stay the same.

For IE7 there's still a small memory leak problem, but it's only noticeable after you've opened and closed a floating dialog for hundreds of times.

The floating dialog code should be pretty robust now, but there're still some items left to work on:

1. The find and replace dialog does not work in IE, since it expects there's a selection in the editor frame, which does not exist now.
2. Opened dialogs should be left in memory to be reused in the future to improve performance and eliminate the remaining memory leak problem in IE7.

[Martin Kou]

The find and replace dialog issue has been fixed in [1269].

The dialog reused idea is scrapped after consulting with Fred yesterday, since the memory leak problem for IE has been eliminated (for IE6) or much reduced (for IE7) now.

[Martin Kou]

Retracted the review request since the other skins are still to be updated.

[Martin Kou]

I've updated the office2003 and silver skins to the compatible with the floating dialog system, as well as adding a test case for z-ordering between Flash animations and floating dialogs.

The floating dialog branch is ready for review now.

[Daniel Insley]

I've opened a ticket (#1726) for a problem I've noticed with the floating dialogs feature branch.

[Frederico Caldeira Knabben]

The current dragging system should use the event.screenX and event.screenY to the mouse move calculations. It would make the code much simpler, with performance improvements. It would also possibly make the Opera hack for the vibration useless.

Much probably #1726 would be fixed with it too.

[Martin Kou]

I've changed the dialog drag logic to use event.screenX/Y instead of event.clientX/Y in [1275].

[Alfonso Martínez de Lizarrondo]

In fckdialog.html the RefreshContainerSize function is duplicated

[Alfonso Martínez de Lizarrondo]

It seems that this patch also fixes #1671 For the moment I haven't found any problem, it seems to work fine.

[Frederico Caldeira Knabben]

Those are some of my findings so far:

• The color selector dialog is not working.

• RefreshContainerSize is duplicated in fckdialog.html.

• Use FCKTools.RegisterDollarFunction instead of "function $".

• Comments should respect our coding style

• The throbber should be a DOM effect solution, with no images to be downloaded.

• Review FCKTools.Hitch. It seems it is not needed.

• Review the Main object. It seems it is useless. We can group some functions in a private scope using (function(){})(), defining public functions with window.PublicFunction. All other function, which don't need a private scope, can be defined in the window scope directly.

• The "API export" block can be removed. Only the Size and Tabs entries need to be exported to the window scope, so we can do it manually. E.g. window.AddTab = Tabs.AddTab. We will probably have less and clearer code in this way.

• The branch should be aligned with trunk, to properly review it.

[Frederico Caldeira Knabben]

Replying to alfonsoml:

In fckdialog.html the RefreshContainerSize function is duplicated

Alfonso, feel free to Review- in those cases. We can Review?/Review- as many times as needed. It shouldn't hurt the sentiments of anyone :)

[Frederico Caldeira Knabben]

Martin, I've added a performance fix with [1320]. I would ask you to please check it, confirming that my changes make sense, and to ensure that I'm not breaking anything with it. Thanks!

[Martin Kou]

Replying to fredck:

Fixed. Ready to be reviewed again.

[Frederico Caldeira Knabben]

Martin, I've managed to review everything in the branch, but its core files fckdialog.js and fckdialog.html. So, here are the findings (very small things):

• The "test_results" files must be removed, or move its contents to the test1.html page, inside a comment block.

• The code is to be fixed/cleaned as soon as #1769 gets committed. So, if you give me a Review+ there, just apply it to the branch too, including the relative fixes.

• The fckdebug.html changes should be moved to a dedicated ticket over trunk. It has nothing to do with the dialogs.

• Please check [1358] and [1360].

I wanted to point them out now, so you can work on it while I'm reviewing those two files.

[Frederico Caldeira Knabben]

Ok, I've completed the review. I'm happy to say that we are almost there!

• Please check my last commits: [1366], [1367], [1369]! and [1370].

test1.html:

• In the third dialog open, a JS error has been thrown. Reproducible with IE7 and IE6, but it's not constant. Ignoring it, the test continues without problems.

• Sad to say that, but there is a constant memory leak in IE6 and IE7. I've checked it before applying any change to the code during the review.

dialog.html:

• Check if Editor() is really needed because it's... weird! E variable would make things clearer, and I doubt about its memory leak consequences as it should point to a pure JavaScript object.

Closing these things, added to the things in my previous comments would make it probably ready, finally. I'm anxious to see it moved to trunk!

[Martin Kou]

The new memory leak was caused by the FCK.Focus() line added in [1366]. Commenting it out in the current branch code eliminates any memory leak in test1.html in IE6.

There has always been a little bit of memory leak in IE7, however. I haven't been able to pinpoint the cause to that, maybe I work on that today.

But first, something must be causing IE6 memory leaks in FCK.Focus() right now, and it has to be fixed first.

[Martin Kou]

I've found the cause of the leak in IE6.

FCK.Focus() calls FCKEditingArea::Focus(). The line

this.Window.focus();

inside FCKEditingArea::Focus() is causing the memory leak.

To prove it, do the following experiment.

1. Clear the contents of FCKEditingArea::Focus().
2. Test for memory leaks in test1.html, observe the memory usage in Process Explorer XP (not taskmgr).
3. Memory usage remains constant during the test.
4. Put a "this.Window.focus();" in FCKEditingArea::Focus().
5. Test for memory leaks as in point 2.
6. Memory usage increases slowly during the test.

[Martin Kou]

It seems replacing Editor() with global variable E does not cause memory leaks. But note that it is entirely possible to leak memory in IE with a JavaScript object - as long as it points to a DOM node down its reference chain/tree.

For example, the following code leaks memory in IE6, and it can be checked in Drip:

var n1 = document.createElement('a');
var jso1 = {'a':n1};
var n2 = document.createElement('a');
var jso2 = {'a':n2};
n1.ref2=jso2;
n2.ref1=jso1;
// circular reference: n1 -> jso2 -> n2 -> jso1 -> n1

... and that is already a simple leak that happens without iframes. Memory leaks get even hairier when there are iframes, as in FCKeditor.

[Martin Kou]

I've tried commenting out all onfocus and onblur event handlers in fck.js and fckpanel.js I can find, but that didn't seem to help the this.Window.focus() memory leak.

[Frederico Caldeira Knabben]

Replying to martinkou:

The new memory leak was caused by the FCK.Focus() line added in [1366]. Commenting it out in the current branch code eliminates any memory leak in test1.html in IE6.

I have no changes when commenting that line!

[Frederico Caldeira Knabben]

Ok, [1384] has been introduced for #1769. It should be merged in the branch and the necessary fixes applied to the dialog code.

[Martin Kou]

Replying to fredck:

Replying to martinkou:

The new memory leak was caused by the FCK.Focus() line added in [1366]. Commenting it out in the current branch code eliminates any memory leak in test1.html in IE6.

I have no changes when commenting that line!

I've got a video of an experiment with commenting that line away, and it did eliminate memory leaks at my side.

​Experiment video (207MB)

[Martin Kou]

It's sad to say but it seems to Focus() leak cannot be solved in any way. All the other issues mentioned in Fred's earlier comments (e.g. JavaScript errors in test1.html, changing Editor() to E, removing test_results, merging changes from trunk to branch, undoing changes in fckdebug.html, etc.) have been taking care of.

[Frederico Caldeira Knabben]

I've reviewed all changes with two small changes: [1392] and [1393].

We are ready to go!

For the memory leak, I've made some research (after downloading Martin's crazy 200MB video), and I've confirmed that removing the Focus() call removes all memory leak with IE7. I've noted that that call is not needed at all for IE, so I've changed it with [1393].

For IE6, I still have leaks, and that line makes no changes. It may be that I'm having those results because I'm using MultipleIEs, not a native IE6 installation.

Anyway, I believe that the source of the leak is the same as the Focus() call. It means that the leak is not present in the dialog code, but in the editor. It is probably related to the fact that the editor receives the focus, which fires many events in the editor. So probably, the leak problem is not even present in the Focus() code. Probably only V3 will be able to fix it.

To conclude, let's move it to trunk!

Martin, I would ask you a lot of attention when merging it to trunk to be sure other commits applied to trunk are not being reverted. So, the branch and the trunk must be well aligned. Please check every single changed line before committing it to trunk, to be sure everything is related to the dialog needs.

[Martin Kou]

I've merged the floating dialog code to trunk and reviewed the changes in [1400].

[Frederico Caldeira Knabben]

Just to keep history of it, this feature has been introduced on trunk with [1398].

[Alexander Baumgartner]

Hello, i hava a solution for the DisplayMainCover-function in fckdialog.js without the need of trigger an resize or scroll-event.

a short description so far i remember (i use it already in an other context):

use: cover.style.width='100%'; use: cover.style.height='100%';

Browserdependend: 1) use for all modern browsers: 1.1) cover.style.position='fixed' (instead of 'absolute')

2) use only for IE5 and IE6: 2.1) cover.style.position='absolute' 2.2) set rootDoc.body.style.width=100%; 2.3) set rootDoc.body.style.height=100%; 2.4) set rootDoc.body.style.overflow='hidden'; 2.5) now you only need to paste your cover-div and cover-iframe over the actual "scrollpos"

3) Only for IE5 and IE6: On close you have to repair: rootDoc.body.style.width rootDoc.body.style.height rootDoc.body.style.overflow

it's only so far i remember (no guarantee of correctness)

i think you don't need the resizeHandler-function. if you are interested, i can search my script and send it to you

ps: you make very, very great work