Bug 80097

Summary: [Forms] Make order of attribute/method in HTMLSelectElement.idl as same as specification
Product: WebKit Reporter: yosin
Component: FormsAssignee: yosin
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, haraken, ojan, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch 1
none
Patch 2 none

Description yosin 2012-03-01 22:10:05 PST
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
Comment 1 yosin 2012-03-01 22:17:26 PST
Created attachment 129816 [details]
Patch 1
Comment 2 Kentaro Hara 2012-03-01 22:20:28 PST
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
Comment 3 yosin 2012-03-01 22:45:21 PST
Do we care about differences between DOM Level 2 and WHATWG specification? These comments doesn't provide useful information other than history.
Comment 4 Kentaro Hara 2012-03-01 22:48:12 PST
(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 5 Kent Tamura 2012-03-01 22:52:45 PST
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.
Comment 6 yosin 2012-03-01 23:11:01 PST
Created attachment 129826 [details]
Patch 2
Comment 7 yosin 2012-03-01 23:15:11 PST
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 8 Kent Tamura 2012-03-02 01:01:17 PST
Comment on attachment 129826 [details]
Patch 2

ok
Comment 9 WebKit Review Bot 2012-03-02 05:24:52 PST
Comment on attachment 129826 [details]
Patch 2

Clearing flags on attachment: 129826

Committed r109562: <http://trac.webkit.org/changeset/109562>
Comment 10 WebKit Review Bot 2012-03-02 05:24:56 PST
All reviewed patches have been landed.  Closing bug.