#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 May 30th, 2012 @ 05:59 AM

    Welcome to our Michael Kors handbags on saleonline sales shop, where you can always buy the most affordable price to a your favorite Michael Kors outlet online. Michael Kors handbags are famous that nobody can ignore the existence of it in the fashion world. This summer, Michael Kors Canada can fulfill your expectation for making a particular image. Please do not hesitate to buy one of them, our Michael Kors Sales online shop will provide the lowest price for you.

  • cxf0401

    cxf0401 May 30th, 2012 @ 06:01 AM

    Add a commenA classic design michael kors outlet.Spacious and elegant, in supple

    lightweight patent leather with an elaborately inlaid logo design,this michael

    kors handbags outlet tote is the bag you will always find yourself reach for. Michael Kors handbags are newest style &

    excellent quality.Welcome to our online sales shop, where you can always buy the most affordable price to a your favorite

    michael kors outlet online
    t

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