Problems with HEAD and latest version of HierMenus

David Joham djoham at yahoo.com
Sat Apr 12 22:38:34 BST 2003


Hi everyone,

I'm working with the folks who develop the Hiermenus to make sure their next version works with
Konqueror and I've run into a couple of issues I'd like to have some discussions on.

The first is a crasher bug. It only happens on HEAD, it does *not* happen on my 3.1/3.1.1 Mandrake
9.1 hybrid. Here's the backtrace:

[New Thread 16384 (LWP 32076)]
0x410e0677 in waitpid () from /lib/i686/libpthread.so.0
#0  0x410e0677 in waitpid () from /lib/i686/libpthread.so.0
#1  0x407fe327 in KCrash::defaultCrashHandler(int) ()
   from /home/kde3/installs/kde/lib/libkdecHi everyone,

I'm working with the folks who develop the Hiermenus to make sure their next version works with
Konqueror and I've run into a couple of issues I'd like to have some discussions on.

The first is a crasher bug. It only happens on HEAD, it does *not* happen on my 3.1/3.1.1 Mandrake
9.1 hybrid. Here's the backtrace:

[New Thread 16384 (LWP 32076)]
0x410e0677 in waitpid () from /lib/i686/libpthread.so.0
#0  0x410e0677 in waitpid () from /lib/i686/libpthread.so.0
#1  0x407fe327 in KCrash::defaultCrashHandler(int) ()
   from /home/kde3/installs/kde/lib/libkdecore.so.4
#2  0x410df5ce in __pthread_sighandler () from /lib/i686/libpthread.so.0
#3  0x412433b8 in __libc_sigaction () from /lib/i686/libc.so.6
#4  0x410dcdeb in raise () from /lib/i686/libpthread.so.0
#5  0x41243224 in raise () from /lib/i686/libc.so.6
#6  0x4124476b in abort () from /lib/i686/libc.so.6
#7  0x4123c745 in __assert_fail () from /lib/i686/libc.so.6
#8  0x41e14536 in KJS::WindowQObject::timerEvent(QTimerEvent*) ()
   from /home/kde3/installs/kde/lib/libkhtml.so.4

The console spits out the following error:

konqueror: kjs_window.cpp:1617: virtual void KJS::WindowQObject::timerEvent(QTimerEvent*):
Assertion `!scheduledActions.isEmpty()' failed.


Peter, it looks like this is failing in your new code in kjs_window.cpp verion 1.329 where you
tried to sync the window.setTimeout functions. Is an assert that fails supposed to crash the
program? I commented the assert line out and everything seemed fine, but I don't know if that will
break your fix to the original problem. Any input here?

The second problem is rather minor (the menu widths aren't quite correct and can cut off some
text), but annoying just the same. You can see an example of the problem at the following address
when you mouse over the Relative-to-Mouse-Position Vertical Variable-Width Popup Menu (Displays on
mouseover) link and follow the menus to the 3rd level: 

http://webreference.com/dhtml/column65/HM4-2-3/LoadMe.html


The problem seems to be that we don't always update our render tree in real-time when nodes are
modified via JavaScript. The heirmenus have a variable width menu that finds the widest menu item
and adjusts itself and its menu items to the width of the that menu item using this code:

//for context, "this" points to the menu as a whole. Items is an array of menu elements

if(this.IsVariableWidth) {
   var MaxItemWidth = 0;
   for(var i=0; i<ItemCount; i++) {
      TempItem = Items[i];
      TempItem.realWidth = (HM_IE) ? TempItem.scrollWidth : TempItem.offsetWidth;
      if (isNaN(TempItem.realWidth)) TempItem.realWidth=TempItem.offsetWidth;
      if(HM_IE5M) TempItem.realWidth += (parseInt(TempItem.style.paddingLeft) +
parseInt(TempItem.style.paddingRight))
               if(!HM_IEnoDTD && !HM_OperaQuirk) {
         TempItem.realWidth -= ((parseInt(TempItem.style.paddingRight) +
parseInt(TempItem.style.paddingLeft)))
      }
      MaxItemWidth = i ? Math.max(MaxItemWidth,TempItem.realWidth) : TempItem.realWidth;
      if(MaxItemWidth==TempItem.realWidth) var TempWidest = TempItem;

   }


   for(var i=0; i<ItemCount; i++) {
      Items[i].style.width = TempWidest.realWidth + "px";
      if(!HM_IEnoDTD && !HM_OperaQuirk) {
         if(Items[i].imgLyr&&!TempWidest.imgLyr) {
            Items[i].style.width = (TempWidest.realWidth - HM_ImageSize - HM_ImageHorizSpace -
HM_ItemPadding) + "px";
         }
         else {
            try {
               Items[i].OffsetY;
            }
            catch(e) {
            }
            Items[i].style.paddingLeft = TempWidest.style.paddingLeft;
            Items[i].style.paddingRight = TempWidest.style.paddingRight;
         }
      }
   }

   this.style.width = this.scrollParent.style.width = (Items[0].offsetWidth +  ((HM_IEnoDTD ||
HM_OperaQuirk) ? HM_BorderWidth * 2 : 0)) + "px";
}


There is a difference in behavior between HEAD and 3.1. In 3.1, neither the widths of the menu
items or the menu itself is widened to the width of the TempWidest object. In HEAD, the menu items
are widened, but the menu itself isn't modified. This leads me to belive that the following line
of code is failing in HEAD:

   this.style.width = this.scrollParent.style.width = (Items[0].offsetWidth +  ((HM_IEnoDTD ||
HM_OperaQuirk) ? HM_BorderWidth * 2 : 0)) + "px";


Trying to figure out what was going on with HEAD, I took a quick look at the implementation of
offsetWidth in khtml/ecma/kjs_dom.cpp and found these lines of code:

    // no DOM standard, found in IE only

    // make sure our rendering is up to date before
    // we allow a query on these attributes.
    DOM::DocumentImpl* docimpl = node.handle()->getDocument();
    KHTMLView* v = 0;
    if ( docimpl ) {
      v = docimpl->view();
      // Only do a layout if changes have occurred that make it necessary.
      if ( v && docimpl->renderer() && !docimpl->renderer()->layouted() )
      {
        docimpl->updateRendering();
        docimpl->view()->layout();
      }

So it looks like we're trying to be smart about when we do a relayout when offsetWidth is being
called. However, something the menus are doing in the resize code above is causing a layout change
that is not being caught by the if ( v && docimpl->renderer() && !docimpl->renderer()->layouted()
) statement. This is causing the menus to not resize properly.

If I take the if statement out and always force a render update, the menus render properly

Does anyone see the reason why the heirmenu code's resize manipulation does not cause us to think
we need to do an updateRendering when offsetWidth is called?


Even if we fix this in HEAD, it's still broken in 3.1 so the hiermenus guys may want to introduce
a workaround to fix this for the 3.1 folks. This is the best I've come up with so far to force
Konq to re-render the page so the width calculations work. I put the code where it is so 3.1
(which remember doesn't resize the menu items or the menu itself properly) would work. 


Items[i].style.paddingLeft = TempWidest.style.paddingLeft;
Items[i].style.paddingRight = TempWidest.style.paddingRight;
if (HM_Konqueror) {
   //konq doesn't refresh the renderer properly during this
   //function. This forces it to do so
   try {
      var x = document.createElement("div");
      document.body.appendChild(x);
      //now remove it
      document.body.removeChild(x);
   }
   catch(e) {
      //just continue
   }
}

Can anyone think of a better/faster solution for 3.1?



Well, this email turned out to be much longer than I anticipated. Sorry. In summary, the three
things I'm looking for help on are the following

1) Why are we crashing in HEAD in the kjs_window.cpp code? Is it OK just to take out the assert?
2) Why are we not updating our rendering when offsetWidth is called in conjunction with the menu
resize code?
3) Is there a better workaround for the 3.1 folks than the one I've thought of?

Thanks for your help everyone!

Best regards,

Davidore.so.4
#2  0x410df5ce in __pthread_sighandler () from /lib/i686/libpthread.so.0
#3  0x412433b8 in __libc_sigaction () from /lib/i686/libc.so.6
#4  0x410dcdeb in raise () from /lib/i686/libpthread.so.0
#5  0x41243224 in raise () from /lib/i686/libc.so.6
#6  0x4124476b in abort () from /lib/i686/libc.so.6
#7  0x4123c745 in __assert_fail () from /lib/i686/libc.so.6
#8  0x41e14536 in KJS::WindowQObject::timerEvent(QTimerEvent*) ()
   from /home/kde3/installs/kde/lib/libkhtml.so.4

The console spits out the following error:

konqueror: kjs_window.cpp:1617: virtual void KJS::WindowQObject::timerEvent(QTimerEvent*):
Assertion `!scheduledActions.isEmpty()' failed.


Peter, it looks like this is failing in your new code in kjs_window.cpp verion 1.329 where you
tried to sync the window.setTimeout functions. Is an assert that fails supposed to crash the
program? I commented the assert line out and everything seemed fine, but I don't know if that will
break your fix to the original problem. Any input here?

The second problem is rather minor, but annoying just the same. You can see an example of the
problem at the following address when you mouse over the Relative-to-Mouse-Position Vertical
Variable-Width Popup Menu (Displays on mouseover) link:

http://webreference.com/dhtml/column65/HM4-2-3/LoadMe.html


The problwm seems to be that we don't always update our render tree in real-time when nodes are
modified via JavaScript. The heirmenus have a variable width menu that finds the widest menu item
and adjusts itself and its menu items to the width of the that menu item using this code:

//for context, "this" points to the menu as a whole. Items is an array of menu elements

if(this.IsVariableWidth) {
   var MaxItemWidth = 0;
   for(var i=0; i<ItemCount; i++) {
      TempItem = Items[i];
      TempItem.realWidth = (HM_IE) ? TempItem.scrollWidth : TempItem.offsetWidth;
      if (isNaN(TempItem.realWidth)) TempItem.realWidth=TempItem.offsetWidth;
      if(HM_IE5M) TempItem.realWidth += (parseInt(TempItem.style.paddingLeft) +
parseInt(TempItem.style.paddingRight))
               if(!HM_IEnoDTD && !HM_OperaQuirk) {
         TempItem.realWidth -= ((parseInt(TempItem.style.paddingRight) +
parseInt(TempItem.style.paddingLeft)))
      }
      MaxItemWidth = i ? Math.max(MaxItemWidth,TempItem.realWidth) : TempItem.realWidth;
      if(MaxItemWidth==TempItem.realWidth) var TempWidest = TempItem;

   }


   for(var i=0; i<ItemCount; i++) {
      Items[i].style.width = TempWidest.realWidth + "px";
      if(!HM_IEnoDTD && !HM_OperaQuirk) {
         if(Items[i].imgLyr&&!TempWidest.imgLyr) {
            Items[i].style.width = (TempWidest.realWidth - HM_ImageSize - HM_ImageHorizSpace -
HM_ItemPadding) + "px";
         }
         else {
            try {
               Items[i].OffsetY;
            }
            catch(e) {
            }
            Items[i].style.paddingLeft = TempWidest.style.paddingLeft;
            Items[i].style.paddingRight = TempWidest.style.paddingRight;
         }
      }
   }

   this.style.width = this.scrollParent.style.width = (Items[0].offsetWidth +  ((HM_IEnoDTD ||
HM_OperaQuirk) ? HM_BorderWidth * 2 : 0)) + "px";
}


There is a difference in behavior between HEAD and 3.1. In 3.1, neither the widths of the menu
items or the menu itself is widened to the width of the TempWidest object. In HEAD, the menu items
are widened, but the menu itself isn't modified. This leads me to belive that the following line
of code is failing in HEAD:

   this.style.width = this.scrollParent.style.width = (Items[0].offsetWidth +  ((HM_IEnoDTD ||
HM_OperaQuirk) ? HM_BorderWidth * 2 : 0)) + "px";


Trying to figure out what was going on with HEAD, I took a quick look at the implementation of
offsetWidth in khtml/ecma/kjs_dom.cpp and found these lines of code:

    // no DOM standard, found in IE only

    // make sure our rendering is up to date before
    // we allow a query on these attributes.
    DOM::DocumentImpl* docimpl = node.handle()->getDocument();
    KHTMLView* v = 0;
    if ( docimpl ) {
      v = docimpl->view();
      // Only do a layout if changes have occurred that make it necessary.
      if ( v && docimpl->renderer() && !docimpl->renderer()->layouted() )
      {
        docimpl->updateRendering();
        docimpl->view()->layout();
      }

So it looks like we're trying to be smart about when we do a relayout when offsetWidth is being
called. However, something the menus are doing in the resize code above is causing a layout change
that is not being caught by the if ( v && docimpl->renderer() && !docimpl->renderer()->layouted()
) statement. This is causing the menus to not resize properly.

If I take the if statement out and always force a render update, the menus render properly

Does anyone see the reason why the heirmenu code's resize manipulation does not cause us to think
we need to do an updateRendering when offsetWidth is called?


Even if we fix this in HEAD, it's still broken in 3.1 so the hiermenus guys may want to introduce
a workaround to fix this for the 3.1 folks. This is the best I've come up with so far to force
Konq to re-render the page so the width calculations work. I put the code where it is so 3.1
(which remember doesn't resize the menu items or the menu itself properly) would work. 


Items[i].style.paddingLeft = TempWidest.style.paddingLeft;
Items[i].style.paddingRight = TempWidest.style.paddingRight;
if (HM_Konqueror) {
   //konq doesn't refresh the renderer properly during this
   //function. This forces it to do so
   try {
      var x = document.createElement("div");
      document.body.appendChild(x);
      //now remove it
      document.body.removeChild(x);
   }
   catch(e) {
      //just continue
   }
}

Can anyone think of a better/faster solution for 3.1?



Well, this email turned out to be much longer than I anticipated. Sorry. In summary, the three
things I'm looking for help on are the following

1) Why are we crashing in HEAD in the kjs_window.cpp code? Is it OK just to take out the assert?
2) Why are we not updating our rendering when offsetWidth is called in conjunction with the menu
resize code?
3) Is there a better workaround for the 3.1 folks than the one I've thought of?

Thanks for your help everyone!

Best regards,

David

__________________________________________________
Do you Yahoo!?
Yahoo! Tax Center - File online, calculators, forms, and more
http://tax.yahoo.com




More information about the kfm-devel mailing list