#1286 ✓ resolved
Luis Fernando Planella Gonzalez

Error in Element.clonePosition

Reported by Luis Fernando Planella Gonzalez | November 28th, 2011 @ 07:24 PM | in 1.7.0.1

I'm experiencing a problem with clonePosition with Prototype 1.7.
It affects Firefox and Chrome (both supporting getBoundingClientRect).
After some struggling, I found that clonePosition has this:

if (parent == document.body) {
  delta[0] -= document.body.offsetLeft;
  delta[1] -= document.body.offsetTop;
}

The problem is that it should the same mechanism for positioning, as the ClientRect is different thatn the offsetLeft/Top:

if (parent == document.body) {
  var parentOffset = Element.viewportOffset(parent);
  delta[0] -= parentOffset[0];
  delta[1] -= parentOffset[1];
}

That fixes the issue.

Comments and changes to this ticket

  • David Dwiggins

    David Dwiggins December 22nd, 2011 @ 07:17 PM

    This fix just solved a nagging problem with autocomplete that has been plaguing the ResourceSpace DAM project. Anything that can be done to get it into the base code would be appreciated!

    -David

  • Andrew Dupont

    Andrew Dupont December 23rd, 2011 @ 09:56 PM

    • Milestone set to 1.7.0.1
    • State changed from “new” to “bug”
    • Assigned user set to “Andrew Dupont”
    • Importance changed from “” to “Medium”

    I'll try to get this in for the next version. Thanks!

  • Victor

    Victor January 19th, 2012 @ 10:22 AM

    So in this case parentOffset will be equal to delta and delta will be just zeroed:

          delta  = Element.viewportOffset(parent);
          // Adjust by BODY offsets. Fixes some versions of safari.
          if (parent === document.body) {
            var parentOffset = Element.viewportOffset(parent); // same as delta
            delta[0] -= parentOffset[0]; // delta[0] = 0;
            delta[1] -= parentOffset[1]; // delta[1] = 0;
          }
    

    and code can be simplified to:

          delta = (parent === document.body) ? [0, 0] : Element.viewportOffset(parent);
    
  • Dan Popescu

    Dan Popescu January 23rd, 2012 @ 09:01 AM

    but full code is:

    var p = Element.viewportOffset(source), delta = [0, 0], parent = null;
    
    if (Element.getStyle(element, 'position') === 'absolute') {
      parent = Element.getOffsetParent(element);
      delta  = Element.viewportOffset(parent);
    }
    
    if (parent === document.body) {
      var parentOffset = Element.viewportOffset(parent);
      delta[0] -= parentOffset[0];
      delta[1] -= parentOffset[1];
    }
    

    you get:
    1. element.position==='absolute' && parent===document.body => [0,0]
    2. element.position!=='absolute' && parent===document.body => [-parentOffset[0],-parentOffset[1]]
    3. element.position==='absolute' && parent!==document.body => [parentOffset[0],parentOffset[1]]
    4. element.position!=='absolute' && parent!==document.body => [0,0]

    so it should be:

    var p = Element.viewportOffset(source), delta = [0, 0], parent = Element.getOffsetParent(element);
    
    if (Element.getStyle(element, 'position') === 'absolute') {
      if(parent !== document.body) {
        delta  = Element.viewportOffset(parent);
      }
    } else {
      if(parent === document.body) {
        delta  = Element.viewportOffset(parent).map(function(x) { return -x;});
      }
    }
    
    if (parent === document.body) {
      var parentOffset = Element.viewportOffset(parent);
      delta[0] -= parentOffset[0];
      delta[1] -= parentOffset[1];
    }
    
  • Dan Popescu

    Dan Popescu January 23rd, 2012 @ 09:03 AM

    sorry, forget to delete bottom, it's

    var p = Element.viewportOffset(source), delta = [0, 0], parent = Element.getOffsetParent(element);
    
    if (Element.getStyle(element, 'position') === 'absolute') {
      if(parent !== document.body) {
        delta  = Element.viewportOffset(parent);
      }
    } else {
      if(parent === document.body) {
        delta  = Element.viewportOffset(parent).map(function(x) { return -x;});
      }
    }
    
  • Dan Popescu

    Dan Popescu January 23rd, 2012 @ 09:07 AM

    ok, i'm stupid - I hope it's because it's early
    of course you're right, disregard the last 2 posts

  • Victor

    Victor January 23rd, 2012 @ 12:07 PM

    No problem :) I have not shown the first important optimization step. Original code:

        var p = Element.viewportOffset(source), delta = [0, 0], parent = null;
        
        // A delta of 0/0 will work for `positioned: fixed` elements, but
        // for `position: absolute` we need to get the parent's offset.
        if (Element.getStyle(element, 'position') === 'absolute') {
          parent = Element.getOffsetParent(element);
          delta = Element.viewportOffset(parent);
        }
        
        // Adjust by BODY offsets. Fixes some versions of safari.
        if (parent === document.body) {
          delta[0] -= document.body.offsetLeft;
          delta[1] -= document.body.offsetTop;
        }
    

    here the second if() can be moved into the body of the first if(), where the parent is set to something different than null:

        var p = Element.viewportOffset(source), delta = [0, 0], parent = null;
    
        // A delta of 0/0 will work for `positioned: fixed` elements, but
        // for `position: absolute` we need to get the parent's offset.
        if (Element.getStyle(element, 'position') === 'absolute') {
          parent = Element.getOffsetParent(element);
          delta = Element.viewportOffset(parent);
    
          // Adjust by BODY offsets. Fixes some versions of safari.
          if (parent === document.body) {
            delta[0] -= document.body.offsetLeft;
            delta[1] -= document.body.offsetTop;
          }
        }
    

    and then comes your fix:

        var p = Element.viewportOffset(source), delta = [0, 0], parent = null;
    
        // A delta of 0/0 will work for `positioned: fixed` elements, but
        // for `position: absolute` we need to get the parent's offset.
        if (Element.getStyle(element, 'position') === 'absolute') {
          parent = Element.getOffsetParent(element);
          delta = Element.viewportOffset(parent);
    
          // Adjust by BODY offsets. Fixes some versions of safari.
          if (parent === document.body) {
            var parentOffset = Element.viewportOffset(parent);
            delta[0] -= parentOffset[0];
            delta[1] -= parentOffset[1];
          }
        }
    

    and we see delta = Element.viewportOffset(parent) and parentOffset = Element.viewportOffset(parent) eliminating each other in case of parent === document.body. Here comes the result:

        var p = Element.viewportOffset(source), delta = [0, 0];
    
        // A delta of 0/0 will work for `position: fixed` elements, but
        // for `position: absolute` we need to get the parent's offset.
        if (Element.getStyle(element, 'position') === 'absolute') {
          var parent = Element.getOffsetParent(element);
          delta = (parent === document.body) ? [0, 0] : Element.viewportOffset(parent); // FIX
        }
    
  • GitHub Robot

    GitHub Robot February 18th, 2012 @ 01:02 AM

    • State changed from “bug” to “resolved”

    (from [0bf9cd1674da5dd39f9980f0e9b78601b78e6ac5]) Fix the way we handle positioning in Element#clonePosition when the source element is absolutely-positioned and has the BODY as its offset parent. [#1286 state:resolved] (Luis Fernando Planella Gonzalez, Victor, Dan Popescu) https://github.com/sstephenson/prototype/commit/0bf9cd1674da5dd39f9...

  • Andrew Dupont

    Andrew Dupont February 18th, 2012 @ 01:04 AM

    Thanks for your help, gentlemen. That is a weird little piece of code.

  • Rolandow

    Rolandow April 25th, 2012 @ 10:48 AM

    Could somebody please paste the complete (fixed) version of the function? Would that be this?:

      clonePosition: function(element, source) {
        var options = Object.extend({
          setLeft:    true,
          setTop:     true,
          setWidth:   true,
          setHeight:  true,
          offsetTop:  0,
          offsetLeft: 0
        }, arguments[2] || { });
    
        source = $(source);
        var p = Element.viewportOffset(source), delta = [0, 0], parent = null;
    
        element = $(element);
    
        if (Element.getStyle(element, 'position') === 'absolute') {
          var parent = Element.getOffsetParent(element);
          delta = (parent === document.body) ? [0, 0] : Element.viewportOffset(parent); // FIX
        }
    
        if (options.setLeft)   element.style.left  = (p[0] - delta[0] + options.offsetLeft) + 'px';
        if (options.setTop)    element.style.top   = (p[1] - delta[1] + options.offsetTop) + 'px';
        if (options.setWidth)  element.style.width = source.offsetWidth + 'px';
        if (options.setHeight) element.style.height = source.offsetHeight + 'px';
        return element;
      }
    
  • David Dwiggins

    David Dwiggins May 9th, 2012 @ 05:09 PM

    Just a heads up that it appears this fix may have caused a new problem for the ResourceSpace autocomplete boxes, where in some cases they are appearing far up the page (or even off the page) from the object they are associated with. I have spent any time diagnosing this, but since the problem can be resolved by removing this fix, I assume it is related to this.

    (Without the fix the autocomplete boxes still appear overtop of the original fields, which is incorrect, but at least less of a problem than having them appear in random locations elsewhere on the page.)

  • Andrew Dupont

    Andrew Dupont May 23rd, 2012 @ 10:13 PM

    Sorry to hear that, David. I'm planning to release 1.7.0.1 within the next week, so if you can isolate the issue in time, and prove that it's still a Prototype bug, then please re-open this ticket. I might be able to apply a further fix (or rollback the existing one) before release. If not, no problem — there's always the next bugfix version.

  • Victor

    Victor May 26th, 2012 @ 10:37 AM

    David, could you give any test page which will show this error? And provide please a few details:

    • Is object (argument to clonePosition) absolutely positioned (what returns Element.getStyle(object, 'position'))?
    • Is it nested directly into document.body (its parent node === document.body) or not?
    • What returns Element.getOffsetParent(object)?
    • Have any parents of this object scroll bars, and are they scrolled or not?
  • cxf0401
  • cxf0401

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

The Prototype JavaScript library.

Shared Ticket Bins

Pages