Bug Tracker

Opened 13 years ago

Closed 12 years ago

#1742 closed bug (fixed)

remove() function crash

Reported by: alexo Owned by:
Priority: major Milestone: 1.2.2
Component: core Version: 1.2.1
Keywords: Cc:
Blocked by: Blocking:

Description

This is the current implementation of the remove function.

  remove: function(a){
    if ( !a || jQuery.filter( a, [this] ).r.length ) {
      jQuery.removeData( this );
      this.parentNode.removeChild( this );
    }
  }

There is a case when I am trying to remove an element which parentNode is null. A check should be added to remove function:

  remove: function(a){
    if ( !a || jQuery.filter( a, [this] ).r.length ) {
      jQuery.removeData( this );
      if (this.parentNode) {
        this.parentNode.removeChild( this );
      }
    }
  }

Thank you! Alex

Change History (5)

comment:1 Changed 13 years ago by brandon

need: ReviewTest Case

Could you please provide a test case for this? I do not know of a situation where you would call remove on an element that doesn't have a parentNode.

comment:2 Changed 13 years ago by alexo

It's not so easy. But it happens when working with ajax and the DOM is being replaced, while DOM elements references point to orphan elements.

comment:3 Changed 12 years ago by pyrolupus

Encountered this same problem with an Ajax form and Impromptu dialogs. Cut away all but the bare necessities for a demo page, then found this ticket. Unsurprisingly, the solution I came up with is exactly what Alex suggested. Demo page:

http://pyrolupus.com/demo/jqremovebug.php

--Pyro

comment:4 Changed 12 years ago by davidserduke

The recent example is a situation where it is calling remove() twice. The demo has a jQuery object with two elements and calls fadeOut on the object with a callback. That callback calls remove on a closure version of the object with both elements. But the callback is called twice, once for each of the elements. It can be fixed by calling remove on 'this' instead.

Still, I think the proposed fix could be a good idea. jQuery tends to be robust enough to handle situations like:

$("div").remove().remove();

without throwing errors. Plus there could still be situations like the original bug poster mentioned that this would fix.

comment:5 Changed 12 years ago by brandon

Resolution: fixed
Status: newclosed

Appears to be fixed in Rev [3790]

Note: See TracTickets for help on using tickets.