search

prototypejs / prototype

Prototype JavaScript framework
http://prototypejs.org/
Other
3.54k stars 639 forks source link

clonePosition regression in 1.7.2 when scrolling page content #248

Closed fnappey closed 9 years ago

fnappey commented 10 years ago

On the same page, when I'm using de 1.7.0 version of Prototype and the Element.clonePosition to setup a sub menu position, it works all time. Calling the version 1.7.2 instead, it works if I don't scroll in page. But as soon as I scroll, the effect of the clonePosition method to setup the vertical position of an element is bugy. Tested on IE8 and last version of Chrome. Thanks in advance.

fntz commented 10 years ago

Can you add a sample code?

fnappey commented 10 years ago 
<!DOCTYPE html>
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
    <title></title>
    <script src="../_common/ajax/prototype-1.7.2.js"></script>
    <style type="text/css">
        body {
            margin: 0; padding: 0;
        }
        #Header {
            height: 400px;
            background-color: #f1f1f1;
        }
        #MainMenu {
            background-color: #0f0;
        }
        #SubMenu {
            position: absolute;
            top: 250px;
            left: 250px;
            background-color: #fc0;
        }
        #Scroller {
            height: 2000px;
        }
    </style>
    <script type="text/javascript">
        function ShowMenu() {
            Element.clonePosition($('SubMenu'), $('MainMenu'), { offsetTop: 20, offsetLeft: 0, setWidth: false, setHeight: false });
        }
    </script>
</head>
<body>
    <div id="Header">Header</div>
    <div id="MainMenu" onmouseover="ShowMenu()">Main Menu</div>
    <div id="Scroller">Force scroll</div>
    <div id="SubMenu">Sub Menu</div>
</body>
</html>

When the mouse comes overs Main menu, Sub menu should be deplaced just 20px below Main menu...

fnappey commented 10 years ago

Quickly comparing the code of both versions of Prototype, it seems that in previous versions, there was a special test for calculing delta when parent == document.body. In the version 1.7.2, in that case, delta is no more calculated.

fntz commented 10 years ago

No, delta calculated https://github.com/sstephenson/prototype/blob/d9411e5/src/prototype/dom/layout.js#L1524

But I understand correctly:

  1. correct http://i.imgur.com/dnETEto.png
  2. not correct http://i.imgur.com/BtMeJ8V.png

right?

fnappey commented 10 years ago

Yes, you understand correctly the problem. For myself, reading the code, delta is not calculated in case parent == document.body. In that case, delta = [0, 0]; In previous version, there was:

   var delta = [0, 0];

    if (Element.getStyle(element, 'position') == 'absolute') {
      delta = Element.viewportOffset(parent);
    }
 if (parent == document.body) {
      delta[0] -= document.body.offsetLeft;
      delta[1] -= document.body.offsetTop;
 }
fntz commented 10 years ago

oops. you right. Maybe you add the same method into Element:

Element.addMethod({
  clonePosition: function(element, source, options) { old method }
});
fnappey commented 10 years ago

For myself, I just reverted to version 1.7. I don't really need of 1.7.2. But it would be great if this was corrected in 1.7.3... clonePosition is one of the great function of Prototype, missing in standard jQuery.

fntz commented 10 years ago

Create a path with fix this bug.

fnappey commented 10 years ago

What does that mean ? Because this is the first time that I report an issue on Github, and I don't really know how it works... Where do you want that I create a path ?

fntz commented 10 years ago

Ok, i create a path for fix.

savetheclocktower commented 9 years ago

Fixed in 2514900f05deb9df5c84b24350f279ca9dc5e814; see #255.