Bug 160183

Summary:

DOMTokenList should be iterable

Product:

WebKit

Reporter:

Chris Dumez <cdumez>

Component:

Bindings

Assignee:

youenn fablet <youennf>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, commit-queue, esprehn+autocc, gyuyoung.kim, kondapallykalyan, youennf

Priority:

Keywords:

WebExposed

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch for landing	none

Chris Dumez

Reported 2016-07-25 19:49:47 PDT

DOMTokenList should be iterable: - https://dom.spec.whatwg.org/#interface-domtokenlist Chrome agrees with the specification.

Attachments
Patch (18.96 KB, patch) 2016-07-26 09:29 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Patch for landing (19.53 KB, patch) 2016-07-26 10:31 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Chris Dumez

Comment 1 2016-07-25 19:50:46 PDT

@youennf: Feel free to take if you have time given that you know this better than I do. Otherwise, I'll do it when I find some time.

youenn fablet

Comment 2 2016-07-26 04:07:46 PDT

Patch should consist in one WebIDL line and plenty of testing lines I guess ;) I'll try to write these. I made a PR to simplify the testing of iterables in https://github.com/w3c/testharness.js/pull/201 Hopefully it will get merged at some point.

youenn fablet

Comment 3 2016-07-26 09:29:46 PDT

Created attachment 284598 [details] Patch

Chris Dumez

Comment 4 2016-07-26 09:39:56 PDT

Comment on attachment 284598 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=284598&action=review r=me with comments. > Source/WebCore/ChangeLog:7 > + Please add a link to the specification. > LayoutTests/fast/dom/domTokenListIterator.html:1 > +<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN"> <!DOCTYPE html> > LayoutTests/fast/dom/domTokenListIterator.html:12 > + testRunner.dumpAsText(); This should not be needed when you use js-test-pre.js > LayoutTests/fast/dom/domTokenListIterator.html:17 > + function checkItemType(item) { There is a shouldBeType() function in js-test > LayoutTests/fast/dom/iterable-tests.js:26 > + shouldBe('end.done', 'true'); shouldBeTrue("end.done"); ? > LayoutTests/fast/dom/iterable-tests.js:27 > + shouldBe('end.value', 'undefined'); shouldBeUndefined("end.value"); ? > LayoutTests/fast/dom/iterable-tests.js:34 > +shouldBeTrue('testedIterable.entries === entriesFunction'); shouldBe("testedIterable.entries", "entriesFunction"); ? > LayoutTests/fast/dom/iterable-tests.js:35 > +shouldBeTrue('testedIterable.forEach === forEachFunction'); ditto > LayoutTests/fast/dom/iterable-tests.js:36 > +shouldBeTrue('testedIterable.keys === keysFunction'); ditto > LayoutTests/fast/dom/iterable-tests.js:37 > +shouldBeTrue('testedIterable.values === valuesFunction'); ditto > LayoutTests/fast/dom/iterable-tests.js:117 > + shouldBeTrue('checkItemType(v)'); shouldBeType() ? > LayoutTests/fast/dom/iterable-tests.js:124 > + shouldBeTrue('typeof k === "number"'); shouldBeType() ? > LayoutTests/fast/dom/iterable-tests.js:131 > + shouldBeTrue('typeof e[0] === "number"'); shouldBeType() ? > LayoutTests/fast/dom/iterable-tests.js:132 > + shouldBeTrue('e[1] === testedIterable[e[0]]'); shouldBe("e[1]", "testedIterable[e[0]]"); ? > LayoutTests/fast/dom/nodeListIterator.html:21 > + function checkItemType(item) { shouldBeType() already exists in js-test-pre.js

Chris Dumez

Comment 5 2016-07-26 09:40:15 PDT

Thanks for working on this!

youenn fablet

Comment 6 2016-07-26 10:31:51 PDT

Created attachment 284602 [details] Patch for landing

youenn fablet

Comment 7 2016-07-26 11:00:35 PDT

Thanks for the review. I updated accordingly except for checkItemType which can handle Object and literal types. > > LayoutTests/fast/dom/domTokenListIterator.html:17 > > + function checkItemType(item) { > > There is a shouldBeType() function in js-test We need to handle Node items which use instanceof and string literals which use typeof. I do not think DOMString mandates generating a String object.

WebKit Commit Bot

Comment 8 2016-07-26 11:02:09 PDT

Comment on attachment 284602 [details] Patch for landing Clearing flags on attachment: 284602 Committed r203728: <http://trac.webkit.org/changeset/203728>

WebKit Commit Bot

Comment 9 2016-07-26 11:02:15 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.