1. #1
    Ext User
    Join Date
    May 2007
    Posts
    5
    Vote Rating
    0
    Zuni is on a distinguished road

      0  

    Default Traced: bug in core Ext.Elements getAlignToXY, another bug remains

    Traced: bug in core Ext.Elements getAlignToXY, another bug remains


    Another bug that can also be seen in the examples, although not so obvious at first since most example pages are so short.

    If you have scrolled the page down (whether the page be a longer document, or your browser window is smaller) and hover over Ext-submenus you get pretty wacky results. The opening submenu opens in the wrong position (doesn't seem to take the scroll into account) on both FF and IE7 and is thus usually unreachable. In addition to this even hovering over the parent menu that shows the submenu causes the browser window to instascroll to the top (on FF), completely wreaking havoc on any attempt to use the menus.

    You can see the effect on, for example, these pages:
    (To make it easier to spot make the page longer by, for example, applying a 600px bottom margin to the page h1-element with firebug, or just make your browser window really small heightwise).

    Grid:
    http://www.extjs.com/deploy/ext/exam...rray-grid.html
    Right click a column and bring out the Columns submenu (while the page has been scrolled down, more the greater the effect).

    Datemenu as submenu:
    http://www.extjs.com/deploy/ext/exam...enu/menus.html
    Open the mainmenu and the try to open the datemenu submenu (while the page has been scrolled down, more the greater the effect).

    You might need to try it a few times to get the effect, there seems to be some effort the script is doing to prevent this, but more often than not it fails. If you scroll while the main menu is visible, it seems to take the scroll into account when determining submenu position. If you scrolled before, however, things go haywire.

    This is a major problem if you plan on putting any of the menus or elements with submenus anywhere expect the top part of your page.

    Tested on FF, IE7. Same effect on all underlying libraries (jQuery, ProtoType, YUI).
    Last edited by Zuni; 7 May 2007 at 2:00 AM. Reason: Changed title

  2. #2
    Ext User
    Join Date
    May 2007
    Posts
    5
    Vote Rating
    0
    Zuni is on a distinguished road

      0  

    Default Traced the bug to Core: Ext.Element

    Traced the bug to Core: Ext.Element


    I think I've managed to trace the bug to Ext.Elements getAlignToXY-method.

    The code is basically responsible for comparing the positions of the elements that are to be aligned with each other, and providing the coordinates for the element. It checks the relative alignment (tl-tr? or such) and calculates the position accordingly.

    It also checks that the element to be positioned is in the viewport, and doesn't, for example, bleed over from the bottom (it adjusts the y-coordinate if this is the case). And this is where the bug resides.

    Element.js, lines 1538-1540:
    Code:
    if((y+h) > dh){
        y = swapY ? r.top-h : dh-h;
    }
    Here y is the previously calculated y-coordinate for the element to be positioned, h is the height of the element, and dh is the height of the browsers viewport. r holds an object containing the other objects coordinates.

    It (attempts to) checks if the element is about to go below the current visible area, and adjust it upwards if so. The problem is that it doesn't take scroll into account at all. Say that I have scrolled 300px down the page, and my browsers viewport height is 550px. An element 250px tall is trying to get positioned to y-coordinate 500. This should be fine, since currently my viewport is showing y-coordinates 300 to 850 of the page, and the element would take from 500 to 750. Since the code ignores the scroll, however, it calculates that since y+h (750) is higher than dh (550), there is something wrong, and tries to adjust the y-coordinate of the element, and things go wrong.

    A more correct format for the if-clause would be something like:
    Code:
    if((y+h) > dh + scrollY){
        y = swapY ? r.top-h : dh-h;
    }
    Even this does not seem to be enough, tho. This does fix the erronous triggering of the repositioning code, but even when it is triggered it seems to fail on some occasions.

    Earlier, swapY is true if the relational positioning is of top-to-bottom or bottom-to-top form, but false if it is bottom-to-bottom or top-to-top. So, as with per submenu defaults you are positioning the submenu topleft to mainmenu topright), swapY is false. In this case it, again ignoring scroll, tries to position itself to the bottom of the viewport. If, on the other hand, we were positioning the submenu below the mainmenu item, say top-left to bottom-left, and swapY would be true, then it would try to position itself right above the main menu (r.top-h). This seems to be pretty ok as such.

    So a fixed version without changes to the swapY's behavior (which I think should be considered), goes:
    Code:
    if((y+h) > dh + scrollY){
        y = swapY ? r.top-h : dh+scrollY-h;
    }
    The other, even more irritating bug remains. Whe fast mouse-overing the mainmenu items (that each display a submenu), the viewport instant-scrolls to the top of the page. This totally prevents using the submenus on any part of the page that requires scrolling. The funny thing is that this consistently works perfect if you hover over and between the items really slow, but not if you do it even a bit faster. Might relate to mouseout or mouseover firing differently in these cases.

    Note:
    The bugs fixed above also affect the lines 1532 to 1534 that deal with doing the same check on X-axis. The corrected format would be:

    Code:
    if((x+w) > dw + scrollX){
        x = swapX ? r.left-w : dw+scrollX-w;
    }

  3. #3
    Sencha User jack.slocum's Avatar
    Join Date
    Mar 2007
    Location
    Tampa, FL
    Posts
    6,955
    Vote Rating
    17
    jack.slocum will become famous soon enough jack.slocum will become famous soon enough

      0  

    Default


    Zuni,

    Thanks for taking the time to track these down and provide such a good explanation. I have tested the fix and it works perfect.

    Thanks again!
    Jack Slocum
    Ext JS Founder
    Original author of Ext JS 1, 2 & 3.
    Twitter: @jackslocum
    jack@extjs.com

  4. #4
    Ext User digeomel's Avatar
    Join Date
    Mar 2007
    Posts
    67
    Vote Rating
    0
    digeomel is on a distinguished road

      0  

    Default


    Quote Originally Posted by jack.slocum View Post
    Zuni,

    Thanks for taking the time to track these down and provide such a good explanation. I have tested the fix and it works perfect.

    Thanks again!
    But I don't see this implemented in 1.0.1a

  5. #5
    Ext JS Premium Member
    Join Date
    Apr 2007
    Posts
    295
    Vote Rating
    7
    jheid will become famous soon enough

      0  

    Default


    Quote Originally Posted by digeomel View Post
    But I don't see this implemented in 1.0.1a
    You can fix it by your own or wait for the next release...

  6. #6
    Ext User digeomel's Avatar
    Join Date
    Mar 2007
    Posts
    67
    Vote Rating
    0
    digeomel is on a distinguished road

      0  

    Default


    Quote Originally Posted by jheid View Post
    You can fix it by your own or wait for the next release...
    I did fix it by myself, although it took me a little more to find the right code block in the obfuscated/compressed version of ext-all.js.

    I am just surprised that although it's been a while since the bug was reported and the bug fix acknowledged, it hasn't been applied yet.

    Don't get me wrong, I'm not complaining here, Jack is doing a great job, I'm just wondering whether he's missed this one.

  7. #7
    Sencha User
    Join Date
    Mar 2007
    Posts
    7,854
    Vote Rating
    4
    tryanDLS is on a distinguished road

      0  

    Default


    There hasn't been a release since 1.0.1a, which pre-dates your post, so where would you expect to see it? It's fixed in SVN and will be in the next release (no date yet).

  8. #8
    Sencha User
    Join Date
    Apr 2012
    Location
    Austin, Texas
    Posts
    2
    Vote Rating
    0
    brian.moeskau is an unknown quantity at this point

      0  

    Default


    Our maintenance releases are usually more frequent, but this particular one has taken a bit longer than usual. By the way, for debugging you should be using ext-all-debug.js since it is not compressed.