Bug 26915 - [HTML5][Forms] Part 1 of datalist&list: <datalist> element support
Summary: [HTML5][Forms] Part 1 of datalist&list: <datalist> element support
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P3 Enhancement
Assignee: Nobody
URL: http://www.whatwg.org/specs/web-apps/...
Keywords:
Depends on:
Blocks: HTML5Forms 27247 27756
  Show dependency treegraph
 
Reported: 2009-07-01 21:59 PDT by Kent Tamura
Modified: 2009-08-18 00:16 PDT (History)
5 users (show)

See Also:


Attachments
Proposed patch (30.01 KB, patch)
2009-07-06 01:56 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Proposed patch (rev.2) (24.78 KB, patch)
2009-07-07 01:19 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Proposed patch (rev.3) (30.11 KB, patch)
2009-07-12 22:00 PDT, Kent Tamura
eric: review-
Details | Formatted Diff | Diff
Proposed patch (rev.4) (31.53 KB, patch)
2009-07-15 22:32 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Proposed patch (rev.5) (31.65 KB, patch)
2009-07-23 23:26 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Proposed patch (rev.6) (41.37 KB, patch)
2009-08-09 23:24 PDT, Kent Tamura
eric: review+
eric: commit-queue-
Details | Formatted Diff | Diff
Proposed patch (rev.7) (41.41 KB, patch)
2009-08-17 23:05 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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 sideshowbarker 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 sideshowbarker 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.