Bug 117163 - Javascript popups at www.ensembl.org show up at wrong positions
Summary: Javascript popups at www.ensembl.org show up at wrong positions
Status: RESOLVED FIXED
Alias: None
Product: konqueror
Classification: Applications
Component: khtml ecma (show other bugs)
Version: 3.5
Platform: Ubuntu Linux
: NOR normal
Target Milestone: ---
Assignee: Konqueror Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-11-27 21:21 UTC by Harm van Bakel
Modified: 2006-07-12 04:31 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
patch (4.55 KB, patch)
2006-03-11 03:42 UTC, Maksim Orlovich
Details
patch #2 (5.75 KB, patch)
2006-03-11 04:27 UTC, Maksim Orlovich
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Harm van Bakel 2005-11-27 21:21:18 UTC
Version:           3.5 (using KDE KDE 3.5.0)
Installed from:    Ubuntu Packages
Compiler:          gcc 4.0.2 
OS:                Linux

The Ensembl genome browser is one of the central repositories for genome assemblies and annotations, e.g. the human genome. When clicking on annotated genes, a javascript popup should appear right next to the pointer with contextual information, however it turns up completely out of position somewhere in the top of the page (for example, see http://www.ensembl.org/Homo_sapiens/contigview?l=2:153017491-153331854 ; click on any track in the 'detailed view'). This problem appeared in konqueror v3.5.
Comment 1 Maksim Orlovich 2006-03-10 19:41:41 UTC
Hmm, the reason this happens is that scrollTop is always(?) 0 in gecko in strict mode. Gonna need to test IE, though. 
Comment 2 Germain Garand 2006-03-11 00:11:59 UTC
quirksmode.org as a quite thorough analysis on those viewport properties along with nice test pages strict/loose. Very scary. It seems we'd need to turn some on/off and from a place to another according to doctype to keep scripts such as this one happy ;(
Comment 3 Maksim Orlovich 2006-03-11 03:31:24 UTC
I have a reasonably simple patch for this, which is imperfect, though. Will attach, along with summary of my testing. To make things worse, IE and Mozilla are inconsistent here. 
Comment 4 Maksim Orlovich 2006-03-11 03:42:23 UTC
Created attachment 15043 [details]
patch

Basically, this removes special body path for scroll*, and instead makes the
generic path in quirks mode forward body to root.
Comment 5 Maksim Orlovich 2006-03-11 03:44:40 UTC
Ugh. And now I notice that somehow scrollTop not longer works as I wished it to do in quirks. Double-checking
Comment 6 Maksim Orlovich 2006-03-11 04:27:56 UTC
Created attachment 15044 [details]
patch #2

A bit better, but still sucks. This also fixes #86794, by the way.
Comment 7 Maksim Orlovich 2006-03-11 04:30:02 UTC
Here are my test results, with patch:
quirks mode:
scrollTop:
IE:        position on body, 0 on documentElement
Mozilla:   position on body, 0 on documentElement
Konqueror: position on both

scrollHeight:
IE:        document height on body, viewport height on documentElement!
Mozilla:   document height on both
Konqueror: document height on both

clientHeight:
IE:        viewport height (minus something) on body, 0 on documentElement!
Mozilla:   viewport height on body,                   0 on documentElement!
Konqueror: viewport height on body,                   document height on documentElement

offsetHeight:
Mozilla:   height of element on both!
IE:        viewport height on both!
Konqueror: viewport height on body,                   document height on documentElement

strict mode:
scrollTop:
IE:       0 on body, position on documentElement
Mozilla:  0 on body, position on documentElement
Konqueror: 0 on body, position on documentElement

scrollHeight:
IE, Mozilla, Konq: scrollHeight of the appropriate element

clientHeight: 
IE, Mozilla: body: height of the body element
             documentElement: viewport height!!!

Konq: height of the element on both! (BUG?)

offsetHeight:
IE:  on body:          height of the body element
     documentElement:  viewport height!
Mozilla: height of the appropriate element
Konq:    height of the appropriate element
Comment 8 Maksim Orlovich 2006-03-11 04:38:01 UTC
hmm, it may be cleanest to make offsetHeight and clientHeight on root to return viewport height (just like scrollHeight does in quirks mode), and to always forward body to root in quirks. This will make us match IE, modulo not returning 0 for some things in quirks.
Comment 9 Germain Garand 2006-03-11 21:52:43 UTC
re #8: agreed... and I think it's even a sane behavior, as the dom spec failed to provide real means of accessing the viewport. If not asking the root, then who else?
Otherwise, how would that affect our Mozilla compatibility?
Comment 10 Maksim Orlovich 2006-07-12 04:31:16 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,