Bug 26915

Summary: [HTML5][Forms] Part 1 of datalist&list: <datalist> element support
Product: WebKit Reporter: Kent Tamura <tkent>
Component: FormsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Enhancement CC: adele, eric, hyatt, michelangelo, mike
Priority: P3    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://www.whatwg.org/specs/web-apps/current-work/#the-datalist-element
Bug Depends on:    
Bug Blocks: 19264, 27247, 27756    
Attachments:
Description Flags
Proposed patch
none
Proposed patch (rev.2)
none
Proposed patch (rev.3)
eric: review-
Proposed patch (rev.4)
none
Proposed patch (rev.5)
none
Proposed patch (rev.6)
eric: review+, eric: commit-queue-
Proposed patch (rev.7) none

Description Kent Tamura 2009-07-01 21:59:00 PDT
We have a HTMLDataListElement DOM object for a <datalist> element.

I'll make a patch.
Comment 1 Kent Tamura 2009-07-01 22:00:13 PDT
> We have a ... 

We should have a ...

Comment 2 Michael[tm] Smith 2009-07-02 07:40:09 PDT
Hi Kent,

(In reply to comment #0)
> We should have a HTMLDataListElement DOM object for a <datalist> element.

I don't know what your implementation plans/schedule are with regard to the <output> element, but just wanted to know you might also want to make a reminder about the adding an HTMLOutputElement IDL (if you haven't already).
Comment 3 Michael[tm] Smith 2009-07-02 07:43:05 PDT
(In reply to comment #2)
> ... but just wanted to know you might also want to" 

make that, "but just wanted to say you might also want to"

Comment 4 Kent Tamura 2009-07-06 01:54:20 PDT
Mike,
<output> element support will be another bug.
Comment 5 Kent Tamura 2009-07-06 01:56:21 PDT
Created attachment 32290 [details]
Proposed patch
Comment 6 Eric Seidel (no email) 2009-07-07 00:36:33 PDT
Comment on attachment 32290 [details]
Proposed patch

The whitespace style updates make this hard to read.
Comment 7 Kent Tamura 2009-07-07 01:19:29 PDT
Created attachment 32363 [details]
Proposed patch (rev.2)

Reverted the whitespace changes in HTMLCollection.cpp
Comment 8 Kent Tamura 2009-07-12 22:00:48 PDT
Created attachment 32645 [details]
Proposed patch (rev.3)

Updates more build files including .gypi
Comment 9 Eric Seidel (no email) 2009-07-15 16:14:28 PDT
Comment on attachment 32645 [details]
Proposed patch (rev.3)

Your ChangeLog is doubled.

Why is this right?
bool HTMLDataListElement::checkDTD(const Node* newChild)
 48 {
 49     return newChild->hasTagName(HTMLNames::optionTag) || HTMLElement::checkDTD(newChild);
 50 }

You should add a test to test the checkDTD code.  If you're trying to prevent adding non-option elements then we need to test that?

Otherwise this looks fine.  r- since you're not a committer and this needs a small update (and ideally another test).
Comment 10 Kent Tamura 2009-07-15 22:30:29 PDT
(In reply to comment #9)
> (From update of attachment 32645 [details])
> Your ChangeLog is doubled.

Fixed.

> Why is this right?
> bool HTMLDataListElement::checkDTD(const Node* newChild)
>  48 {
>  49     return newChild->hasTagName(HTMLNames::optionTag) ||
> HTMLElement::checkDTD(newChild);
>  50 }

According to the standard, we need to allow <options>s and inline elements.  So HTMLElement::checkDTD() was wrong.  I have changed it to inInlineTagList(), and added a test for this behavior.
Comment 11 Kent Tamura 2009-07-15 22:32:22 PDT
Created attachment 32835 [details]
Proposed patch (rev.4)

- Updated ChangeLog
- Changed checkDTD()
- Added another test
Comment 12 Kent Tamura 2009-07-23 23:26:56 PDT
Created attachment 33409 [details]
Proposed patch (rev.5)

Just updated the patch for project.pbxproj because the previous patch made a conflict with the current source.
Comment 13 Peter Kasting 2009-08-04 15:53:21 PDT
dhyatt was working on <datalist> support, he should be CCed on all bugs about it
Comment 14 Eric Seidel (no email) 2009-08-07 12:36:34 PDT
Comment on attachment 33409 [details]
Proposed patch (rev.5)

As an unfinished feature, it seems this should be guarded by some sort of ENABLE_
Comment 15 Kent Tamura 2009-08-09 23:24:17 PDT
Created attachment 34441 [details]
Proposed patch (rev.6)

Intoruduces ENABLE_DATALIST.
Comment 16 Eric Seidel (no email) 2009-08-17 18:25:16 PDT
Comment on attachment 34441 [details]
Proposed patch (rev.6)

OK.
Comment 17 Eric Seidel (no email) 2009-08-17 18:32:32 PDT
Comment on attachment 34441 [details]
Proposed patch (rev.6)

Rejecting patch 34441 from commit-queue.  This patch will require manual commit.

Patch https://bugs.webkit.org/attachment.cgi?id=34441 from bug 26915 failed to download and apply.
Comment 18 Eric Seidel (no email) 2009-08-17 22:43:37 PDT
patching file WebCore/Configurations/FeatureDefines.xcconfig
Hunk #1 succeeded at 37 with fuzz 2.
Hunk #2 FAILED at 60.
1 out of 2 hunks FAILED -- saving rejects to file WebCore/Configurations/FeatureDefines.xcconfig.rej
Comment 19 Kent Tamura 2009-08-17 23:05:40 PDT
Created attachment 35017 [details]
Proposed patch (rev.7)

Patch for today's WebKit.
Comment 20 Eric Seidel (no email) 2009-08-17 23:49:05 PDT
Comment on attachment 35017 [details]
Proposed patch (rev.7)

LGTM.
Comment 21 Eric Seidel (no email) 2009-08-18 00:16:19 PDT
Comment on attachment 35017 [details]
Proposed patch (rev.7)

Clearing flags on attachment: 35017

Committed r47420: <http://trac.webkit.org/changeset/47420>
Comment 22 Eric Seidel (no email) 2009-08-18 00:16:25 PDT
All reviewed patches have been landed.  Closing bug.