Bug 86794 - [test case] document.body.clientHeight does not give the height of the full frame
Summary: [test case] document.body.clientHeight does not give the height of the full f...
Status: RESOLVED WORKSFORME
Alias: None
Product: konqueror
Classification: Applications
Component: khtml ecma (show other bugs)
Version: unspecified
Platform: RedHat Enterprise Linux Linux
: NOR normal
Target Milestone: ---
Assignee: Konqueror Developers
URL:
Keywords:
: 63815 (view as bug list)
Depends on:
Blocks:
 
Reported: 2004-08-08 14:38 UTC by David Monniaux
Modified: 2008-11-23 15:52 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
test case showing discrepancies between window.innerHeight and document.body.clientHeight (517 bytes, text/html)
2004-08-08 15:41 UTC, David Monniaux
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Monniaux 2004-08-08 14:38:52 UTC
Version:            (using KDE KDE 3.2.2)
Installed from:    RedHat RPMs
OS:                Linux

document.body.clientHeight does not contain, as expected, the height of the frame in which the document is displayed, but the height of the actually displayed text.
Comment 1 David Monniaux 2004-08-08 15:16:34 UTC
Bug not present in 3.1
Comment 2 David Monniaux 2004-08-08 15:32:46 UTC
Demonstrated by:
The output value is about 499 (inner window height) on 3.1, and 60 (height of text) on 3.2:

<html>
<head><title>Essais</title></head>
<body>
<p>oprofile is a profiling system for x86 systems running Linux 2.2/2.4.
Profiling runs transparently during the background, and profile data
can be collected at any time. oprofile makes use of the hardware performance
counters provided on Intel P6, and AMD Athlon family processors, and can use
the RTC for profiling on othe x86 processor types.</p>
<script type="text/javascript">alert(window.innerHeight);</script>
</body></html>
Comment 3 David Monniaux 2004-08-08 15:39:06 UTC
When not using frames, document.body.clientHeight and window.innerHeight should be the same (which happens in 3.1). In 3.2 and 3.3 for this short page, document.body.clientHeight < window.innerHeight.
Comment 4 David Monniaux 2004-08-08 15:41:56 UTC
Created attachment 7036 [details]
test case showing discrepancies between window.innerHeight and document.body.clientHeight
Comment 5 Stephan Kulow 2004-08-08 16:31:14 UTC
http://bugzilla.mozilla.org/show_bug.cgi?id=180552 contains quite some details about this property
Comment 6 Stephan Kulow 2004-08-08 17:03:51 UTC
We need a special case for body's clientHeight too. But a horizontal scrollbar would need to substract from the value I think

--- rendering/render_object.cpp 5 Aug 2004 00:32:33 -0000       1.267
+++ rendering/render_object.cpp 8 Aug 2004 15:02:27 -0000
@@ -429,7 +429,10 @@ short RenderObject::clientWidth() const

 int RenderObject::clientHeight() const
 {
-    return height() - borderTop() - borderBottom() -
+    int h = height();
+    if (isBody())
+       h = canvas()->view()->height();
+    return h - borderTop() - borderBottom() -
       (layer() ? layer()->horizontalScrollbarHeight() : 0);
 }
Comment 7 David Monniaux 2004-08-08 17:12:02 UTC
When using frames, things get even more broken:

http://quatramaran.ens.fr/~monniaux/essais/frames.html

On Konqueror 3.2, it gives window.innerHeight=26!
Comment 8 Maksim Orlovich 2006-01-25 18:55:33 UTC
*** Bug 63815 has been marked as a duplicate of this bug. ***
Comment 9 Maksim Orlovich 2006-07-12 04:31:11 UTC
SVN commit 561392 by orlovich:

Improve the compatibility of our scrollTop/Left/Height/Width,
offsetWidth/offsetHeight,clientWidth/clientHeight on body and the root
element. These aren't quite perfectly compatible, but should be 
clearly better...

This fixes core of #86794, #117163, and #130577 (amazon.com book reader 
 --- though error reporting popups seem to mess that
up a bit(!?))

CCBUG:86794
BUG:117163
BUG:130577



 M  +46 -0     kjs_dom.cpp  
 M  +0 -33     kjs_html.cpp  
 M  +1 -2      kjs_html.h  


--- branches/KDE/3.5/kdelibs/khtml/ecma/kjs_dom.cpp #561391:561392
@@ -182,6 +182,46 @@
   return DOMObjectLookupGetValue<DOMNode, DOMObject>(exec, propertyName, &DOMNodeTable, this);
 }
 
+static khtml::RenderObject* handleBodyRootQuirk(const DOM::Node& node, khtml::RenderObject* rend, int token)
+{
+  //This emulates the quirks of various height/width properties on the viewport and root. Note that it 
+  //is (mostly) IE-compatible in quirks, and mozilla-compatible in strict.
+  if (!rend) return 0;
+
+  bool quirksMode = rend->style() && rend->style()->htmlHacks();
+  
+  //There are a couple quirks here. One is that in quirks mode body is always forwarded to root...
+  //This is relevant for even the scrollTop/scrollLeft type properties.
+  if (quirksMode && node.handle()->id() == ID_BODY) {
+    while (rend->parent() && !rend->isRoot())
+      rend = rend->parent();
+  }
+
+  //Also, some properties of the root are really done in terms of the viewport.
+  //These are  {offset/client}{Height/Width}. The offset versions do it only in 
+  //quirks mode, the client always.
+  if (!rend->isRoot()) return rend; //Don't care about non-root things here!
+  bool needViewport = false;
+
+  switch (token) {
+    case DOMNode::OffsetHeight:
+    case DOMNode::OffsetWidth:
+      needViewport = quirksMode;
+      break;
+    case DOMNode::ClientHeight:
+    case DOMNode::ClientWidth:
+      needViewport = true;
+      break;
+  }
+  
+  if (needViewport) {
+    //Scan up to find the new target
+    while (rend->parent())
+      rend = rend->parent();
+  }
+  return rend;
+}
+
 Value DOMNode::getValueProperty(ExecState *exec, int token) const
 {
   switch (token) {
@@ -287,6 +327,9 @@
 
     khtml::RenderObject *rend = node.handle()->renderer();
 
+    //In quirks mode, may need to forward if to body.
+    rend = handleBodyRootQuirk(node, rend, token);
+
     switch (token) {
     case OffsetLeft:
       return rend ? static_cast<Value>( Number( rend->offsetLeft() ) ) : Undefined();
@@ -427,6 +470,9 @@
 
     khtml::RenderObject *rend = node.handle() ? node.handle()->renderer() : 0L;
 
+    //In quirks mode, may need to forward.
+    rend = handleBodyRootQuirk(node, rend, token);
+
     switch (token) {
       case ScrollLeft:
         if (rend && rend->layer()) {
--- branches/KDE/3.5/kdelibs/khtml/ecma/kjs_html.cpp #561391:561392
@@ -676,10 +676,6 @@
   text		KJS::HTMLElement::BodyText	DontDelete
   vLink		KJS::HTMLElement::BodyVLink	DontDelete
 # IE extension
-  scrollLeft	KJS::HTMLElement::BodyScrollLeft DontDelete
-  scrollTop	KJS::HTMLElement::BodyScrollTop	 DontDelete
-  scrollWidth   KJS::HTMLElement::BodyScrollWidth DontDelete|ReadOnly
-  scrollHeight  KJS::HTMLElement::BodyScrollHeight DontDelete|ReadOnly
   onload        KJS::HTMLElement::BodyOnLoad     DontDelete
 @end
 @begin HTMLFormElementTable 11
@@ -1257,20 +1253,6 @@
         Value nodeValue(kjsDocNode);
         return kjsDocNode->getListener( DOM::EventImpl::LOAD_EVENT );
     }
-    default:
-      // Update the document's layout before we compute these attributes.
-      DOM::DocumentImpl* docimpl = node.handle()->getDocument();
-      if (docimpl)
-        docimpl->updateLayout();
-
-      switch( token ) {
-      case BodyScrollLeft:
-        return Number(body.ownerDocument().view() ? body.ownerDocument().view()->contentsX() : 0);
-      case BodyScrollTop:
-        return Number(body.ownerDocument().view() ? body.ownerDocument().view()->contentsY() : 0);
-      case BodyScrollHeight:   return Number(body.ownerDocument().view() ? body.ownerDocument().view()->contentsHeight() : 0);
-      case BodyScrollWidth:    return Number(body.ownerDocument().view() ? body.ownerDocument().view()->contentsWidth() : 0);
-      }
     }
   }
   break;
@@ -2487,21 +2469,6 @@
       case BodyLink:            { body.setLink(str); return; }
       case BodyText:            { body.setText(str); return; }
       case BodyVLink:           { body.setVLink(str); return; }
-      case BodyScrollLeft:
-      case BodyScrollTop: {
-        QScrollView* sview = body.ownerDocument().view();
-        if (sview) {
-          // Update the document's layout before we compute these attributes.
-          DOM::DocumentImpl* docimpl = body.handle()->getDocument();
-          if (docimpl)
-            docimpl->updateLayout();
-          if (token == BodyScrollLeft)
-            sview->setContentsPos(value.toInteger(exec), sview->contentsY());
-          else
-            sview->setContentsPos(sview->contentsX(), value.toInteger(exec));
-          }
-        return;
-      }
       case BodyOnLoad:
         DOM::DocumentImpl *doc = static_cast<DOM::DocumentImpl *>(node.ownerDocument().handle());
         if (doc && checkNodeSecurity(exec, node))
--- branches/KDE/3.5/kdelibs/khtml/ecma/kjs_html.h #561391:561392
@@ -84,8 +84,7 @@
            LinkSheet, TitleText, MetaName, MetaHttpEquiv, MetaContent, MetaScheme,
            BaseHref, BaseTarget, IsIndexForm, IsIndexPrompt, StyleDisabled,
            StyleSheet, StyleType, StyleMedia, BodyBackground, BodyVLink, BodyText,
-           BodyLink, BodyALink, BodyBgColor,  BodyScrollLeft, BodyScrollTop,
-           BodyScrollHeight, BodyScrollWidth, BodyOnLoad,
+           BodyLink, BodyALink, BodyBgColor,  BodyOnLoad,
            FormAction, FormEncType, FormElements, FormLength, FormAcceptCharset,
            FormReset, FormTarget, FormName, FormMethod, FormSubmit, SelectAdd,
            SelectTabIndex, SelectValue, SelectSelectedIndex, SelectLength,
Comment 10 Michael Leupold 2008-04-06 16:09:56 UTC
Primary testcase works for me on trunk r794020. Second testcase (c7) using frames is not reachable but is possibly a duplicate of BUG:57995
Comment 11 Michael Leupold 2008-04-13 23:31:54 UTC
Working on 3.5.9 as well.
Comment 12 Médéric Boquien 2008-11-23 15:52:50 UTC
Add the new email of the reporter to the CC list, he cannot access the original one anymore.