Bug 99380

Summary:	Regression : the "DLFP toolbar" (from http://linuxfr.org) doesn't work any more in konqueror
Product:	[Applications] konqueror	Reporter:	pierre <pierre.linux59>
Component:	khtml ecma	Assignee:	Konqueror Developers <konq-bugs>
Status:	RESOLVED FIXED
Severity:	normal
Priority:	NOR
Version:	unspecified
Target Milestone:	---
Platform:	Compiled Sources
OS:	Linux
Latest Commit:		Version Fixed In:
Sentry Crash Report:

Description pierre 2005-02-14 17:41:09 UTC

Version:            (using KDE Devel)
Installed from:    Compiled sources
Compiler:          gcc version 3.3.4 pre 3.3.5 20040809
OS:                Linux

Hi

Since I switched from KDE 3.3.2 to KDE 3.3.9x, I can't use the DLFP (Da Linux French Page) toolbar any more.
This toolbar is totally written in Javascript, and has a lot of fonctionnalities :
1- Select your CSS on linuxfr
2- What is the smaller note you want to see
3- Browse in the unread comments
Explanation : linuxfr is like slashdot : you can comment a piece of news for instance, and the other people click on the "Usefull" or "Useless" links to give a note to your comment.
So when you refresh a page, the toolbar is really usefull to go directly to the unread comments !
And it worked perfectly with Konqueror 3.3.2. A friend tested with Konqueror 3.3.2 and confirmed : it works...
If you want to test it, you must open an account on linuxfr.org...

Thanks for reading me !

Comment 1 pierre 2005-02-14 18:57:26 UTC

After one hour of testing, code reading... I discovered that when changing the user agent to opera, the toolbar works !
Take a look at the toolbar sourcecode :
http://linuxfr.org/dlfptoolbar.js
Here is the code including the user agent check :
function goto_newcomment(j)
{
  if (myarray.length == 0) return;
  var ancre;
   if (j < 0) j = myarray.length - 1;
  if (j >= myarray.length) j = 0;
  current_number = j;
  ancre = getChild(myarray[j].parentNode, 0);
  if (navigator.userAgent.toLowerCase().indexOf("opera") != -1)
  {
    var theHref = window.location.href;
    if (theHref.indexOf("#") != -1)
      theHref = theHref.substring(0,theHref.indexOf("#"));
    window.location.href = theHref+"#"+ancre.name;
    replaceText('current', current_number + 1);
    return;
  }
  var List = document.getElementsByTagName("a");
  for (var i=0;i<List.length;i++)
  {
    if (List[i].name == ancre.name)
    {
      var topPos = getTopPos(List[i]);
      if (document.all)
          document.body.scrollTop = topPos;//offset+300;
      else
        document.documentElement.scrollTop = topPos;//offset+300;
      break;
    }
  }
  replaceText('current', current_number + 1);
}
So the problem is here :
    var theHref = window.location.href;
    if (theHref.indexOf("#") != -1)
      theHref = theHref.substring(0,theHref.indexOf("#"));
    window.location.href = theHref+"#"+ancre.name;
    replaceText('current', current_number + 1);
    return;
And I checked : when I click on the link to browse the comments, the URL in the location bar doesn't change !
But yet I'm coming out of my capacities :/

Comment 2 Germain Garand 2005-03-03 12:21:06 UTC

CVS commit by ggarand: 

Mozilla/IE strict compatibility:
document.documentElement.scroll{Top,Left} concern the canvas, not the root block.

fix regression on linuxfr.org

BUG: 99380


  M +7 -0      ChangeLog   1.394
  M +30 -6     ecma/kjs_dom.cpp   1.186


--- kdelibs/khtml/ChangeLog  #1.393:1.394
@@ -1,2 +1,9 @@
+2005-03-03  Germain Garand  <germain@ebooksfrance.org>
+
+        * ecma/kjs_dom.cpp (getValueProperty/putValueProperty):
+        Mozilla/IE strict compatibility: document.documentElement.scroll{Top,Left}
+        concern the canvas, not the root block. 
+        Cf. http://www.quirksmode.org/viewport/compatibility.html
+
 2005-03-01  Allan Sandfeld Jensen <kde@carewolf.com>
 

--- kdelibs/khtml/ecma/kjs_dom.cpp  #1.185:1.186
@@ -293,7 +293,17 @@ Value DOMNode::getValueProperty(ExecStat
       return rend ? static_cast<Value>( Number(rend->scrollHeight()) ) : Undefined();
     case ScrollLeft:
-      return Number( rend && rend->layer() ? rend->layer()->scrollXOffset() : 0 );
+      if (rend && rend->layer()) {
+          if (rend->isRoot() && !rend->style()->hidesOverflow())
+              return Number( node.ownerDocument().view() ? node.ownerDocument().view()->contentsX() : 0);
+          return Number( rend->layer()->scrollXOffset() );
+      }
+      return Number( 0 );
     case ScrollTop:
-      return Number( rend && rend->layer() ? rend->layer()->scrollYOffset() : 0 );
+      if (rend && rend->layer()) {
+          if (rend->isRoot() && !rend->style()->hidesOverflow())
+              return Number( node.ownerDocument().view() ? node.ownerDocument().view()->contentsY() : 0);
+          return Number( rend->layer()->scrollYOffset() );
+      }
+      return Number( 0 );
     default:
       kdDebug(6070) << "WARNING: Unhandled token in DOMNode::getValueProperty : " << token << endl;
@@ -394,12 +404,26 @@ void DOMNode::putValueProperty(ExecState
   case ScrollTop: {
     khtml::RenderObject *rend = node.handle() ? node.handle()->renderer() : 0L;
-    if (rend && rend->layer() && rend->style()->hidesOverflow())
+    if (rend && rend->layer()) {
+        if (rend->style()->hidesOverflow())
         rend->layer()->scrollToYOffset(value.toInt32(exec));
+        else if (rend->isRoot()) {
+            QScrollView* sview = node.ownerDocument().view();
+            if (sview)
+                sview->setContentsPos(sview->contentsX(), value.toInt32(exec));
+        }
+    }       
     break;
   }
   case ScrollLeft: {
     khtml::RenderObject *rend = node.handle() ? node.handle()->renderer() : 0L;
-    if (rend && rend->layer() && rend->style()->hidesOverflow())
+    if (rend && rend->layer()) {
+        if (rend->style()->hidesOverflow())
       rend->layer()->scrollToXOffset(value.toInt32(exec));
+        else if (rend->isRoot()) {
+            QScrollView* sview = node.ownerDocument().view();
+            if (sview)
+                sview->setContentsPos(value.toInt32(exec), sview->contentsY());
+        }
+    }
     break;
   }

Comment 3 Pinaraf 2005-03-03 19:02:47 UTC

Thanks :)

Comment 4 Germain Garand 2005-03-18 17:27:05 UTC

CVS commit by ggarand: 

backport regression fix
CCBUG: 99380


  M +30 -6     kjs_dom.cpp   1.185.2.1


--- kdelibs/khtml/ecma/kjs_dom.cpp  #1.185:1.185.2.1
@@ -293,7 +293,17 @@ Value DOMNode::getValueProperty(ExecStat
       return rend ? static_cast<Value>( Number(rend->scrollHeight()) ) : Undefined();
     case ScrollLeft:
-      return Number( rend && rend->layer() ? rend->layer()->scrollXOffset() : 0 );
+      if (rend && rend->layer()) {
+          if (rend->isRoot() && !rend->style()->hidesOverflow())
+              return Number( node.ownerDocument().view() ? node.ownerDocument().view()->contentsX() : 0);
+          return Number( rend->layer()->scrollXOffset() );
+      }
+      return Number( 0 );
     case ScrollTop:
-      return Number( rend && rend->layer() ? rend->layer()->scrollYOffset() : 0 );
+      if (rend && rend->layer()) {
+          if (rend->isRoot() && !rend->style()->hidesOverflow())
+              return Number( node.ownerDocument().view() ? node.ownerDocument().view()->contentsY() : 0);
+          return Number( rend->layer()->scrollYOffset() );
+      }
+      return Number( 0 );
     default:
       kdDebug(6070) << "WARNING: Unhandled token in DOMNode::getValueProperty : " << token << endl;
@@ -394,12 +404,26 @@ void DOMNode::putValueProperty(ExecState
   case ScrollTop: {
     khtml::RenderObject *rend = node.handle() ? node.handle()->renderer() : 0L;
-    if (rend && rend->layer() && rend->style()->hidesOverflow())
+    if (rend && rend->layer()) {
+        if (rend->style()->hidesOverflow())
         rend->layer()->scrollToYOffset(value.toInt32(exec));
+        else if (rend->isRoot()) {
+            QScrollView* sview = node.ownerDocument().view();
+            if (sview)
+                sview->setContentsPos(sview->contentsX(), value.toInt32(exec));
+        }
+    }       
     break;
   }
   case ScrollLeft: {
     khtml::RenderObject *rend = node.handle() ? node.handle()->renderer() : 0L;
-    if (rend && rend->layer() && rend->style()->hidesOverflow())
+    if (rend && rend->layer()) {
+        if (rend->style()->hidesOverflow())
       rend->layer()->scrollToXOffset(value.toInt32(exec));
+        else if (rend->isRoot()) {
+            QScrollView* sview = node.ownerDocument().view();
+            if (sview)
+                sview->setContentsPos(value.toInt32(exec), sview->contentsY());
+        }
+    }
     break;
   }