Bug 28769 – The type of HTMLInputElement::list should follow the standard

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Kent Tamura]

In bug#27756, Sam wrote:
| > +#if defined(ENABLE_DATALIST) && ENABLE_DATALIST
| > +                 // The type of the list is HTMLElement according to the standard.
| > +                 // We intentionally use HTMLDataListElement for it because our implementation
| > +                 // always returns an HTMLDataListElement instance.
| > +        readonly attribute HTMLDataListElement list;
| > +#endif
| 
| It is not clear to me why the spec has this returning an Element, but I would
| rather we match it.  Except for extended attributes, I would really like to
| keep these IDLs as close to the specs word as possible.

[Kent Tamura]

Created attachment 38664 [details]
Proposed patch

[Darin Adler]

Comment on attachment 38664 [details]
Proposed patch

It seems OK to change the .idl file, but do we really have to change the .h and .cpp files? Having a more-specific type can allow simpler and slightly more efficient code internally, and should do no harm.

Let me put it differently. What visible change does this make. If it has no visible effect on web pages or WebKit clients, then I think we should minimize the change.

[Eric Seidel (no email)]

Comment on attachment 38664 [details]
Proposed patch

I agree with Darin.  This "bug" occurred because I asked Kent in original review comments to change the type from HTMLElement* to HTMLDataListElement* to make internal clients easier.  I don't really care one way or the other re: the idl, but if we'd like to be sure to match the spec exactly, that's fine too.

Basically, keep the idl change, revert the rest would be my recommendation.

[Kent Tamura]

Ok.
But how to resolve the following build errors in that case?

DOMHTMLInputElement.mm: In function ‘DOMHTMLElement* -[DOMHTMLInputElement list](DOMHTMLInputElement*, objc_selector*)’:
DOMHTMLInputElement.mm:175: error: no matching function for call to ‘kit(WebCore::HTMLDataListElement*)’
JSHTMLInputElement.cpp: In function ‘JSC::JSValue WebCore::jsHTMLInputElementList(JSC::ExecState*, const JSC::Identifier&, const JSC::PropertySlot&)’:
JSHTMLInputElement.cpp:297: error: no matching function for call to ‘toJS(JSC::ExecState*&, WebCore::JSDOMGlobalObject*, WebCore::HTMLDataListElement*)’

[Kent Tamura]

Created attachment 38797 [details]
Proposed patch (rev.2)

- add HTMLInputElement::dataList()
- make datalist-related methods const

[Kent Tamura]

Created attachment 38798 [details]
Proposed patch (rev.3)

- Update ChangeLog

[Eric Seidel (no email)]

Comment on attachment 38798 [details]
Proposed patch (rev.3)

LGTM.

[Eric Seidel (no email)]

Comment on attachment 38798 [details]
Proposed patch (rev.3)

Clearing flags on attachment: 38798

Committed r47889: <http://trac.webkit.org/changeset/47889>

[Eric Seidel (no email)]

All reviewed patches have been landed.  Closing bug.