Bug Tracker

Modify

Ticket #1742 (closed bug: fixed)

Opened 6 years ago

Last modified 6 years ago

remove() function crash

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

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

comment:1 Changed 6 years ago by brandon

  • need changed from Review to Test 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 6 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 6 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 6 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 6 years ago by brandon

  • Status changed from new to closed
  • Resolution set to fixed

Appears to be fixed in Rev [3790]

Please follow the  bug reporting guidlines and use  jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

View

Add a comment

Modify Ticket

Action
as closed
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.