#90 ✓ resolved
Padraig Kennedy

Element#getOffsetParent IE issue (affects several other methods)

Reported by Padraig Kennedy | May 9th, 2008 @ 06:38 PM | in 1.7

Element.clonePosition causes an error in IE7 when run on elements with position: absolute;

This is due to IE7 not returning a value for the viewportOffset(); function the first time it is called.

The error thrown by IE7 is: 'style' is null or not an object.

The attached file includes a test case which will cause the error.

Thanks,

Pádraig.

Comments and changes to this ticket

  • Padraig Kennedy

    Padraig Kennedy May 9th, 2008 @ 07: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

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

    This should be fixed in a trunk version.

    Could you check if it solves the issue?

  • Padraig Kennedy

    Padraig Kennedy May 10th, 2008 @ 12:18 PM

    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

  • Nick Stakenburg
  • Padraig Kennedy
  • John-David Dalton

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

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

    Nick Stakenburg May 14th, 2008 @ 03:16 AM

    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...

  • John-David Dalton

    John-David Dalton May 14th, 2008 @ 02:55 PM

    • no changes were found...
  • Nick Stakenburg

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

    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.

  • Nick Stakenburg

    Nick Stakenburg May 17th, 2008 @ 03:57 PM

    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);
      },
    
  • Taco van den Broek

    Taco van den Broek June 3rd, 2008 @ 04:52 PM

    @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.

  • rvagg

    rvagg October 4th, 2008 @ 04:57 AM

    • Tag set to ie6, ie7, patched, tested

    How come this hasn't made it to 1.6.0.3? I've been using 1.6.0 for a while now because this issue breaks a few things for me.

    btw, this bug was also covered fairly extensively in http://dev.rubyonrails.org/ticke... (including the reasons why this doesn't affect 1.6.0)

  • John-David Dalton
  • Jan Hansen

    Jan Hansen December 4th, 2008 @ 03:37 PM

    This bug is marked as resolved for milestone 1.6.0.3, but not included in version 1.6.0.3. Why not? Will it be included in 1.6.0.4?

    What is the best approach until it is "officially released"? I've replaced the code with Nick Stakenburg's suggested fix above, which seem to be working.

    Any advice?

  • Jon Evans

    Jon Evans December 10th, 2008 @ 02:24 PM

    This bug just got me too, using 1.6.0.3 with [window.js](http://prototype-window.xilinus.... window.js). Latest version from git master branch today was still broken. Release version 1.6.0.3 with Nick Stakenburg's replacement for getOffsetParent() works fine (as Jan Hansen says above).

  • Nick Stakenburg

    Nick Stakenburg December 10th, 2008 @ 02:40 PM

    Ticket should probably be reopened, sounds like this bug slipped back in when 1.6.0.3 was reverted to an older release.

  • Juriy Zaytsev

    Juriy Zaytsev December 10th, 2008 @ 02:50 PM

    • State changed from “resolved” to “bug”
    • Milestone changed from 1.6.0.3 to 1.6.1
  • Jon Evans

    Jon Evans December 10th, 2008 @ 03:18 PM

    Not so sure about the fix now either - I just got "Error: 'tagName' is null or not an object" on this line:

    
    while ((element = element.parentNode) && element.tagName.toUpperCase() != 'HTML')
    

    I have a popup window (using window.js) with a scrolling inner div. I got the error to pop up by scrolling down really fast with a mouse wheel. I suspect a race condition as I can't reproduce it by scrolling at normal speed.

    What can I do from the script debugger window to find out what 'element' is? It's not null or undefined, but .style, .innerText and .innerHTML are all undefined.

    I wish IE had 'Firebug'.

  • Jon Evans

    Jon Evans December 10th, 2008 @ 03:33 PM

    OK, I managed to get a bit more info:

    
    element.nodeType
    11
    element.nodeName
    "#document-fragment"
    
  • apinstein

    apinstein December 26th, 2008 @ 10:06 PM

    • Tag changed from ie6, ie7, patched, tested to ie6, ie7, patched, tested, workaround

    I am experiencing this issue as well. It does seem to be triggered when the "destination" element is position: absolute. FWIW, this workaround works for my application, until it's fixed:

    // svEl is position: absolute

    svEl.setStyle( { position: 'relative' } );
    svEl.clonePosition(mapEl, { setWidth: false, setHeight: false, offsetTop: (mapElDim.height - svElDim.height), offsetLeft: (mapElDim.width - svElDim.width) });
    svEl.setStyle( { position: 'absolute' } );
    
    
  • David Rees

    David Rees January 30th, 2009 @ 02:17 AM

    I've been hit by this bug, too, as a Tapestry5 user. There is this bug filed there:

    http://issues.apache.org/jira/br...

    The patch to fix this issue there is different than all the patches listed here, as well as the one listed at http://dev.rubyonrails.org/ticke...>

    Any comments on what the best solution is to this issue?

  • Juriy Zaytsev

    Juriy Zaytsev February 23rd, 2009 @ 12:01 AM

    • Assigned user set to “Juriy Zaytsev”
  • Diego Perini

    Diego Perini March 15th, 2009 @ 03:21 AM

    Juriy, maybe a line like this is slightly better than the proposed one above.

    
    while ((element = element.parentNode).nodeType == 1)
    

    The advantage about checking the nodeType is that it should be faster then accessing a property and then uppercasing it.

    Upper/Lower case testing would be necessary for HTML, XHTML, XML compliance and should cover cases where different namespaced elements (thus different case tags) can be found in the same hosting document.

    With the above change we don't have to depend on some assumption about the element tag name case.

    Not sure this will cover the OP issue completely, if not it is still an applicable improvement.

  • Diego Perini

    Diego Perini March 15th, 2009 @ 05:58 PM

    If you expect the "document" being passed directly to the getOffsetParent() function there should be a similar check on nodeType == 1 before allowing the code to enter the loop and then maybe better be safe in the conditional itself:

    
    
    while ((element = element.parentNode) && element.nodeType == 1)
    
    

    The "parentNode" nodeType will always be 1 until the stepping up loop through parents reaches the "document" node which has a nodeType of 9. So it is used to break the loop before that exact point, ensuring also the "documentElement" node passes through the loop body.

  • Diego Perini

    Diego Perini March 16th, 2009 @ 03:20 PM

    I didn't know if you already had these links that seemed interesting for the resolution of this ticket, so:

    recent W3C working draft 20 Feb 08:

    http://www.w3.org/TR/cssom-view/...

    test case for the above rules.

    http://www.gtalbot.org/BrowserBu...

  • Juriy Zaytsev

    Juriy Zaytsev March 23rd, 2009 @ 04:48 AM

    • Milestone changed from 1.6.1 to 1.7
  • Jen Rawson

    Jen Rawson April 15th, 2009 @ 09:59 PM

    The root cause of this problem is in the try/catch statements in the IE wrapper functions for getOffsetParent(), positionedOffset(), and viewportOffset().

    Here:

    
      Element.Methods.getOffsetParent = Element.Methods.getOffsetParent.wrap(
        function(proceed, element) {
          element = $(element);
          try { element.offsetParent }
          catch(e) { return $(document.body) }
          var position = element.getStyle('position');
          ...
    

    and here:

    
      $w('positionedOffset viewportOffset').each(function(method) {
        Element.Methods[method] = Element.Methods[method].wrap(
          function(proceed, element) {
            element = $(element);
            try { element.offsetParent }
            catch(e) { return Element._returnOffset(0,0) }
            var position = element.getStyle('position');
            ...
    
    

    The code above is testing if element.offsetParent throws an error, which would occur if element was null. In the case where element is , element.offsetParent is null. Because no error is thrown, the code continues to execute.

    In both code snippets, the next line to execute is:

    
    var position = element.getStyle('position');
    

    In the IE getStyle() function, the following line throws an error because the element does not have a style property.

    
    var value = element.style[style];
    

    One solution would be to check if element.offsetParent is null in the IE wrapper functions.

    Here:

    
      Element.Methods.getOffsetParent = Element.Methods.getOffsetParent.wrap(
        function(proceed, element) {
          element = $(element);
          try { element.offsetParent }
          catch(e) { return $(document.body) }
          if(element.offsetParent == null) return $(document.body);
          var position = element.getStyle('position');
          ...
    

    and here:

    
      $w('positionedOffset viewportOffset').each(function(method) {
        Element.Methods[method] = Element.Methods[method].wrap(
          function(proceed, element) {
            element = $(element);
            try { element.offsetParent }
            catch(e) { return Element._returnOffset(0,0) }
            if(element.offsetParent == null) return Element._returnOffset(0,0);
            var position = element.getStyle('position');
            ...
    
    
  • Jen Rawson

    Jen Rawson April 15th, 2009 @ 10:04 PM

    Oops. There is a typo in my previous comment. I meant to say:

    The code above is testing if element.offsetParent throws an error, which would occur if element was null. In the case where element is html, element.offsetParent is null. Because no error is thrown, the code continues to execute.

  • Juriy Zaytsev

    Juriy Zaytsev April 15th, 2009 @ 10:14 PM

    My branch has an actual test for offsetParent throwing error, rather than forking based on Prototype.Browser.IE. There's also no try/catch, as it is never needed. Can you check if it fixes your issue?

    http://github.com/kangax/prototype/tree/master

  • Michael White

    Michael White April 16th, 2009 @ 06:42 AM

    Wow... I didn't expect to find this bug listed here but I'm glad I did.

    I spent a few hours tracking down the source of the error described and I came up with the following results:

    Line 2089 in the 1.6.0.3 release is the following:

    while ((element = element.parentNode) && element != document.body)

    it SHOULD be this:

    while ((element = element.parentNode) && element != document.body && element != document)

    For some reason IE 7 sometimes uses the HTML (document) object as the parentNode property of an element OTHER than the body element. Maybe someone knows why this happens - all I know is that the patched line above works. :D

    I noticed that the bug also exists in the new RC for 1.6.1

  • Michael White

    Michael White April 16th, 2009 @ 06:45 AM

    Arg... Sorry about my code not being formatted in the previous comment. Try this:

    Line 2089 in the 1.6.0.3 release should be changed from:

    @@@javascript while ((element = element.parentNode) && element != document.body)

    
    
    **to**
    
    @@@javascript
    while ((element = element.parentNode) && element != document.body && element != document)
    

    which just makes sure the current element is not the document object. Limited explanations about this are in my previous comment.

  • Michael White

    Michael White April 16th, 2009 @ 06:47 AM

    Ok folks... this thing REALLY needs a preview for us formatting newbies. Once more... WITH the spaces before the word "javascript" grrrr

    Line 2089 in the 1.6.0.3 release should be changed from:

    
    while ((element = element.parentNode) && element != document.body)
    

    to

    
    while ((element = element.parentNode) && element != document.body && element != document)
    

    which just makes sure the current element is not the document object. Limited explanations about this are in my previous comment.

  • Michael White

    Michael White April 16th, 2009 @ 07:10 AM

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

    Title edit change to correct the grammar.

    You can "affect" something.

    Something "effects" you.

    This bug "affects" several methods.

  • Michael White

    Michael White April 16th, 2009 @ 07:11 AM

    I meant something can have an "effect" on you. I really should proof-read more carefully... :(

  • Jen Rawson

    Jen Rawson April 16th, 2009 @ 07:33 PM

    Juriy- I'm not set up to use git and in my experience, installing, finding the necessary modules, etc. could be an all day project. If you can give me a quick way to test your branch (a prototype.js file) I'll give it a go. Thanks in advance!

  • Juriy Zaytsev
  • Jon Evans

    Jon Evans July 10th, 2009 @ 02:41 PM

    @Juiry

    Please can you produce a patch against the latest github master containing just your fix for this specific issue?

    It would be great if you could. I'm currently using 1.6.0.3 + Nick's patch above, and I'm looking to move to 1.6.1_rc3 + somebody's patch for this issue. I want 1.6.1_rc3 for the mouseenter / mouseleave event support.

  • Tobie Langel

    Tobie Langel July 24th, 2009 @ 12:46 PM

    • Tag changed from ie6, ie7, patched, tested, workaround to patched

    [not-tagged:"ie6" not-tagged:"ie7" bulk edit command]

  • T.J. Crowder

    T.J. Crowder November 16th, 2009 @ 04:50 PM

    • Assigned user cleared.

    [responsible:none bulk edit command]

  • Radoslav Stankov

    Radoslav Stankov March 1st, 2010 @ 08:56 PM

    • Assigned user set to “Andrew Dupont”
    • Tag changed from patched to needs:review, patched, section:dom
  • ClementsLaura30
  • asdasd
  • GitHub Robot

    GitHub Robot October 17th, 2010 @ 08:26 AM

    • State changed from “bug” to “resolved”

    (from [e4503ee9912be5abff287b88f7c27a9f43632fdd]) Handle cases where document or document.documentElement is passed into Element#getOffsetParent. Fixes IE errors with many layout/positioning methods. [#90 state:resolved] (Padraig Kennedy, Andrew Dupont) http://github.com/sstephenson/prototype/commit/e4503ee9912be5abff28...

  • poloralphlauren
  • chrisboa
  • ammya
  • amanda1216

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

Attachments

Referenced by

Pages