Ticket #1659 (new Bug)

Opened 12 months ago

Last modified 2 months ago

Setting DocType to Standards Compliant Collapses <body> in IE

Reported by: Scott Owned by:
Priority: Normal Milestone: CKEditor 3.0
Component: General Version:
Keywords: Confirmed IE Review- HasPatch Cc:

Description

When you set FCKConfig.DocType to a standards compliant type, the min-height: 100% css trick on the body no longer works.

This means the body collapses to the size of the elements contained within the body.

IE only.

Attachments

1659.patch (1.2 KB) - added by fredck 11 months ago.
1659_1.patch (1.3 KB) - added by alfonsoml 11 months ago.
Proposed SVN patch
1659_2.patch (1.2 KB) - added by alfonsoml 11 months ago.
Revised patch

Change History

Changed 12 months ago by alfonsoml

  • keywords Confirmed IE added

I bet that there was a ticket filed about this previously and I did close it as WFM because I couldn't reproduce it with the current nightly (but it was buggy in the previous released version), but now I can't find it and for sure the behavior of IE is really unpleasant.

Steps to reproduce:

  1. set for example 
    FCKConfig.DocType = '<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">' ;
  2. to see more easily the weird behavior add to fck_editorarea.css this rule 
    body
{
	background-color: #AAA;
	padding: 5px 5px 5px 5px;
	margin: 0px;
	border:2px solid green;
}
  1. load sample01.html in IE.

Symptoms:

  1. the green border is just on the first line of content instead of covering all the text area.
  2. move the mouse up and down. it only change to a text cursor over that first line.

And more important to understand why it is severe:

  1. Make the editor initialize with empty contents (oFCKeditor.Value = '' ;).
  2. Now try to click in the editor area to start writing.

It only works if you click on the first line and it's really strange for any user.

Changed 11 months ago by Scott

I was messing around today on an IE 6 development machine today. But how about using IE proprietry expressions to fix this... something similar to this perhaps?

body {

height: expression(document.documentElement.clientHeight);

}

It does make a scrollbar always show, but perhaps subtracting something like the margins or padding off the client height could fix this?

I am not exactly sure why the scroll bars appear when using this code.

Changed 11 months ago by alfonsoml

css expressions are expensive with regards to CPU, so I would rather avoid them unless there is no other solution.

Changed 11 months ago by fredck

Changed 11 months ago by fredck

  • keywords HasPatch added
  • milestone set to FCKeditor 2.6

The attached patch should give similar results as Scott solution with expression. I've set the height to 90% because, as Scott described, the box model on strict mode will force the scrollbars to appear. With 90%, only a small part of the editing area will not be clickable, but the end user should not even note that in the every day use of the editor.

I've used some CSS hacks to make the selectors visible to IE6 and IE7 only.

I'm not yet asking it for review, as I would like to have your impressions first.

Changed 11 months ago by Scott

My impression is ... yuck! I think that setting it to 90% is just a temporary hack which doesnt fix the problem. We use a lot of drag and drop stuff with the editor and it becomes really evident when the bottom 10% is not a drop target.

I think it may be possible to fix the issue by specifying a negative margin and setting the height to 100%. I have done a little bit of testing with this, but have not managed to come up with a solution which will work with arbitrary paddings on the body.

I have however developed a solution using expressions (yes I know they are expensive). But they seem to be the only way to get it to work with totally arbitrary CSS. 

html
{
	height: expression(document.documentElement.clientHeight);
}

body
{
	height: expression(document.documentElement.clientHeight 
			- parseInt(this.currentStyle.paddingTop)
			- parseInt(this.currentStyle.paddingBottom));
}

Changed 11 months ago by fredck

Expressions! yuck! yuck! one for each entry (just kidding :)

Do you have the the same results by changing the "html" selector body to "height: 100%"? If I recall it well, the body is making the scrollbars appearing, you would have less one yuck!.

In the other hand, to avoid expressions, I think a feasible solution would be listening to the resize event, and set the height style at that point. In this way we avoid making the browser recalculate it thousands of times while using the editor (potentially crashing IE, btw).

Changed 11 months ago by alfonsoml

for me, it seems that the html rule isn't even necessary to set it to 100%. Just using 

body
{
	height: expression(document.documentElement.clientHeight 
			- parseInt(this.currentStyle.paddingTop)
			- parseInt(this.currentStyle.paddingBottom));
}

works fine, but I think that the best solution as pointed out by Fred is to listen to the onresize event and then manually adjust the height of the body because that way the code is executed only when needed. The problem with expressions is that apparently they are executed continuously, as soon as anything changes.

Changed 11 months ago by Scott

Ok sure. I am no fan of CSS expressions either.

Sorry - I thought that the html CSS rule was required. With html{ height: 100% }, it shows a scroll bar. With no html rule, it works fine.

The following code appears to work. Feel free to integrate it into the code-base wherever you feel it fits, changing the function names. 

function AnInitializationFunction()
{
	FCKTools.AddEventListenerEx( FCK.EditingArea.Window, 'resize', TheResizeWindowFunction );
	TheResizeWindowFunction();
},
	
function TheResizeWindowFunction()
{
	var oBody = FCK.EditingArea.Document.body;

	var iHeight = FCK.EditingArea.Document.documentElement.clientHeight 
		- parseInt( oBody.currentStyle.paddingTop )
		- parseInt( oBody.currentStyle.paddingBottom );

	oBody.style.height = iHeight + 'px';
}

Cheers guys - happy new year.

Changed 11 months ago by Scott

Note! Note! Note! I just realized...

Using the above function puts a style="height: xxx" on the body tag when full page editing!

So this probably needs to be taken out when converting to source.. and protection added if a body height pre-exists.

Changed 11 months ago by alfonsoml

Proposed SVN patch

Changed 11 months ago by alfonsoml

  • owner set to alfonsoml

I've attached a patch based mostly on the last code, but it does use runtimeStyle so there are no problems with serialization.

It runs only if the document is in strict mode. Now that I think about it, the check should be done only when setting the event listener. I'll upload a new patch.

Changed 11 months ago by alfonsoml

Revised patch

Changed 11 months ago by alfonsoml

  • keywords Review? added; HasPatch removed
  • status changed from new to assigned

The new patch avoids the continuous check of IsStrictMode

Changed 11 months ago by Scott

If the contents of the editor are larger than the body height, the body element doesnt extend past the original window height. To replicate, just add a lot of empty paragraphs until a scroll bar appears. Then insert something like a table. Use the IE DOM inspector and inspect the body element. Move your mouse to the right of the table, and you will see the cursor becomes a pointer instead of a text caret.

If you click the editor when the cursor is a pointer, it scrolls the editor all the way to the top which is very frustrating.

Perhaps we need to look at scrollHeight when calculating this. The problem then is if we start using scrollHeight, then this function needs to be run every time the content changes height - which I assume is very difficult to detect. Otherwise, the height of body will be stuck at the height when the content was first loaded. 

var oBody = FCK.EditingArea.Document.body;
		
// Remove the height so that scrollHeight calculates correctly
oBody.runtimeStyle.height = '';

var iHeight = FCK.EditingArea.Document.documentElement.clientHeight 
	- parseInt( oBody.currentStyle.paddingTop )
	- parseInt( oBody.currentStyle.paddingBottom )
	;

if( iHeight > oBody.scrollHeight )
{
	oBody.runtimeStyle.height = iHeight + 'px';
}

Changed 11 months ago by Scott

It is a pity we can't make the html element editable. It may have fixed the problem. Sadly, it doesn't work. 

oDoc.documentElement.contentEditable = true ;

Changed 11 months ago by Scott

Ok I think I have a solution. You may or may not like it. It is a hack, but I have now done a fair bit of research on this.

What if we add the following CSS: 

body
{
	position: absolute;
	bottom: 0px;
	right: 0px;
	overflow-y: scroll;
}

Then remove the overflow statement at line ~190 of fck_1.js.

Changed 11 months ago by Scott

I have been testing this method a fair bit now and it appears to be working ok.

The only problem is when the body has a margin. It pushes the scroll in from from the sides of the editor.

Can I suggest using this block of CSS, (perhaps with overflow: auto), and also running a bit of javascript which sets the runtime style margin to 0px, and adds the old margin to the runtime style padding?

Changed 11 months ago by Scott

After days of working with this... I have a solution :D... check it out...

If you havent been reading my previous messages, it basically works like this:

1) Absolute position the body element. Then set the top, left, bottom, right to 0px. This is wierd but it works. This allows us to make the body have 100% height without it messing up when the height of the content is greater than that of the window.

2) The scroll bar is no longer on the document element. The scroll bar now appears on the body element. This is done by setting overflow to auto. We have to do this as a result of #1

3) Because we are now scrolling from the body, any margins move the scroll bars away from the sides of the editor. So we add any margins to the padding. This does not matter as on the body element, padding and margin are virtually the same as the body element doesnt have any parent nodes

4) Any CSS styles that set the height of the body must be muted out because they cause dramas with the bottom CSS attribute.

Could you guys please integrate this into fck at a location of your choice and test. 

	Initialize: function()
	{
		// These fixes are only for IE under strict mode
		if ( !FCKBrowserInfo.IsIE || FCKRegexLib.Html4DocType.test( FCKConfig.DocType ) )
		{
			return;
		}
	
		var oBody = FCK.EditingArea.Document.body;

		// Set the base CSS
		var arrStyles = {'position': 'absolute', 'top': '0px', 'right': '0px', 'bottom': '0px', 'left': '0px'};
		
		for(var strStyle in arrStyles)
		{
			oBody.runtimeStyle[strStyle] = arrStyles[strStyle];
		}
		
		var arrSides = ['Top', 'Right', 'Bottom', 'Left'];
		
		// Convert all margin to padding so the scroll bars appear at the right position
		for(var i = 0; i < arrSides.length; i++)
		{
			var strSide = arrSides[i];

			oBody.runtimeStyle['padding' + strSide] = parseInt( oBody.currentStyle['padding' + strSide] )
				+ parseInt( oBody.currentStyle['margin' + strSide] )
				+ 'px';
			
			oBody.runtimeStyle['margin' + strSide] = '0px';
		}
		
		// The height must be auto or it messes up the absolute bottom: 0px css attribute
		oBody.runtimeStyle.height = 'auto';
		oBody.runtimeStyle.minHeight = 'auto';
	}

Changed 11 months ago by Scott

PS: I moved all the body CSS Styles here to be in JS so because they are only for IE.

Changed 11 months ago by Scott

Sorry guys, I had some cached CSS and forgot to add the overflow style to the JS. I wish you could edit these posts.

Updated function: 

	Initialize: function()
	{
		// These fixes are only for IE under strict mode
		if ( !FCKBrowserInfo.IsIE || FCKRegexLib.Html4DocType.test( FCKConfig.DocType ) )
		{
			return;
		}
	
		var oBody = FCK.EditingArea.Document.body;

		// Set the base CSS
		var arrStyles = {'position': 'absolute', 'top': '0px', 'right': '0px', 'bottom': '0px', 'left': '0px', 'overflow': 'auto'};
		
		for(var strStyle in arrStyles)
		{
			oBody.runtimeStyle[strStyle] = arrStyles[strStyle];
		}
		
		var arrSides = ['Top', 'Right', 'Bottom', 'Left'];
		
		// Convert all margin to padding so the scroll bars appear at the right position
		for(var i = 0; i < arrSides.length; i++)
		{
			var strSide = arrSides[i];

			oBody.runtimeStyle['padding' + strSide] = parseInt( oBody.currentStyle['padding' + strSide] )
				+ parseInt( oBody.currentStyle['margin' + strSide] )
				+ 'px';
			
			oBody.runtimeStyle['margin' + strSide] = '0px';
		}
		
		// The height must be auto or it messes up the absolute bottom: 0px css attribute
		oBody.runtimeStyle.height = 'auto';
		oBody.runtimeStyle.minHeight = 'auto';
	}

Also, you still need to take out that overflow line in line ~190 of fck_1.js

Changed 11 months ago by Scott

Update: The absolute positioning is no longer necessary. And causes flicker when hovering over the toolbar.

This fixes it. 

	Initialize: function()
	{
		// These fixes are only for IE under strict mode
		if ( !FCKBrowserInfo.IsIE || FCKRegexLib.Html4DocType.test( FCKConfig.DocType ) )
		{
			return;
		}
	
		var oBody = FCK.EditingArea.Document.body;

		// Set the base CSS
		var arrStyles = {'height': '100%', 'overflow': 'auto'};
		
		for(var strStyle in arrStyles)
		{
			oBody.runtimeStyle[strStyle] = arrStyles[strStyle];
		}
		
		var arrSides = ['Top', 'Right', 'Bottom', 'Left'];
		
		// Convert all margin to padding so the scroll bars appear at the right position
		for(var i = 0; i < arrSides.length; i++)
		{
			var strSide = arrSides[i];

			oBody.runtimeStyle['padding' + strSide] = parseInt( oBody.currentStyle['padding' + strSide] )
				+ parseInt( oBody.currentStyle['margin' + strSide] )
				+ 'px';
			
			oBody.runtimeStyle['margin' + strSide] = '0px';
		}
	}

Changed 11 months ago by Scott

*** FINAL - Ignore all previous posts ***

Argh. Sorry - caching fooled me again. I am using a custom script to bundle and compress plugins & css using YUI compressor and it makes development on the editor complicated - I need to fix that. I have been developing this from plugin-space.

Ok this should be the last post I make on this. The height of the body was not expanding after rebuilding and clearing the cache. So merged with the original height calculation function, it appears to work.

- So the body has a fixed height to be the same as the containing window. It is calculated on initialization and resize.

- The body has overflow: auto, so that it scrolls whenever the bodys size exceeds that of the fixed height. The scroll bar is effectively moved from being on the documentElement to being a part of the body.

- Because we are now scrolling from the body, any margins move the scroll bars away from the sides of the editor. So we add any margins to the padding. This does not matter as on the body element, padding and margin are virtually the same as the body element doesnt have any parent nodes

- If you see double scroll bars, the overflow line in fck_1.js needs to be removed around line 190. 

function FCK.InitializeIEStrict()
{
	var oBody = FCK.EditingArea.Document.body;

	// Set the base CSS
	oBody.runtimeStyle.overflow = 'auto';
		
	var arrSides = ['Top', 'Right', 'Bottom', 'Left'];
		
	// Convert all margin to padding so the scroll bars appear at the right position
	for(var i = 0; i < arrSides.length; i++)
	{
		var strSide = arrSides[i];

		oBody.runtimeStyle['padding' + strSide] = parseInt( oBody.currentStyle['padding' + strSide] )
			+ parseInt( oBody.currentStyle['margin' + strSide] )
			+ 'px';
			
		oBody.runtimeStyle['margin' + strSide] = '0px';
	}
		
	FCKTools.AddEventListenerEx( FCK.EditingArea.Window, 'resize', Doc_OnResize );
	Doc_OnResize();
},
	
function Doc_OnResize() 
{
	var oBody = FCK.EditingArea.Document.body;

	var iHeight = FCK.EditingArea.Document.documentElement.clientHeight 
		- parseInt( oBody.currentStyle.paddingTop )
		- parseInt( oBody.currentStyle.paddingBottom )
		;
	
	oBody.style.height = iHeight + 'px';
}

FCK.InitializeBehaviors = function( dontReturn )  
{
	...

	// Catch cursor selection changes.  
	this.EditorDocument.attachEvent("onselectionchange", Doc_OnSelectionChange ) ;  

	// #1659 The body in standards mode doesn't get full height.  
	// These fixes are only for IE under strict mode
	if ( !FCKBrowserInfo.IsIE || FCKTools.IsStrictMode( FCK.EditorDocument ) ) 
	{  
		FCK.InitializeIEStrict() ;  
	}
}

Enjoy :D

PS: This may break the auto-grow plugin as it broke mine (my version is totally different to the fck one though).

Changed 10 months ago by fredck

  • keywords Review- HasPatch added; Review? removed

Based on Scott's comments, I'm assuming Alfonso's patch is not the definitive solution. Actually, I've just tested it. I was not able to reproduce the table problem as Scott described, but I was able to check that the cursor changes to arrow between paragraphs placed in the scrolled bottom of the document, and clicking there makes the editor scroll to the top, just like Scott said. It is an unpleasant thing, really.

Scott proposed a dirty hack to fix it. It could work, but I would be really happier if we could solve this issue with some kind of CSS magic. Also, the "break the auto-grow plugin" means more work should be done on this.

Changed 10 months ago by Scott

I have used my dirty hack for a little while now. It is not a definitive solution either. Because we are scrolling on the body tag, and not the window, all absolute positioned elements are stuffed up.

I have another fix which uses min-height on the body. But this is no good either, as IE 6 does not support this without expressions.

Changed 9 months ago by fredck

  • milestone changed from FCKeditor 2.6 to FCKeditor 2.6.1

Changed 8 months ago by martinkou

It is currently not possible to use standards compliant mode by setting DocType in any browser due to #2102.

Changed 5 months ago by alfonsoml

  • owner alfonsoml deleted
  • status changed from assigned to new
  • milestone changed from FCKeditor 2.6.3 to CKEditor 3.0

I don't have any solution for the next release, so moving to a future milestone and let's hope that someone is able to find the magic fix.

Changed 5 months ago by fredck

I would be happy if we could have a IE7+ solution for it at least. Let's investigate it for V3.

Changed 2 months ago by arczi

#960 has been marked as a DUP

Note: See TracTickets for help on using tickets.