#1137 bug
Mike

Element.relativize does not work properly

Reported by Mike | September 9th, 2010 @ 06:37 PM | in 1.8

The documentation says that Element.relativize is "used to undo a call to Element.absolutize." However, if you call Element.absolutize on an element and then call Element.relativize, the position of the element is still 'absolute' while the top and left values are reset to their original values.

There are two versions of these methods, one version in dom.js and the other in layout.js. This is unnecessary and confusing, especially since the one from dom.js works but is overridden by the broken one in layout.js.

To fix the problem, the the original position style needs to be added to the originalStyles object when calling absolutize.

Comments and changes to this ticket

  • ronin-118324 (at lighthouseapp)

    ronin-118324 (at lighthouseapp) October 2nd, 2010 @ 07:48 PM

    Notice:

    The method element.absolutize returns bad results (top, left position)
    On the line 2315 in prototype-1.6.1.js original:

     if (p !== 'static') break;
    

    so if the parent element has relative position
    th loop breaks.
    For me this works fine:

     if (p == 'static') break;
    

    the loops continues and add to the top and left parents position.

  • MarcusSchwarz

    MarcusSchwarz November 23rd, 2010 @ 06:32 PM

    This one is still there in 1.7

    Element.absolutize()
    

    makes use of the method

    element.store('prototype_absolutize_original_styles', {
    left:   element.getStyle('left'),
    top:    element.getStyle('top'),
    width:  element.getStyle('width'),
    height: element.getStyle('height')
    });
    

    In this case, the original position is not stored. So if

    Element.relativize()
    
    is called,
    var originalStyles = element.retrieve('prototype_absolutize_original_styles');
    
    is not able to update the position back to its original value.

    Before 1.7, Element.relativize() worked differently:

    element.style.position = 'relative';
    var top  = parseFloat(element.style.top  || 0) - (element.originalTop || 0);
    var left = parseFloat(element.style.left || 0) - (element.originalLeft || 0);
    

    It always sets the position-attribute to "relative", which is more or less the intentional meaning of that function.

    So either you store the position-attribute to "prototype_absolutize_original_styles", or you set the position-attribute to "relative" in Element.relativize()

  • Pete Deffendol

    Pete Deffendol December 10th, 2010 @ 06:51 PM

    It seems safer to restore the original position attribute when calling relativize. As such, then calls to absolutize should store it along with the actual position values.

    I've submitted a pull request at Github - https://github.com/sstephenson/prototype/pull/11

    It doesn't include tests - I'm not sure how to set those up at the moment, and the existing tests for absolutize/relativize seem rather incomplete as it is.

  • Mike

    Mike December 10th, 2010 @ 09:10 PM

    Pete's fix is the same fix I've been using in my own code for months now. It works.

  • ronin-118324 (at lighthouseapp)
  • Sébastien Grosjean - ZenCocoon

    Sébastien Grosjean - ZenCocoon December 17th, 2010 @ 01:45 AM

    There's an issue with relativize for sure and can be considered pretty serious for layout calculations as it doesn't set the position to relative at all, only revert an Element.absolutize call which doesn't save the initial position neither.

    Therefore, I would recommend something little different than proposed before:

    1) Fix Element.absolutize to store the current position before overwriting it.
    2) Move the actual Element.relativize to Element.undoAbsolutize as it's what it does.
    3) Make Element.relativize set the position to relative as it's what is expected.
    4) Make Element.undoRelativize to restore to the initial position.

    I've made a patch for this, tested and documented. You can find it the push request at https://github.com/sstephenson/prototype/pull/12 and the origin of the patch at https://github.com/ZenCocoon/prototype/tree/absolutize-relativize

  • Pete Deffendol

    Pete Deffendol December 17th, 2010 @ 05:31 PM

    I like Sébastien's approach because the method names are more meaningful, but the concern there is that it changes the API, which is probably best left to something more than a minor bugfix release. (Not that it's my decision.)

    Fundamentally, there was a regression from 1.6.x to 1.7, and the simpler fix takes care of it.

  • Sébastien Grosjean - ZenCocoon

    Sébastien Grosjean - ZenCocoon December 17th, 2010 @ 05:54 PM

    I agree, that would be backward incompatible and this is not nice.

    That can also have potential issues if undoing in wrong order.

    eg:

    Element.absolutize(element);
    Element.relativize(element);
    Element.undoAbsolutize(element);
    Element.undoRelativize(element);
    

    As of now, that would break and return the absolute position.

    A solution is to make Element.relativize trigger Element.undoAbsolutize before the rest of it's code. So as Element.absolutize to trigger Element.undoRelativize.

    I've updated my branch and the new push request is available at https://github.com/sstephenson/prototype/pull/13

    Thanks Pete for the warning.

  • Andrew Dupont

    Andrew Dupont December 18th, 2010 @ 10:20 AM

    • Milestone set to After 1.7
    • State changed from “new” to “bug”
    • Assigned user set to “Andrew Dupont”
    • Tag changed from layout relativize absolutize to absolutize, layout, relativize
    • Importance changed from “” to “Medium”

    I lean toward Pete's fix as a solution for 1.7.0.1, but something like Sébastien's fix as a solution for 1.8 (or maybe even 1.7.1). I say something like Sébastien's fix because I'd want absolutize and relativize to behave as much as possible like they've behaved historically, even though they're inaccurately named.

    I really don't like the fact that a method called relativize doesn't actually set position: relative on an element. But I'm willing to live with it rather than completely change the meaning of a method that's been in Prototype for years.

  • Sébastien Grosjean - ZenCocoon

    Sébastien Grosjean - ZenCocoon December 18th, 2010 @ 10:25 AM

    • Tag cleared.

    Hi Andrew,

    That make sense to keep the API, that's a really important thing when possible. Here why I believe my second patch ideal as keeping the API intact and behaving as it use to be isn't it?

    However it also bring the improved API for cleaner use and smooth transition over time.

    Anyway, having Element.relativize fixed remain the most important as of today as it quickly breaks layout calculation depending on Element.absolutize.

    Thanks for your hard work.

  • Sébastien Grosjean - ZenCocoon

    Sébastien Grosjean - ZenCocoon December 18th, 2010 @ 10:27 AM

    • Tag set to absolutize layout relativize

    Sorry for the tag cleaning. JS disabled made them go away.

  • Sébastien Grosjean - ZenCocoon

    Sébastien Grosjean - ZenCocoon December 18th, 2010 @ 10:27 AM

    • Tag changed from absolutize layout relativize to absolutize, layout, relativize
  • Andrew Dupont

    Andrew Dupont September 1st, 2012 @ 02:29 AM

    • Milestone changed from After 1.7 to 1.8
    • Importance changed from “Medium” to “High”
  • Jason Westbrook

    Jason Westbrook May 7th, 2014 @ 04:49 PM

    Fixed in pull request https://github.com/sstephenson/prototype/pull/11

    and included in the 1.7.2 release

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