Bug 130577

Summary: rendering of Amazon Online Reader is wrong
Product: [Applications] konqueror Reporter: Simon Perreault <nomis80>
Component: khtmlAssignee: Konqueror Developers <konq-bugs>
Status: RESOLVED FIXED    
Severity: normal CC: maksim
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Fedora RPMs   
OS: Linux   
Latest Commit: Version Fixed In:
Attachments: patch

Description Simon Perreault 2006-07-10 22:29:26 UTC
Version:            (using KDE KDE 3.5.3)
Installed from:    Fedora RPMs

On amazon.com, click on a book image marked "Search inside!" You should be directed to the Amazon Online Reader. This doesn't render correctly in Konqueror. Using various user-agent strings, the one which worked best was the default one. I can see two problems:

- The left sidebar doesn't display.
- The center display area is cropped vertically. It should be taller, extending the whole height of the browser.
Comment 1 Maksim Orlovich 2006-07-10 23:26:28 UTC
somehow we don't get some of the style attrs on that sidebar...
Comment 2 Maksim Orlovich 2006-07-10 23:40:20 UTC
hmm, might be clientHeight in the body...Now where did I put this patch...
Comment 3 Maksim Orlovich 2006-07-11 00:01:34 UTC
Yep, my patch to fix how these properties work on body fixes it. Problem is, I did the work on it like half a year back, and don't remember the details anymore, but do remember there was something I wasn't happy with. 

Comment 4 Maksim Orlovich 2006-07-11 00:03:19 UTC
Created attachment 16948 [details]
patch

here it is. Now I just need to find my notes on it, redo all the testing, find
mail conversations with other developers. (sigh)
Comment 5 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,
Comment 6 Florian Hackenberger 2006-12-11 19:58:49 UTC
Is this supposed to be fixed in 3.5.5? Because Amazon look inside still doesn't render correctly for me (can't see the page).