#1659 closed Bug (fixed)
Setting DocType to Standards Compliant Collapses <body> in IE
| Reported by: | Scott McNaught | Owned by: | Garry Yao |
|---|---|---|---|
| Priority: | Normal | Milestone: | CKEditor 3.1 |
| Component: | General | Version: | |
| Keywords: | Confirmed Oracle IE Review+ | Cc: | Senthil |
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 (10)
Change History (66)
comment:1 Changed 16 years ago by
| Keywords: | Confirmed IE added |
|---|
comment:2 Changed 16 years ago by
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.
comment:3 Changed 16 years ago by
css expressions are expensive with regards to CPU, so I would rather avoid them unless there is no other solution.
Changed 16 years ago by
| Attachment: | 1659.patch added |
|---|
comment:4 Changed 16 years ago by
| Keywords: | HasPatch added |
|---|---|
| Milestone: | → 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.
comment:5 Changed 16 years ago by
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));
}
comment:6 Changed 16 years ago by
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).
comment:7 Changed 16 years ago by
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.
comment:8 Changed 16 years ago by
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.
comment:9 Changed 16 years ago by
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.
comment:10 Changed 16 years ago by
| Owner: | set to Alfonso Martínez de Lizarrondo |
|---|
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.
comment:11 Changed 16 years ago by
| Keywords: | Review? added; HasPatch removed |
|---|---|
| Status: | new → assigned |
The new patch avoids the continuous check of IsStrictMode
comment:12 Changed 16 years ago by
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';
}
comment:13 Changed 16 years ago by
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 ;
comment:14 Changed 16 years ago by
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.
comment:15 Changed 16 years ago by
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?
comment:16 Changed 16 years ago by
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';
}
comment:17 Changed 16 years ago by
PS: I moved all the body CSS Styles here to be in JS so because they are only for IE.
comment:18 Changed 16 years ago by
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
comment:19 Changed 16 years ago by
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';
}
}
comment:20 Changed 16 years ago by
* 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).
comment:21 Changed 16 years ago by
| 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.
comment:22 Changed 16 years ago by
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.
comment:23 Changed 16 years ago by
| Milestone: | FCKeditor 2.6 → FCKeditor 2.6.1 |
|---|
comment:24 Changed 16 years ago by
It is currently not possible to use standards compliant mode by setting DocType in any browser due to #2102.
comment:25 Changed 16 years ago by
| Milestone: | FCKeditor 2.6.3 → CKEditor 3.0 |
|---|---|
| Owner: | Alfonso Martínez de Lizarrondo deleted |
| Status: | assigned → new |
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.
comment:26 Changed 16 years ago by
I would be happy if we could have a IE7+ solution for it at least. Let's investigate it for V3.
comment:28 Changed 15 years ago by
| Owner: | set to Martin Kou |
|---|---|
| Status: | new → assigned |
comment:29 Changed 15 years ago by
| Keywords: | Review? added; Review- removed |
|---|
I've found a very simple way to fix in in CKEditor v3 - it doesn't need resize handlers.
Changed 15 years ago by
| Attachment: | 1659_3.patch added |
|---|
comment:30 Changed 15 years ago by
Hmm - Are you sure setting height: 100% works?
From my memory... Try adding a whole lot of <p> tags to the page, then when there is enough content to scroll, click in the white area to the right where the cursor is a pointer, not an editing caret. The editor will likely scroll to the top.
Note: I have not tested your solution in v3.
comment:31 Changed 15 years ago by
| Keywords: | Review- added; Review? removed |
|---|
Just did some tests... it works in IE6, but doesn't work in IE7.
Changed 15 years ago by
| Attachment: | 1659_4.patch added |
|---|
comment:32 Changed 15 years ago by
| Keywords: | Review? added; Review- removed |
|---|
minHeight in body seems to work well in IE7.
comment:33 Changed 15 years ago by
| Keywords: | Review- added; Review? removed |
|---|
- There should be a way to have that "22" pixel difference calculated from the document styles.
- The hardcoded styles at the end of the patch could go inside contents.css, making it easier to customize.
comment:34 Changed 15 years ago by
| Keywords: | Review? added; Review- removed |
|---|
I've changed the CSS hack to be specific to IE6, since with the new JS logic IE7 doesn't need the CSS hack anymore.
I've also added support for IE8.
Changed 15 years ago by
| Attachment: | 1659_5.patch added |
|---|
comment:35 Changed 15 years ago by
| Keywords: | Review- added; Review? removed |
|---|
Oops, retracted, the patch in content.css is affecting the style drop down combos.
Changed 15 years ago by
| Attachment: | 1659_6.patch added |
|---|
comment:36 Changed 15 years ago by
| Keywords: | Review? added; Review- removed |
|---|
So editor.addCss() is still the simplest way to do it - adding the hack to content.css would break combo boxes in IE6.
comment:37 Changed 15 years ago by
| Milestone: | CKEditor 3.0 → CKEditor 3.1 |
|---|
comment:38 Changed 15 years ago by
| Cc: | Senthil added |
|---|---|
| Keywords: | Oracle added |
comment:40 Changed 15 years ago by
Replying to fredck:
#1659 has been marked as DUP.
Actually #3832 is a DUP.
I'm attaching patch from that ticket for consideration as it quite resolves this issue using CSS only. It's probably possible to have it 100% working using only css if we could introduce wrapper element inside iframe's body, which could stretch body and handle margin/padding from content.
Changed 15 years ago by
| Attachment: | 1659_7_ref.patch added |
|---|
comment:42 Changed 15 years ago by
Just wanted to make sure people are aware, 1659_7_ref.patch breaks styles, format, and size dropdown floats in IE6. (It causes the scrollbars to disappear.)
comment:43 Changed 15 years ago by
Here is the fixed 1659_7 patch (works fine in IE6 and IE7). Sorry its not in .patch format; I dont have SVN on this PC to make one with.
/* #3832 IE8 Can't always click in the editor and give focus */
body, html
{
height: 95%;
/* IE6 */
_height: 85%;
}
body.cke_panel_frame
{
height: auto;
/* IE6 */
_height: auto;
}
/* END #3832 */
Changed 14 years ago by
| Attachment: | 1659_8.patch added |
|---|
comment:45 Changed 14 years ago by
I think we're heading into a wrong direction on this problem, the thing to fix is not making <body> fit the viewport (which is wrong for various evil results we already seen above), but to fix the IE missing behavior of "focus the editable body content when click on document".
So here I it make sense to propose here let's recall our original purpose on this ticket and I'm attaching a patch targeting exactly this issue.
Ticket Test added at :
http://ckeditor.t/tt/1659/1.html.
comment:46 Changed 14 years ago by
| Keywords: | Review- added; Review? removed |
|---|
The idea of the patch is good, and it should be enough for our needs here. Some review though:
- Check if it's possible to make the code simpler, without having to do all the offset checks, but still taking into consideration the scrollbars issue.
- The current behavior is a bit strange, as it shows the browser's focus box around the editor for a fraction of second when clicking. There should be another way to set the focus, or even a way to avoid the focus box.
- The caret disappears at every second click in the bottom empty area.
comment:47 Changed 14 years ago by
| Owner: | changed from Martin Kou to Garry Yao |
|---|---|
| Status: | assigned → new |
Changed 14 years ago by
| Attachment: | 1659_9.patch added |
|---|
comment:48 Changed 14 years ago by
| Keywords: | Review? added; Review- HasPatch removed |
|---|---|
| Status: | new → assigned |
New patch targeting the above mentioned issues.
comment:50 Changed 14 years ago by
| Keywords: | Review+ added; Review? removed |
|---|
Wow... this solution is pretty good. Congrats!
comment:52 Changed 14 years ago by
I might be wrong here because I haven't tested the patches above...
I can see this fixing the click focus problem. But the cursor is going to be pointer rather than a text caret. This one of the reasons why in my earlier attempts I was trying to extend the viewport / body element.
Is this patch going to be used in conjunction with some CSS to restore the text caret cursor?
comment:53 Changed 14 years ago by
Just another note... You can set documentElement.style.cursor = 'text', and it gives you a nice text cursor for the area below the body.
But you still get cursor: default between paragraphs or to the right of paragraphs that dont extend the full length of the line.
Also, as you are all probably aware, setting cursor styles has no effect on elements nested underneath a contentEditable element.
comment:54 Changed 14 years ago by
Scott, we'll be releasing the 3.0.2 and 3.1 soon, which will contain this fix. You can then test it and check if it impacts on usability. It works well for me, and I doubt users will feel any issue because of the cursor.
In any case, if you think it's worth, feel free to opening a new dedicated ticket after you confirm it in the release. Thanks!
comment:55 Changed 13 years ago by
| Keywords: | NeedsTest 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:
body { background-color: #AAA; padding: 5px 5px 5px 5px; margin: 0px; border:2px solid green; }Symptoms:
And more important to understand why it is severe:
It only works if you click on the first line and it's really strange for any user.