Bug 9867

Summary:

REGRESSION: code that modifies form element in Firefox modifies form attribute in WebKit

Product:

WebKit

Reporter:

Alexander Kellett <a>

Component:

DOM

Assignee:

Alexander Kellett <a>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

darin, ddkilzer, ggaren, mitz, mjs

Priority:

Keywords:

HasReduction, InRadar, Regression

Version:

420+

Hardware:

Mac

OS:

OS X 10.4

Attachments:

Description	Flags
reduced testcase	none
further reduced testcase	none
testcase patch	none
lacking changelog, looking for code review	darin: review-
binding changes, layouttests	darin: review+

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	Details Formatted Diff Diff
lacking changelog, looking for code review (2.56 KB, patch) 2006-07-24 14:26 PDT, Alexander Kellett	darin: review-	Details Formatted Diff Diff
binding changes, layouttests (6.45 KB, patch) 2006-07-26 22:37 PDT, Alexander Kellett	darin: review+	Details Formatted Diff Diff
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.