New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 632935 link

Starred by 5 users
Status: Fixed
Owner:
█ raphael....@intel.com
OoO until Feb 4th
Closed: Feb 2017
Cc:
yukishiino@chromium.org
peria@chromium.org
█ msrchandra@chromium.org
lfg@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Various functions on single-type iterables are not aliases of the relevant Array.protoype functions

Reported by bzbar...@mit.edu, Jul 30 2016 
UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:50.0) Gecko/20100101 Firefox/50.0

Example URL:

Steps to reproduce the problem:
1. Load any web page.
2. Evaluate the expression "document.childNodes.forEach == Array.prototype.forEach" in the console.

What is the expected behavior?
Evaluates to true.

What went wrong?
Evaluates to false.

Does it occur on multiple sites: Yes

Is it a problem with a plugin? No 

Did this work before? No 

Does this work in other browsers? Yes 

Chrome version: 54.0.2810.2 (Official Build) dev (64-bit)  Channel: n/a
OS Version: OS X 10.10
Flash Version: 

The same issue comes up for keys, values, entries, and Symbol.iterator.

Spec at http://heycam.github.io/webidl/#es-forEach says:

  If the interface defines an indexed property getter,
  then the Function object is the initial value of the
  “forEach” data property of %ArrayPrototype%
  ([ECMA-262], section 6.1.7.4). 

and similar for the other property names.

Comment 1 by kojii@chromium.org, Jul 30 2016

Components: -Blink Blink>Bindings
Labels: -OS-Mac OS-All

Comment 2 by msrchandra@chromium.org, Aug 1 2016

Cc: msrchandra@chromium.org
Labels: Needs-Feedback
@bzbarsky -- Thank You for the report.
Followed the steps as mentioned in the report and observed the expression evaluates False.
Re-checked te same on Firefox and observed the same behavior.
Could you again please re-check and let us know if i am missing anything.
Thanks in Advance.

Comment 3 by bzbar...@mit.edu, Aug 1 2016 

Were you using a current Firefox nightly?  Release Firefox doesn't implement a single-type iterator on NodeList at all, so there document.childNodes.forEach is just undefined.

Current Firefox nightlies do implement it, and document.childNodes.forEach == Array.prototype.forEach.

Comment 4 by bzbar...@mit.edu, Aug 1 2016 

And note that Chrome _does_ get this right for things with an indexed getter that aren't one-type iterables.  For example, in Chrome:

  document.getElementsByTagName("html")[Symbol.iterator] == Array.prototype[Symbol.iterator]

tests true, as it should.  But 

  document.childNodes[Symbol.iterator] == Array.prototype[Symbol.iterator]

tests false.  And yes, both are functions; they're just different functions.

Comment 5 by bzbar...@mit.edu, Aug 2 2016 

I was just told that WebKit nightly gets this right, and indeed it does.

Comment 6 by mitay...@mozilla.com, Aug 2 2016

Labels: Hotlist-Interop

Comment 7 by phistuck@chromium.org, Aug 2 2016

Labels: -Needs-Feedback
Status: Untriaged (was: Unconfirmed)

Comment 9 by peria@chromium.org, Aug 25 2016

Cc: peria@chromium.org
Labels: -Pri-2 Pri-3
Owner: yukishiino@chromium.org
Status: Available (was: Untriaged)
Project Member

Comment 10 by bugdroid1@chromium.org, Sep 15 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3d6d14555abd484cf8be6f15e02d8ba5b239cf33

commit 3d6d14555abd484cf8be6f15e02d8ba5b239cf33
Author: yukishiino <yukishiino@chromium.org>
Date: Thu Sep 15 13:48:12 2016

binding: Indexed properties should accompany with %ArrayProto_values%.

http://heycam.github.io/webidl/#es-iterator

If an interface has indexed property getter and length attribute,
then @@iterator must be %ArrayProto_values%.

Plus, @@iterator must be {writable: true, enumerable: false,
configurable: true}.

BUG= 632935 

Review-Url: https://codereview.chromium.org/2341643003
Cr-Commit-Position: refs/heads/master@{#418858}

[modify] https://crrev.com/3d6d14555abd484cf8be6f15e02d8ba5b239cf33/third_party/WebKit/LayoutTests/imported/wpt/dom/lists/DOMTokenList-iteration-expected.txt
[modify] https://crrev.com/3d6d14555abd484cf8be6f15e02d8ba5b239cf33/third_party/WebKit/LayoutTests/imported/wpt/dom/nodes/Node-childNodes-expected.txt
[modify] https://crrev.com/3d6d14555abd484cf8be6f15e02d8ba5b239cf33/third_party/WebKit/Source/bindings/scripts/v8_interface.py
[modify] https://crrev.com/3d6d14555abd484cf8be6f15e02d8ba5b239cf33/third_party/WebKit/Source/bindings/templates/interface_base.cpp
[modify] https://crrev.com/3d6d14555abd484cf8be6f15e02d8ba5b239cf33/third_party/WebKit/Source/bindings/tests/results/core/V8TestIntegerIndexed.cpp
[modify] https://crrev.com/3d6d14555abd484cf8be6f15e02d8ba5b239cf33/third_party/WebKit/Source/bindings/tests/results/core/V8TestIntegerIndexedGlobal.cpp
[modify] https://crrev.com/3d6d14555abd484cf8be6f15e02d8ba5b239cf33/third_party/WebKit/Source/bindings/tests/results/core/V8TestIntegerIndexedPrimaryGlobal.cpp
[modify] https://crrev.com/3d6d14555abd484cf8be6f15e02d8ba5b239cf33/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface.cpp
[modify] https://crrev.com/3d6d14555abd484cf8be6f15e02d8ba5b239cf33/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface2.cpp
[modify] https://crrev.com/3d6d14555abd484cf8be6f15e02d8ba5b239cf33/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface3.cpp
[modify] https://crrev.com/3d6d14555abd484cf8be6f15e02d8ba5b239cf33/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceGarbageCollected.cpp
[modify] https://crrev.com/3d6d14555abd484cf8be6f15e02d8ba5b239cf33/third_party/WebKit/Source/bindings/tests/results/core/V8TestObject.cpp
[modify] https://crrev.com/3d6d14555abd484cf8be6f15e02d8ba5b239cf33/third_party/WebKit/Source/bindings/tests/results/modules/V8TestInterface5.cpp

Comment 11 by raphael....@intel.com, Feb 3 2017

Cc: yukishiino@chromium.org
Owner: raphael....@intel.com
Status: Started (was: Available)
Summary: Various functions on single-type iterables are not aliases of the relevant Array.protoype functions (was: Various functions on single-type iterables are not aliases of the relevant Array.prottoype functions)
Project Member

Comment 12 by bugdroid1@chromium.org, Feb 7 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/e7b78ac22937f994d35376b38416a5844d6beced

commit e7b78ac22937f994d35376b38416a5844d6beced
Author: raphael.kubo.da.costa <raphael.kubo.da.costa@intel.com>
Date: Tue Feb 07 21:42:06 2017

Expose more %ArrayPrototype% functions to the public API.

In addition to Array.prototype.values() which is already exposed, Blink
needs access to entries(), forEach() and keys() to properly set the
corresponding functions in value iterators for WebIDL conformance.

Add a few new entries to NATIVE_CONTEXT_IMPORTED_FIELDS and expand
V8_INTRINSICS_LIST, as well as some API tests for all these new exposed
functions.

BUG= chromium:632935 
R=caitp@igalia.com,jochen@chromium.org,verwaest@chromium.org,yukishiino@chromium.org

Review-Url: https://codereview.chromium.org/2670833008
Cr-Commit-Position: refs/heads/master@{#43017}

[modify] https://crrev.com/e7b78ac22937f994d35376b38416a5844d6beced/include/v8.h
[modify] https://crrev.com/e7b78ac22937f994d35376b38416a5844d6beced/src/contexts.h
[modify] https://crrev.com/e7b78ac22937f994d35376b38416a5844d6beced/src/js/array.js
[modify] https://crrev.com/e7b78ac22937f994d35376b38416a5844d6beced/test/cctest/test-api.cc
Project Member

Comment 13 by bugdroid1@chromium.org, Feb 11 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c85bc01513148e381d656d88a7f8a773704d6c34

commit c85bc01513148e381d656d88a7f8a773704d6c34
Author: raphael.kubo.da.costa <raphael.kubo.da.costa@intel.com>
Date: Sat Feb 11 13:37:00 2017

bindings: Make some value iterator properties aliases to Array.prototype functions

The WebIDL spec states that value iterators in an interface should cause the
following operations to be added and be originally set to the corresponding
property of ECMAScript's %ArrayPrototype%:
* entries
* forEach
* keys
* values

Stop generating custom methods for those operations in the bindings when a
value iterator is defined; instead, just set properties with those names to
the corresponding intrinsics values from V8.

This allows us to get a lot of code for free, avoid duplicating iterator
code in Blink and remove the ValueIterable class altogether from the
bindings code.

However, doing so does require us to adjust non-conforming IDLs:
https://heycam.github.io/webidl/#idl-iterable states that "a value iterator
must only be declared on an interface that supports indexed properties", and
"supports indexed properties" means an interface has an indexed getter
operation as well as an integer-type attribute called "length". Some IDLs in
Blink only had an "iterable<V>" declaration without the accompanying getter
and "length" attribute. In the case of both CSSTransformValue and
CSSUnparsedValue, a spec bug was also filed upstream to tackle this at the
source.

Finally, the bindings generation code now raises a ValueError if an
interface only has an iterable<V> declaration without the accompanying
indexed property getter and length attribute.

BUG=545318, 632935 

Review-Url: https://codereview.chromium.org/2683853005
Cr-Commit-Position: refs/heads/master@{#449857}

[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/LayoutTests/external/wpt/dom/nodes/Node-childNodes-expected.txt
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/LayoutTests/fast/js/iterable-object-expected.txt
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/LayoutTests/http/tests/worklet/webexposed/global-interface-listing-paint-worklet-expected.txt
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/bindings/core/v8/Iterable.h
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/bindings/scripts/idl_definitions.py
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/bindings/scripts/v8_interface.py
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/bindings/tests/idls/core/TestInterface3.idl
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/bindings/tests/idls/modules/TestInterface5.idl
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface3.cpp
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface3.h
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/bindings/tests/results/core/test_interface_3.cc
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/bindings/tests/results/modules/V8TestInterface5.cpp
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/bindings/tests/results/modules/V8TestInterface5.h
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/core/css/cssom/CSSTransformValue.cpp
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/core/css/cssom/CSSTransformValue.h
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/core/css/cssom/CSSTransformValue.idl
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/core/css/cssom/CSSUnparsedValue.cpp
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/core/css/cssom/CSSUnparsedValue.h
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/core/css/cssom/CSSUnparsedValue.idl
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/core/css/cssom/CSSUnparsedValueTest.cpp
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/core/dom/BUILD.gn
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/core/dom/DOMTokenList.cpp
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/core/dom/DOMTokenList.h
[delete] https://crrev.com/01199c020ca74ac8d32a7d2939d0a090cf08d801/third_party/WebKit/Source/core/dom/NodeList.cpp
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/core/dom/NodeList.h
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/core/testing/Internals.cpp
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/core/testing/Internals.h
[modify] https://crrev.com/c85bc01513148e381d656d88a7f8a773704d6c34/third_party/WebKit/Source/core/testing/Internals.idl

Comment 14 by raphael....@intel.com, Feb 13 2017

Status: Fixed (was: Started)
I think this is done and we're finally aligned with the spec and other engines \o/
►

Sign in to add a comment