[webkit-reviews] review denied: [Bug 65542] Need support for dirname attribute : [Attachment 111386] Updated patch

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied Rakesh <rakesh.kn at motorola.com>'s
request for review:
Bug 65542: Need support for dirname attribute
https://bugs.webkit.org/show_bug.cgi?id=65542

Attachment 111386: Updated patch
https://bugs.webkit.org/attachment.cgi?id=111386&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
r- because of test coverage.


View in context: https://bugs.webkit.org/attachment.cgi?id=111386&action=review


> Source/WebCore/html/HTMLTextFormControlElement.cpp:584
> +    TextDirection textDirection =
directionalityIfhasDirAutoAttribute(isAuto);
> +    if (isAuto)
> +	   dir = String(textDirection == RTL ? "rtl" : "ltr");
> +    else if (hasAttribute(dirAttr))
> +	   dir = getAttribute(dirAttr).string();
> +    else
> +	   dir = String("ltr");

I don't know this is a right code to detect direction of the value.
Does any WebKit RTL expert have comment on this?

> Source/WebCore/html/HTMLTextFormControlElement.h:82
> +    String dirNameAttributeFormData() const;

I think the function name is not reasonable.  How about formDataDirection(), or
directionForFormData()?

> LayoutTests/fast/forms/submit-form-with-dirname-attribute.html:16
> +<form action="#action" method="GET" name="f">
> +    <p><label>Comment: <input type=text name="comment" dirname="comment.dir"
required></label></p>
> +    <p><button name="mode" type=submit value="add">Post Comment</button></p>

> +</form>

The test coverage is too low.  We need to test
* <input> and <textarea>
* The target element has no dir attribute, dir=rtl, dir=ltr, dir=invalid.
* The target element has no dir attribute but an ancestor element has dir=
attribute.
* The target element has RTL value, and LTR value with/without dir= attribtue.