Form field loses name if moved right after placement

The bug seems to occur in IE only.

This can be fixed by creating the input element with the name attribute included:

• fck_textfield.html

  old	new
  48	48	GetE('txtSize').value   = GetAttribute( oActiveEl, 'size' ) ;
  49	49	GetE('txtMax').value    = GetAttribute( oActiveEl, 'maxLength' ) ;
  50	50	GetE('txtType').value   = oActiveEl.type ;
  51
  52		GetE('txtType').disabled = true ;
  53	51	}
  54	52	else
  55	53	oActiveEl = null ;
  …	…
  72	70	return false ;
  73	71	}
  74	72
  	73	// IE doesn't allow easily to change properties of an existing object, so remove the old and create a new one.
  	74	if ( oActiveEl && oEditor.FCKBrowserInfo.IsIE )
  	75	{
  	76	oActiveEl.parentNode.removeChild( oActiveEl ) ;
  	77	oActiveEl = null ;
  	78	}
  	79
  75	80	if ( !oActiveEl )
  76	81	{
  77		oActiveEl = oEditor.FCK.EditorDocument.createElement( 'INPUT' ) ;
  78		oActiveEl.type = GetE('txtType').value ;
  	82	// #676, IE doesn't play nice with the name attribute.
  	83	if ( oEditor.FCKBrowserInfo.IsIE )
  	84	{
  	85	oActiveEl = oEditor.FCK.EditorDocument.createElement( '<INPUT type="' + GetE('txtType').value + '" name="' + GetE('txtName').value + '">' ) ;
  	86	}
  	87	else
  	88	{
  	89	oActiveEl = oEditor.FCK.EditorDocument.createElement( 'INPUT' ) ;
  	90	}
  79	91	oEditor.FCKUndo.SaveUndoStep() ;
  80	92	oActiveEl = oEditor.FCK.InsertElement( oActiveEl ) ;
  81	93	}
  82	94
  83	95	oActiveEl.name = GetE('txtName').value ;
  	96	oActiveEl.type = GetE('txtType').value ;
  	97
  84	98	SetAttribute( oActiveEl, 'value'        , GetE('txtValue').value ) ;
  85	99	SetAttribute( oActiveEl, 'size'         , GetE('txtSize').value ) ;
  86	100	SetAttribute( oActiveEl, 'maxlength', GetE('txtMax').value ) ;

If we care only about the moment when it's created then it enough to use just the

		// #676, IE doesn't play nice with the name attribute.
		if ( oEditor.FCKBrowserInfo.IsIE )
		{
			oActiveEl = oEditor.FCK.EditorDocument.createElement( '<INPUT type="' + GetE('txtType').value + '" name="' + GetE('txtName').value + '">' ) ;
		}
		else
		{
			oActiveEl = oEditor.FCK.EditorDocument.createElement( 'INPUT' ) ;
		}

part, but the fact is that after the element has been created, its name is edited and then moved, the name will revert back to its previous value, so the solution seems to be to recreate the element for IE everytime that it's edited.

The drawback is that any other properly not handled in this dialog will be lost, but on the other side it allows to change the type property after the element has been created (#738)

But I'm thinking now that instead of just destroying the old element we can hold it just a little, and then copy every property except the ones that the dialog allows editing, so unknown properties (for the dialog, like Class, Id or Style) will remain and we keep the bugfix.

That late idea seems to work, and this code fixes the problems for text inputs and textareas:

• common/fck_dialog_common.js

  old	new
  57	57	return ( oValue == null ? valueIfNull : oValue ) ;
  58	58	}
  59	59
  	60	// Utility function to create an element with a name attribute in IE, so it behaves properly when moved around
  	61	// It also allows to change the name or other special attributes in an existing node
  	62	function CreateNamedElement( oEditor, oOriginal, nodeName, oAttributes )
  	63	{
  	64	var oNewNode ;
  	65	// IE doesn't allow easily to change properties of an existing object, so remove the old and create a new one.
  	66	var oldNode = null ;
  	67	if ( oOriginal && oEditor.FCKBrowserInfo.IsIE )
  	68	{
  	69	oldNode = oOriginal ;
  	70	oOriginal = null ;
  	71	}
  	72
  	73	// If the node existed (and it's not IE), then we don't need to do anything here
  	74	if ( oOriginal )
  	75	return oOriginal ;
  	76
  	77	// #676, IE doesn't play nice with the name attribute.
  	78	if ( oEditor.FCKBrowserInfo.IsIE )
  	79	{
  	80	var sbHTML = [] ;
  	81	sbHTML.push( '<' + nodeName ) ;
  	82	for( var prop in oAttributes )
  	83	{
  	84	sbHTML.push( ' ' + prop + '="' + oAttributes[prop] + '"' ) ;
  	85	}
  	86	sbHTML.push( '>' ) ;
  	87	if ( !oEditor.FCKListsLib.EmptyElements[nodeName.toLowerCase()] )
  	88	sbHTML.push( '</' + nodeName + '>' ) ;
  	89
  	90	oNewNode = oEditor.FCK.EditorDocument.createElement( sbHTML.join('') ) ;
  	91	// Check if we are just changing the properties of an existing node: copy its properties
  	92	if ( oldNode )
  	93	{
  	94	CopyAttributes( oldNode, oNewNode ) ;
  	95	MoveContents( oldNode, oNewNode ) ;
  	96	oldNode.parentNode.removeChild( oldNode ) ;
  	97	oldNode = null ;
  	98	// Trick to refresh the selection object and avoid error in FCKDomRange.MoveToSelection due to the removed node
  	99	var oSel = oEditor.FCK.EditorDocument.selection ;
  	100	oSel.createRange() ; // Now oSel.type will be 'None' reflecting the real situation
  	101	}
  	102	oNewNode = oEditor.FCK.InsertElement( oNewNode ) ;
  	103	}
  	104	else
  	105	{
  	106	oNewNode = oEditor.FCK.InsertElement( nodeName ) ;
  	107	}
  	108
  	109	return oNewNode ;
  	110	}
  	111
  	112	// Copy all the attributes from one node to the other, kinda like a clone
  	113	function CopyAttributes( oSource, oDest )
  	114	{
  	115	var aAttributes = oSource.attributes ;
  	116
  	117	for ( var n = 0 ; n < aAttributes.length ; n++ )
  	118	{
  	119	var oAttribute = aAttributes[n] ;
  	120
  	121	if ( oAttribute.specified )
  	122	{
  	123	var sAttName = oAttribute.nodeName ;
  	124	// We can set the type only once, so do it with the proper value, not copying it.
  	125	if ( sAttName == 'type' )
  	126	continue ;
  	127
  	128	var sAttValue = oSource.getAttribute( sAttName, 2 ) ;
  	129	if ( sAttValue == null )
  	130	sAttValue = oAttribute.nodeValue ;
  	131
  	132	oDest.setAttribute( sAttName, sAttValue, 0 ) ;  // 0 : Case Insensitive
  	133	}
  	134	}
  	135	// The style:
  	136	oDest.style.cssText = oSource.style.cssText ;
  	137	}
  	138
  	139	// Move the contents from one node to the other
  	140	function MoveContents( oSource, oDest )
  	141	{
  	142	while ( oSource.firstChild )
  	143	{
  	144	var oNode = oSource.removeChild( oSource.firstChild ) ;
  	145	oDest.appendChild( oNode ) ;
  	146	}
  	147	}
  	148
  	149
  60	150	var KeyIdentifierMap =
  61	151	{
  62	152	End             : 35,

• fck_textarea.html

  old	new
  56	56	function Ok()
  57	57	{
  58	58	oEditor.FCKUndo.SaveUndoStep() ;
  59
  60		if ( !oActiveEl )
  61		{
  62		oActiveEl = oEditor.FCK.InsertElement( 'textarea' ) ;
  63		}
  64	59
  	60	oActiveEl = CreateNamedElement( oEditor, oActiveEl, 'TEXTAREA', {name: GetE('txtName').value} ) ;
  	61
  65	62	oActiveEl.name = GetE('txtName').value ;
  66	63	SetAttribute( oActiveEl, 'cols', GetE('txtCols').value ) ;
  67	64	SetAttribute( oActiveEl, 'rows', GetE('txtRows').value ) ;

• fck_textfield.html

  old	new
  48	48	GetE('txtSize').value   = GetAttribute( oActiveEl, 'size' ) ;
  49	49	GetE('txtMax').value    = GetAttribute( oActiveEl, 'maxLength' ) ;
  50	50	GetE('txtType').value   = oActiveEl.type ;
  51
  52		GetE('txtType').disabled = true ;
  53	51	}
  54	52	else
  55	53	oActiveEl = null ;
  …	…
  72	70	return false ;
  73	71	}
  74	72
  75		if ( !oActiveEl )
  76		{
  77		oActiveEl = oEditor.FCK.EditorDocument.createElement( 'INPUT' ) ;
  78		oActiveEl.type = GetE('txtType').value ;
  79		oEditor.FCKUndo.SaveUndoStep() ;
  80		oActiveEl = oEditor.FCK.InsertElement( oActiveEl ) ;
  81		}
  	73	oEditor.FCKUndo.SaveUndoStep() ;
  82	74
  	75	oActiveEl = CreateNamedElement( oEditor, oActiveEl, 'INPUT', {name: GetE('txtName').value, type: GetE('txtType').value } ) ;
  	76
  83	77	oActiveEl.name = GetE('txtName').value ;
  	78	oActiveEl.type = GetE('txtType').value ;
  	79
  84	80	SetAttribute( oActiveEl, 'value'        , GetE('txtValue').value ) ;
  85	81	SetAttribute( oActiveEl, 'size'         , GetE('txtSize').value ) ;
  86	82	SetAttribute( oActiveEl, 'maxlength', GetE('txtMax').value ) ;

The code for the rest of the elements can be written just as soon as this can land, it's just a matter of doing the same changes in their dialogs.

Proposed SVN patch

This patch takes care of all the form elements, the code also takes care of #738

I must apply the ideas from #738 to create a new element only when it's really needed and to try to select better the newly created element.

New patch

I've updated the patch so now it recreates the element only if one of the special attributes has changed, and after closing the dialog the element is properly selected, not a text selection like it did happen previously. Both ideas from #738.

The method used makes sense and it worked great in my tests. So I'm giving it a Review+.

Fixed with [1504]

I've realized now that the MoveContents function added to fck_dialog_common.js is the same idea than FCKDomTools.MoveChildren

I think that it would be better to use the FCKDomTools.MoveChildren in order to avoid duplication (I thought about adding all the functionality to FCKDomTools, but as it isn't used in the core, then it would increase the base file size without any special reason)

I'll prepare a new patch.

Reuse the existing FCKDomTools.MoveChildren

Fixed with [1601]