[Webkit-unassigned] [Bug 49240] Implement formaction, formenctype, formmethod and formtarget attributes for the input tag

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=49240


Kent Tamura <tkent at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #73579|review?                     |review-
               Flag|                            |




--- Comment #8 from Kent Tamura <tkent at chromium.org>  2010-11-10 22:25:17 PST ---
(From update of attachment 73579)
View in context: https://bugs.webkit.org/attachment.cgi?id=73579&action=review

> LayoutTests/fast/forms/formaction-attribute.html:12
> +if (window.layoutTestController)
> +    layoutTestController.dumpAsText();

You dont need to do this.  js-test-pre.js does it.

> LayoutTests/fast/forms/formaction-attribute.html:14
> +doneAction = false;

Prepend "var ".

> LayoutTests/fast/forms/formaction-attribute.html:29
> +<script>

Why does this HTML have two <script>s?
I think you can merge them into one.

> LayoutTests/fast/forms/formaction-attribute.html:32
> +var x = document.getElementsByTagName('input')[0];

"x" is not a good name.

> LayoutTests/fast/forms/formaction-attribute.html:36
> +successfullyParsed = true;

Prepend "var ".

> LayoutTests/fast/forms/formmethod-attribute-expected.txt:1
> +PASS The formmethod attribute was successfully used

Please add a test description.  You can use description() defined by js-test-pre.js

> LayoutTests/fast/forms/formmethod-attribute.html:14
> +<script>

add description() call please.

> LayoutTests/fast/forms/formmethod-attribute.html:16
> +    layoutTestController.dumpAsText();

You don't need to call dumpAsText() because of js-test-pre.js.

> LayoutTests/fast/forms/formmethod-attribute.html:35
> +successfullyParsed = true;

add "var ".

> LayoutTests/fast/forms/formtarget-attribute.html:7
> +<script>

You can merge this <script> block into the <script> below.

> LayoutTests/fast/forms/formtarget-attribute.html:12
> +        layoutTestController.dumpAsText();

You don't need to call dumpAsText() because of js-test-pre.js.

> WebCore/ChangeLog:19
> +        (WebCore::HTMLFormControlElement::formAction):
> +        (WebCore::HTMLFormControlElement::formEnctype):
> +        (WebCore::HTMLFormControlElement::setFormEnctype):
> +        (WebCore::HTMLFormControlElement::formMethod):
> +        (WebCore::HTMLFormControlElement::formTarget):

Please update the ChangeLog.  This block refers to non-existent functions.

> WebCore/html/HTMLFormControlElement.cpp:87
> +String HTMLFormControlElement::formEnctype() const

Why didn't you remove formEnctype() and setFormEnctype()?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.