Bug Tracker

Opened 13 years ago

Closed 13 years ago

#910 closed bug (fixed)

DOM manipulations with FORM elements fail

Reported by: Renato Owned by:
Priority: major Milestone:
Component: core Version: 1.1
Keywords: Cc:
Blocked by: Blocking:

Description

If you execute this snippet of code, you see the form is not appended, only its children

$("form:first").clone().
appendTo("body");

The problem is in the end of jQuery.clean function at line 498:

if ( arg[0] == undefined )
	r.push( arg );
else
	r = jQuery.merge( r, arg );

In fact, the form element can be used as an array to inspect its inputs, so instead of just pushing the form element, jQuery execute the "else" merging the content of the form element and returning its inputs.

Changing to this code, solves the problem:

if ( arg[0] == undefined || jQuery.nodeName(arg,"form") )
	r.push( arg );
else
	r = jQuery.merge( r, arg );

Attachments (4)

new_test.diff (1.1 KB) - added by Renato 13 years ago.
a test for the bug
new_test.2.diff (1.1 KB) - added by Renato 13 years ago.
a test for the bug
new_test.3.diff (1.1 KB) - added by Renato 13 years ago.
the test
clean.diff (520 bytes) - added by brandon 13 years ago.
patch

Download all attachments as: .zip

Change History (8)

Changed 13 years ago by Renato

Attachment: new_test.diff added

a test for the bug

Changed 13 years ago by Renato

Attachment: new_test.2.diff added

a test for the bug

Changed 13 years ago by Renato

Attachment: new_test.3.diff added

the test

comment:1 Changed 13 years ago by Renato

Here is the patch with the test code to be added to the test suite

Index: coreTest.js
===================================================================
--- coreTest.js	(revisione 1258)
+++ coreTest.js	(copia locale)
@@ -204,7 +204,7 @@
 });
 
 test("append(String|Element|Array<Element>|jQuery)", function() {
-	expect(9);
+	expect(10);
 	var defaultText = 'Try them out:'
 	var result = $('#first').append('<b>buga</b>');
 	ok( result.text() == defaultText + 'buga', 'Check if text appending works' );
@@ -233,6 +233,10 @@
 	ok( $("#sap").append([]), "Check for appending an empty array." );
 	ok( $("#sap").append(""), "Check for appending an empty string." );
 	ok( $("#sap").append(document.getElementsByTagName("foo")), "Check for appending an empty nodelist." );
+	
+	reset();
+	$("#sap").append(document.getElementById('form'));
+	ok( $("#sap>form").size() == 1, "Check for appending a form" );
 });
 
 test("appendTo(String|Element|Array&lt;Element&gt;|jQuery)", function() {
@@ -678,4 +682,4 @@
 	    //console.log( close[0]); // it's the <a> and not a <span> element
 	    return false;
 	}).click();
-});
 Nessun a-capo alla fine del file
+});

comment:2 Changed 13 years ago by brandon

Resolution: fixed
Status: newclosed

Fixed in revision 1294.

Thank you for the patch+test.

comment:3 Changed 13 years ago by brandon

Resolution: fixed
Status: closedreopened

Reopening because the fix is causing Safari to crash.

Changed 13 years ago by brandon

Attachment: clean.diff added

patch

comment:4 Changed 13 years ago by brandon

Resolution: fixed
Status: reopenedclosed

Fixed in Revision 1422.

Note: See TracTickets for help on using tickets.