For ease of maintainability, attributes and methods declaration order should be as same as specification. http://www.whatwg.org/specs/web-apps/current-work/multipage/the-button-element.html#the-select-element Similar work was done for input element: https://bugs.webkit.org/show_bug.cgi?id=79622
Created attachment 129816 [details] Patch 1
Comment on attachment 129816 [details] Patch 1 View in context: https://bugs.webkit.org/attachment.cgi?id=129816&action=review > Source/WebCore/html/HTMLSelectElement.idl:-31 > - // Modified in DOM Level 2: Why did you remove this comment? > Source/WebCore/html/HTMLSelectElement.idl:-45 > - // Modified in DOM Level 2: Ditto > Source/WebCore/html/HTMLSelectElement.idl:-67 > - // These methods are not in DOM Level 2 IDL, but are mentioned in the standard: > - // "The contained options can be directly accessed through the select element as a collection." Ditto
Do we care about differences between DOM Level 2 and WHATWG specification? These comments doesn't provide useful information other than history.
(In reply to comment #3) > Do we care about differences between DOM Level 2 and WHATWG specification? These comments doesn't provide useful information other than history. tkent: WDYT? Such comments remain in other IDL files though.
Comment on attachment 129816 [details] Patch 1 View in context: https://bugs.webkit.org/attachment.cgi?id=129816&action=review >> Source/WebCore/html/HTMLSelectElement.idl:-31 >> - // Modified in DOM Level 2: > > Why did you remove this comment? This comment explains why we have #if defined(LANGUAGE_OBJECTIVE_C)... So, we shouldn't remove it. >> Source/WebCore/html/HTMLSelectElement.idl:-45 >> - // Modified in DOM Level 2: > > Ditto This comment looks non-sense. ok to remove it. >> Source/WebCore/html/HTMLSelectElement.idl:-67 >> - // "The contained options can be directly accessed through the select element as a collection." > > Ditto It's ok to remove this comment because the WHATWG spec has them.
Created attachment 129826 [details] Patch 2
It seems author who wrote "// Modified in DOM Level 2:" that she simply copy and paste from DOM Level 2 specification. http://www.w3.org/TR/2003/REC-DOM-Level-2-HTML-20030109/html.html#ID-94282980 interface HTMLSelectElement : HTMLElement { readonly attribute DOMString type; attribute long selectedIndex; attribute DOMString value; // Modified in DOM Level 2: attribute unsigned long length; // raises(DOMException) on setting "// Modified in DOM Level 2" doesn't tell me reason of why there is #ifdef here. I put explanatory comment instead of just "Modified in DOM Level2."
Comment on attachment 129826 [details] Patch 2 ok
Comment on attachment 129826 [details] Patch 2 Clearing flags on attachment: 129826 Committed r109562: <http://trac.webkit.org/changeset/109562>
All reviewed patches have been landed. Closing bug.