Prototype

Comments and changes to this ticket

• 

  Taco van den Broek June 3rd, 2008 @ 02:40 PM

  • → Title changed from “style not always defined in call to getStyle()” to “style not always defined in call to getStyle() ”

  This problem has to do with a bug in the current Element.getOffsetParent() function. When any ancestor of the body element is passed to this function in IE7 this problem will occur, try:

  Element.getOffsetParent(document.body.parentNode);
  

  In the current source in git this problem is 'solved' by adding && element !== document to the while guard of the getOffsetParent function in dom.js. This will cause Element.getOffsetParent() to return document.body when the offsetParent of any ancestor of the body is requested what could lead to an infinite loop (since the resulting object isn't the passed element and still has parentNodes).

  A better solution would be to return the passed element when it is an ancestor of the body element. I have attached a patch that will add this behaviour and unit tests to confirm this behaviour.

  [offtopic]

  Why isn't there a convention about line-endings in the prototype source? Due to inconsistent line-endings my diff is way to large!

  [/offtopic]

  • 0001-fixed-IE7-bug-when-passing-an-ancestor-of-document.b.patch 89 KB
• 

  John-David Dalton June 3rd, 2008 @ 02:56 PM

  returning document.body keeps the functionality consistent between minor releases also it allows you to simple check for element != document.body in your loops and avoids more complex logic. If done correctly there is no fear of infinite looping :)

• 

  John-David Dalton June 3rd, 2008 @ 02:56 PM

  • → State changed from “new” to “duplicate”
• 

  Taco van den Broek June 3rd, 2008 @ 03:01 PM

  Good point, the unit tests should be altered slightly then. Passing the document object will still cause errors in IE7 though but that might be a too far-fetched edge case ;)

• 

  John-David Dalton June 3rd, 2008 @ 08:01 PM

  See if this gives ya any issues in IE 7.

  This is the one from my fork:

  getOffsetParent: function(element) {
    element = $(element);
    var op = element.offsetParent, docElement = document.documentElement;
    if (op && op != docElement) return $(op);
  
    while ((element = element.parentNode) && element !== docElement &&
     element !== document) {
      if (Element.getStyle(element, 'position') !== 'static')
        return $(element);
      }
    return $(document.body);
  }
  

• 

  John-David Dalton June 3rd, 2008 @ 08:38 PM

  Welp you are correct :D,

  though your patch addresses it in a round about way.

  The error is actually caused further down in the code when "If IE" it wraps the getOffsetParent method and does a Element#getStyle check on the passed "element". So when its document it fails. I don't see this as an issue because the "document" object is not a valid Element and so you wouldn’t expect it to work.

  I don't think we should guarantee non Elements working in any Element method.