121726 - The order of the form elements changed when the name property of the form element is setted by using JavaScript.

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Andrew Brygin]

Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.7+) Gecko/20020122

When we change the name property of the form element by using JavaScript then 
the order of the form elements is changed too. It mkaes impossible to adderss 
the elements of form by the "some_form.elements[i]" way.

For example if we have JavaScript code like this:
 some_form.elements[0].name = new_name;
 some_form.elements[0].value = new_value;

then actualy we set new_value to some another element of the form instead 
renamed element.

Note that Netsacpe 4.xx does not change order of elements after renaming.

This is not a windows scecific bug, Mozilla have described behavior on Solaris 
platform too.

Comment 1

[Phil Schwartau]

Browser, not engine. Reassigning to DOM Level 0

[e-mail] : note, you can attach a reduced testcase bugs
(use the "Create a New Attachment" link above). Normally we need either
a testcase or a URL to go to; otherwise, we can't debug the problem.

However, I believe this is a duplicate of another bug; will post
that in a minute...

Comment 2

[Phil Schwartau]

This observation was made in bug 105398:

------- Additional Comment #1 From Boris Zbarsky 2001-10-18 07:39 -------

The real bug is that the form.elements array is sorted incorrectly.  In
particular, the sort order _changes_ when you change an element's name.

See also bug 49951.

--------------------------------------------------------------------------


Note that bug 105398 was then duped against bug 49951,
"form.elements[] ordered differently than 4.x"

So, I will dupe this bug, too. If anyone disagrees, please 
reopen this bug; thanks - 

*** This bug has been marked as a duplicate of 49951 ***

Comment 3

[Andrew Brygin]

Attachment: Testcase for described problem

Comment 4

[Phil Schwartau]

Andrew: thanks! If you have a chance, though, could you attach your
testcase to bug 49951? That's where the work is going to be done.

You are cc'ed on that bug, so you'll be able to follow its progress. 

Comment 5

[Phil Schwartau]

[e-mail]: I noticed you cc'd yourself on this bug. 
But this bug has been marked a duplicate of bug 49951.

So you might want to cc yourself there - that's where all the
work will be done, and where all the updates will come from. 

Comment 6

[Pete Zha]

Attachment: Remove some code that operat element vector during set attribute

We should not remove element from element array when name, id or type changed.
I don't think this bug is dup of 49951, 49951 is another proble when parser
form element into vector.
Please verify. thanks!

Comment 7

[Denis Antrushin]

I don't believe this is dup of 49951 - seems as different problems to me

Comment 8

[Pete Zha]

This patch I supplied seems have some problem...
If the element's name set from none to a not none value, we should remove it 
from the noNameElementTable.
But my patch break this logic by simply remove those code.
Later I will supply another patch to resolve this problem.
Could someone give me some suggestion? Really thanks!

BTW:Reopen this bug.

Comment 9

[Denis Antrushin]

I think that mNoNameLookupTable is used for form elements which is not supposed
to get into form.elements array, isn't it?

Comment 10

[Sergi]

Reopened due to Denis's request.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Over to jkeiser; this is his ballpark.

Comment 12

[Pete Zha]

Denis:
mNoNameLookupTable is a hastable that store element that out of following 
element type:
  case NS_FORM_BUTTON_BUTTON :
  case NS_FORM_BUTTON_RESET :
  case NS_FORM_BUTTON_SUBMIT :
  case NS_FORM_INPUT_BUTTON :
  case NS_FORM_INPUT_CHECKBOX :
  case NS_FORM_INPUT_FILE :
  case NS_FORM_INPUT_HIDDEN :
  case NS_FORM_INPUT_RESET :
  case NS_FORM_INPUT_PASSWORD :
  case NS_FORM_INPUT_RADIO :
  case NS_FORM_INPUT_SUBMIT :
  case NS_FORM_INPUT_TEXT :
  case NS_FORM_SELECT :
  case NS_FORM_TEXTAREA :
  case NS_FORM_FIELDSET :

Comment 13

[Johnny Stenback (:jst)]

mNoNameLookupTable holds form controls that are of types *not* listed above.

Comment 14

[Pete Zha]

Attachment: Patch

Remove AddElement/RemoveElement code when change element's name.
But leave type change code.

jkeiser: Thanks for explaining so much detail tomorrow. But I have one more
problem following:
If javascript change element's type from text to hidden or anything else in the
namedFormControl types. We should not remove it from .elements right? But if we
do the removeElement/addElement the order of form's elements will be changed...

Hope you have idea on this.

Comment 15

[John Keiser (jkeiser)]

Changing type from text to hidden, we should not really remove it and re-add it,
but that's going to be a bigger job than fixing name and id.  You really have to
detect the from-type and to-type and see if they both should stay in
form.elements (implying no change is needed).  The reason for this problem is,
when you change type from text to image, it *should* remove from .elements, and
vice versa--it should add into elements when type changes from image to text.

Plans are in the works to make AddElement() work proper-like.  I think we should
avoid changing the type case at the moment.  If you want to work on those plans,
in fact, I can give you gobs of relevant code and point you in the right
direction :)

Let me know what you want to do; the patch you submitted looks good, even works
on Bugzilla, the Ultimate Forms Testing Ground.

Those interested in this bug may also be interested in bug 123302.  Or not.

Comment 16

[Pete Zha]

Anyway, the patch can fix this bug right? Leave type change issue(It's not in 
the spec).
So, please review it and make it check in if possible. Thanks.

Comment 17

[John Keiser (jkeiser)]

Comment on attachment 67711 [details] [diff] [review]
Patch

r=jkeiser

Comment 18

[Johnny Stenback (:jst)]

Comment on attachment 67711 [details] [diff] [review]
Patch

sr=jst, but the empty lines above the lines that were removed should be taken
out too.

Comment 19

[Johnny Stenback (:jst)]

Thanks Pete for the fix, I just checked it in.

FIXED.

Comment 20

[Pete Zha]

->myself for tracking