"wrap" attribute is newly added into the "textarea" element. (http://www.w3.org/TR/html5/the-button-element.html#attr-textarea-wrap) This attribute is successfully working on WebKit, but it cannot be accessed by JavaScript. This attrubute can be accessed by JavaScript on other browsers, such as FireFox and Opera. That's because this attribute is not inserted in HTMLTextAreaElement.idl file in WebKit. (I don't know this is intentional or not.) I think "wrap" attribute should be included in HTMLTextAreaElement.idl file, because that is specified in the spec. document. (http://www.w3.org/TR/html5/the-button-element.html#the-textarea-element)
Created attachment 108332 [details] Patch Added wrap attribute to HTMLTextAreaElement.idl
Comment on attachment 108332 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=108332&action=review > Source/WebCore/html/HTMLTextAreaElement.idl:40 > + attribute [Reflect] DOMString wrap; Please insert this line at the next to "row" attribute. We'd like to follow the IDL in the specification as possible. http://www.whatwg.org/specs/web-apps/current-work/multipage/the-button-element.html#the-textarea-element Also [Reflect] is wrong in this case. http://www.whatwg.org/specs/web-apps/current-work/multipage/the-button-element.html#attr-textarea-wrap If the wrap HTML attribute is missing, we need to return "soft" for the wrap IDL attribute. > LayoutTests/fast/forms/textarea-wrap-attribute.html:6 > +<textarea wrap="hard" cols="20"> This is a simple text-area1 </textarea> > +<textarea wrap="soft"> This is a simple text-area2 </textarea> The test coverage is too low. - wrap HTML attribtue is missing - wrap HTML attribute has an invalid value - setting values to wrap IDL attribute - ... > LayoutTests/fast/forms/textarea-wrap-attribute.html:22 > +logResult("Text Area with hard wrap","hard", textArea.wrap); You had better load fast/js/resources/js-test-pre.js, and write shouldBe('textArea.wrap', '"hard"');
(In reply to comment #2) Thanks Kent for the review comments. > http://www.whatwg.org/specs/web-apps/current-work/multipage/the-button-element.html#the-textarea-element > > Also [Reflect] is wrong in this case. > http://www.whatwg.org/specs/web-apps/current-work/multipage/the-button-element.html#attr-textarea-wrap I actually confused here because http://www.whatwg.org/specs/web-apps/current-work/multipage/the-button-element.html#dom-textarea-wrap this links says "The cols, placeholder, required, rows, and wrap attributes must reflect the respective content attributes of the same name." Could you please comfirm that sure we don't want wrapAttr to be [Reflect] in IDL. Other review comment I will incorporate with next patch once above thing is clear.
(In reply to comment #3) > (In reply to comment #2) > > Thanks Kent for the review comments. > > > http://www.whatwg.org/specs/web-apps/current-work/multipage/the-button-element.html#the-textarea-element > > > > Also [Reflect] is wrong in this case. > > http://www.whatwg.org/specs/web-apps/current-work/multipage/the-button-element.html#attr-textarea-wrap > > I actually confused here because http://www.whatwg.org/specs/web-apps/current-work/multipage/the-button-element.html#dom-textarea-wrap > this links says "The cols, placeholder, required, rows, and wrap attributes must reflect the respective content attributes of the same name." > Could you please comfirm that sure we don't want wrapAttr to be [Reflect] in IDL. > > Other review comment I will incorporate with next patch once above thing is clear. Oh, I was wrong. - "wrap" HTML attribute is an enumerated attribute. - "wrap" IDL attribute reflects the HTML attribute, and not "limited to only known values". So [Reflect] is correct. I'm sorry for my confusion. http://www.whatwg.org/specs/web-apps/current-work/multipage/urls.html#limited-to-only-known-values
Thanks Kent for clearing my doubt. But here is another problem. If we say wrapAttr as [Reflect] in idl, I have test case where I specify wrap attr but no keyvalue like <textarea wrap=> This is a simple text-area2 </textarea> So in script if I do something like var textArea = document.getElementsByTagName('textarea')[0]; alert(textArea.wrap); As per spec ( section 4.10.13 The textarea element) it should return me SoftWrap but it returns nothing. But as wrapAttr is enumerated attribute so as per this (2.8.1 Reflecting content attributes in IDL attributes) http://www.whatwg.org/specs/web-apps/current-work/multipage/urls.html#limited-to-only-known-values this spec it should return "empty string if the attribute is in a state that has no associated keyword value" which is true in this case as there is no way to return default value. Both these are contradicting with each other. I think wrap attribute shouldn't be [Reflect] attribute and should have get/set methods for JS. Kent/Ian Hickson Could you please guide me if I am wrong.
Per spec, HTMLTextAreaElement.wrap reflects the wrap="" attribute. It is not "limited to only known values", and it is a DOMString attribute, which means it just returns the content attribute's value directly. If the markup is <textarea wrap=> then the value of the "wrap" content attribute is the empty string. i.e. Kent in comment 4 is correct. HTH.
Created attachment 108634 [details] Updated Patch Please find the attached patch as per the review comments.
Comment on attachment 108634 [details] Updated Patch View in context: https://bugs.webkit.org/attachment.cgi?id=108634&action=review r- because of test style. > Source/WebCore/html/HTMLTextAreaElement.idl:4 > + * Portions Copyright (C) 2011 Motorola Mobility, Inc. All rights reserved. You don't need to add "Portions". > LayoutTests/fast/forms/textarea-wrap-attribute.html:10 > +<script src="script-tests/textarea-wrap-attribute.js"></script> Do not add a separated .js file. This way is obsolete. Please put the content of textarea-wrap-attribute.js into here, and remove textarea-wrap-attribute.js. > LayoutTests/fast/forms/script-tests/textarea-wrap-attribute.js:10 > +//If wrap attribute present. This comment (and the followings) is useful information. Please print it to the test result. You can print it by debug('Check if wrap attribute present').
Created attachment 108811 [details] Patch > > Source/WebCore/html/HTMLTextAreaElement.idl:4 > > + * Portions Copyright (C) 2011 Motorola Mobility, Inc. All rights reserved. > You don't need to add "Portions". Removed portions. > > LayoutTests/fast/forms/textarea-wrap-attribute.html:10 > Do not add a separated .js file. This way is obsolete. removed textarea-wrap-attribute.js, added the code to textarea-wrap-attribute.html. > > LayoutTests/fast/forms/script-tests/textarea-wrap-attribute.js:10 > > +//If wrap attribute present. > This comment (and the followings) is useful information. Please print it to the test result. You can print it by debug('Check if wrap attribute present'). Added the debug statement to get more information on the test case.
Comment on attachment 108811 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=108811&action=review > LayoutTests/fast/forms/textarea-wrap-attribute.html:17 > +//If wrap attribute is not specified it sould be empty String. The comment is redundant. Please remove it. > LayoutTests/fast/forms/textarea-wrap-attribute.html:21 > +//Check if it sets warpAttr as hard ditto. > LayoutTests/fast/forms/textarea-wrap-attribute.html:22 > +debug('<br>Check if it sets warpAttr value hard, should return hard.'); We prefer using no HTML tags for debug(). Please write: debug(''); debug('Check if ... > LayoutTests/fast/forms/textarea-wrap-attribute.html:27 > +//Check if it sets warpAttr as soft > +debug('<br>Check if it sets warpAttr value as soft, should return soft.'); ditto. > LayoutTests/fast/forms/textarea-wrap-attribute.html:32 > +//If attribute present but no keyVal. > +debug('<br>Check if warpAttr present but no keyVal specified, should return empty String.'); ditto. > LayoutTests/fast/forms/textarea-wrap-attribute.html:37 > +//If invalid attribute is set. > +debug('<br>Check if it sets warpAttr invaild value, should return foo.'); ditto.
Created attachment 108814 [details] Updated Patch Updated patch as per review comments.
Comment on attachment 108814 [details] Updated Patch ok.
Comment on attachment 108814 [details] Updated Patch Clearing flags on attachment: 108814 Committed r96096: <http://trac.webkit.org/changeset/96096>
All reviewed patches have been landed. Closing bug.