Prototype

Comments and changes to this ticket

• 

  Padraig Kennedy May 9th, 2008 @ 12:39 PM

  Looked into this further.

  It seems that since the element is 'HTML' in this case the initial ifs don't catch it. The while loop then tries to get the positioning of the HTML parentNode, which doesn't have a style attribute, so it errors.

  Adding in another 'if' to test for 'HTML' and returning the element itself seems to solve it.

  Hope this helps,

  Pádraig.

    getOffsetParent: function(element) {                       
  	if (element.offsetParent) return $(element.offsetParent);
      if (element == document.body) return $(element);
      if (element.nodeName == 'HTML') return $(element); // This is added
  
      while ((element = element.parentNode) && element != document.body) {               
  	  if (Element.getStyle(element, 'position') != 'static') {
          return $(element);                                    
        }
  	}                                                           
      return $(document.body);
    },    
  

• 

  Juriy Zaytsev May 9th, 2008 @ 02:26 PM

  This should be fixed in a trunk version.

  Could you check if it solves the issue?

• 

  Padraig Kennedy May 10th, 2008 @ 05:18 AM

  I downloaded and built the latest version from git just now, and the problem is still there.

  The IE wrapper on the getViewportOffset function calls getOffsetParent on the 'html' element which causes the error described above.

  PK

• 
  This has been marked as spam

  Nick Stakenburg May 10th, 2008 @ 07:15 AM

  It's fixed in Andrew's branch, once that merges with Sam's it'll be fixed.

  http://github.com/savetheclocktower/prototype

• 
  This has been marked as spam

  Padraig Kennedy May 10th, 2008 @ 07:30 AM

  Great; thanks guys. I'll try it out again when the next version is out.

  PK

• 

  John-David Dalton May 13th, 2008 @ 10:39 AM

  • → Title changed from “clonePosition() problem in IE7” to “Element#getOffsetParent() IE issue (effects several other methods)”
  • → State changed from “new” to “bug”
  • → Milestone changed from “” to “1.6.0.3”

  Padraig Kennedy can you try my fork -> I have fixed the offsetParent bug via:

  2) [ #11473 ] Element#getOffsetParent and Element#viewportOffset IE error:

  Patch

  Unit Tests

  Changelog

• 

  John-David Dalton May 13th, 2008 @ 10:39 AM

  By the way I don't see this as spam :D

• 

  John-David Dalton May 13th, 2008 @ 10:41 AM

  • → Assigned user changed from “” to “John-David Dalton”
• 

  John-David Dalton May 13th, 2008 @ 01:24 PM

  • → Title changed from “Element#getOffsetParent() IE issue (effects several other methods)” to “Element#getOffsetParent IE issue (effects several other methods)”
• 

  Nick Stakenburg May 13th, 2008 @ 07:14 PM

  It's more complex then the patch needed for the issue discribed here or what's discribed as fixed in the patched changelog. Seems like this approach fixes another issue alltogether with strict doctypes, nice work.

• 

  John-David Dalton May 13th, 2008 @ 07:31 PM

  Thanx,

  Well, my patch fixes issues with methods like clonePosition, viewportOffset, and getOffsetParent because they all suffered from the same issue of incorrectly getting documentElement back as the offsetParent instead of document.body.

  I don't think its more complex than needed to fix the issue, I think it simply addresses the issue.

  The changelog reflects the symptom of the issue reported. Though the actuall fix is deeper than that. I will put a note on the commit about a possible edit.

  A related ticket for viewportOffset and Opera is here

• 

  Nick Stakenburg May 13th, 2008 @ 08:16 PM

  The reason I think it's complex is because the IE style bug discussed here is fixed in Andrew's branch like this:

  http://github.com/savetheclockto...

  That ensures you don't end up with the document node, effectively fixing the style issue. Your patch introduces returning document.body over document.documentElement in getOffsetParent, a good thing imo.

  I opened the related ticket for Opera a while back, it was added in the same commit:

  http://github.com/savetheclockto...

  related to that:

  http://prototype.lighthouseapp.c...

• 

  Padraig Kennedy May 14th, 2008 @ 05:06 AM

  Hi John,

  I tested it with your fork and the issue is resolved.

  Thanks,

  Pádraig.

• 

  John-David Dalton May 14th, 2008 @ 07:55 AM

  Ahh nodeType == 9, pretty cool :)

  In regards to the Opera fix, so that patch makes Opera 9.5 work? I am not familiar with the bug, so forgive my ignorance. What about the previous versions of Opera that have the bug? Won’t checking for the HTML tag instead of the BODY cause them to fail?

  By the way, I never put togeather that you where "staaky" until now :)

• 

  Nick Stakenburg May 14th, 2008 @ 08:07 AM

  Yes, it makes it works on Opera 9.5. The problem was that scrollTop and scrollLeft are set on both the HTML and the BODY element in previous versions of Opera. Opera 9.5 now supports it like other browsers, having those properties only on the HTML element. Without the patch an invalid offset is returned in Opera 9.5. The patch is backwards compatible.

• 

  John-David Dalton May 16th, 2008 @ 09:00 AM

  Andrew's patch will fail this unit test:

  
      /* IE with strict doctype may try to return documentElement as offsetParent on relatively positioned elements */  
      $(document.body).insert( new Element('div', {id:'ie_offset_parent_bug'}).setStyle('position:relative'));  
      this.assertEqual('BODY', $('ie_offset_parent_bug').getOffsetParent().tagName.toUpperCase()); 
  

  because it assumes the element.offsetParent is correct.

• 

  Nick Stakenburg May 17th, 2008 @ 08:57 AM

  I'm for returning document.body over document.documentElement, but also for simplicity. How about this approach?

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

• 

  John-David Dalton May 17th, 2008 @ 09:51 AM

  Nice, I can dig it :)

• 

  Taco van den Broek June 3rd, 2008 @ 09:52 AM

  @John-David: In reply to to ticket #134 regarding this issue I've proposed another fix and added some unit tests. Please take a look at my reply and patch.

• 

  John-David Dalton June 20th, 2008 @ 04:08 PM

  • → State changed from “bug” to “resolved”

  resolved