RESOLVED FIXED 9867
REGRESSION: code that modifies form element in Firefox modifies form attribute in WebKit 
https://bugs.webkit.org/show_bug.cgi?id=9867
Summary REGRESSION: code that modifies form element in Firefox modifies form attribut...
Alexander Kellett
Reported 2006-07-12 02:04:11 PDT
Clicking on submit button in the attached testcase should result in going to this url:    /?action=Recalculate Unfortunately, in the current nightly it goes here:    /?action=
Attachments
reduced testcase (141 bytes, text/html)
2006-07-12 04:09 PDT, Alexander Kellett 		no flags
Details
further reduced testcase (195 bytes, text/html)
2006-07-12 09:50 PDT, Alexander Kellett 		no flags
Details
testcase patch (1.19 KB, patch)
2006-07-12 11:34 PDT, Alexander Kellett 		no flags
DetailsFormatted DiffDiff
lacking changelog, looking for code review (2.56 KB, patch)
2006-07-24 14:26 PDT, Alexander Kellett 		darin: review-
DetailsFormatted DiffDiff
binding changes, layouttests (6.45 KB, patch)
2006-07-26 22:37 PDT, Alexander Kellett 		darin: review+
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
David Kilzer (:ddkilzer)
Comment 1 2006-07-12 03:48:38 PDT
(In reply to comment #0) > Clicking on submit button in the attached testcase [...] You might want to attach the testcase. :)
Alexander Kellett
Comment 2 2006-07-12 04:09:07 PDT
Created attachment 9397 [details] reduced testcase
mitz
Comment 3 2006-07-12 07:54:42 PDT
I think the bug is specific to the name you chose for your hidden field: this.form.action is the form's action attribute ("/" in this case). You can access the action input as this.form.elements.action or this.form.elements["action"]. In Firefox the action input takes precedence over the action attribute.
Darin Adler
Comment 4 2006-07-12 08:57:19 PDT
We can reduce this further to a case that doesn't involve form submission or clicking on buttons at all.
Alexander Kellett
Comment 5 2006-07-12 09:50:49 PDT
Created attachment 9404 [details] further reduced testcase
Alexander Kellett
Comment 6 2006-07-12 11:34:06 PDT
Created attachment 9407 [details] testcase patch
Alice Liu
Comment 7 2006-07-14 16:37:57 PDT
<rdar://problem/4631743>
Eric Seidel (no email)
Comment 8 2006-07-24 02:22:07 PDT
As Alex correctly points out, this was likely caused by my moving of HTMLFormElement to be autogen'd: http://bugzilla.opendarwin.org/attachment.cgi?id=8529 I wasn't aware of this strange behavior at the time. The easy fix is to add a class attribute to the autogen system which allows name getters to take precedent over normal attributes in certain cases.
Eric Seidel (no email)
Comment 9 2006-07-24 02:24:56 PDT
I meant to reference the bug above: http://bugzilla.opendarwin.org/show_bug.cgi?id=9057
Alexander Kellett
Comment 10 2006-07-24 04:08:07 PDT
Need to know the result of: <body onload="foo()"/> <form action="/">    <input type=hidden name="action" value=""> </form> <script>    function foo() {       document.forms[0].action.value = "Passed";       HTMLFormElement.prototype["action"] = function() { return "foo"; };       alert(document.forms[0].action);    } </script> In Internet Explorer. Exception thrown in firefox.
mitz
Comment 11 2006-07-24 06:16:07 PDT
(In reply to comment #10) > Need to know the result of: > > <body onload="foo()"/> > <form action="/"> >    <input type=hidden name="action" value=""> > </form> > <script> >    function foo() { >       document.forms[0].action.value = "Passed"; >       HTMLFormElement.prototype["action"] = function() { return "foo"; }; >       alert(document.forms[0].action); >    } > </script> > > In Internet Explorer. Exception thrown in firefox. > Line: 8 Char: 1 Error: 'HTMLFormElement' is undefined Similar results in TOT: Can't find variable: HTMLFormElement
Alexander Kellett
Comment 12 2006-07-24 06:30:10 PDT
mitz: stupid classes all have different names depending on the browser. i've got a patch ready for this bug. posting tonight.
Alexander Kellett
Comment 13 2006-07-24 14:26:59 PDT
Created attachment 9656 [details] lacking changelog, looking for code review
Darin Adler
Comment 14 2006-07-24 20:54:16 PDT
Comment on attachment 9656 [details] lacking changelog, looking for code review Looks fine, although I might have suggested an alternate form of HasNameGetter instead of an entirely separate attribute. My only question is whether we want the name getter to override all the attributes or only some. If you add a change log and layout tests, then this would be a review+ I think.
Alexander Kellett
Comment 15 2006-07-24 21:02:27 PDT
From testing early on .elements is also overriden so I generalized a bit :). I'll test the other overrides tonight. Seperate attribute does sound like a better idea, but I had issues coming up with a good name, if anyone has good suggestions i'll update the patch.
Alexander Kellett
Comment 16 2006-07-26 22:37:53 PDT
Created attachment 9707 [details] binding changes, layouttests
Darin Adler
Comment 17 2006-07-27 08:34:41 PDT
Comment on attachment 9707 [details] binding changes, layouttests OK, r=me.
Darin Adler
Comment 18 2006-07-29 07:43:12 PDT
Comment on attachment 9707 [details] binding changes, layouttests I noticed a few minor things wrong with the layout test: 1) It doesn't say what it tests. 2) It should be a plain text test, but it's not. 3) Since it's not a plain text test, the patch needs to include checksum and png. I think I'll change it to a plain text test before landing it.
Darin Adler
Comment 19 2006-07-29 08:07:10 PDT
Alex already landed this! r15654
Note You need to log in before you can comment on or make changes to this bug.