26915 – [HTML5][Forms] Part 1 of datalist&list: <datalist> element support

RESOLVED FIXED 26915

[HTML5][Forms] Part 1 of datalist&list: <datalist> element support

https://bugs.webkit.org/show_bug.cgi?id=26915

Summary [HTML5][Forms] Part 1 of datalist&list: <datalist> element support

Kent Tamura

Reported 2009-07-01 21:59:00 PDT

We have a HTMLDataListElement DOM object for a <datalist> element. I'll make a patch.

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
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

Kent Tamura

Comment 1 2009-07-01 22:00:13 PDT

> We have a ... We should have a ...

sideshowbarker

Comment 2 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).

sideshowbarker

Comment 3 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"

Kent Tamura

Comment 4 2009-07-06 01:54:20 PDT

Mike, <output> element support will be another bug.

Kent Tamura

Comment 5 2009-07-06 01:56:21 PDT

Created attachment 32290 [details] Proposed patch

Eric Seidel (no email)

Comment 6 2009-07-07 00:36:33 PDT

Comment on attachment 32290 [details] Proposed patch The whitespace style updates make this hard to read.

Kent Tamura

Comment 7 2009-07-07 01:19:29 PDT

Created attachment 32363 [details] Proposed patch (rev.2) Reverted the whitespace changes in HTMLCollection.cpp

Kent Tamura

Comment 8 2009-07-12 22:00:48 PDT

Created attachment 32645 [details] Proposed patch (rev.3) Updates more build files including .gypi

Eric Seidel (no email)

Comment 9 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).

Kent Tamura

Comment 10 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.

Kent Tamura

Comment 11 2009-07-15 22:32:22 PDT

Created attachment 32835 [details] Proposed patch (rev.4) - Updated ChangeLog - Changed checkDTD() - Added another test

Kent Tamura

Comment 12 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.

Peter Kasting

Comment 13 2009-08-04 15:53:21 PDT

dhyatt was working on <datalist> support, he should be CCed on all bugs about it

Eric Seidel (no email)

Comment 14 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_

Kent Tamura

Comment 15 2009-08-09 23:24:17 PDT

Created attachment 34441 [details] Proposed patch (rev.6) Intoruduces ENABLE_DATALIST.

Eric Seidel (no email)

Comment 16 2009-08-17 18:25:16 PDT

Comment on attachment 34441 [details] Proposed patch (rev.6) OK.

Eric Seidel (no email)

Comment 17 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.

Eric Seidel (no email)

Comment 18 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

Kent Tamura

Comment 19 2009-08-17 23:05:40 PDT

Created attachment 35017 [details] Proposed patch (rev.7) Patch for today's WebKit.

Eric Seidel (no email)

Comment 20 2009-08-17 23:49:05 PDT

Comment on attachment 35017 [details] Proposed patch (rev.7) LGTM.

Eric Seidel (no email)

Comment 21 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>

Eric Seidel (no email)

Comment 22 2009-08-18 00:16:25 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.