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

Issue 703990 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
OoO until Feb 4th
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature

Blocked on:
issue v8:6627

Blocking:
issue 701561



Sign in to add a comment

Support LegacyUnenumerableNamedProperties

Project Member Reported by foolip@chromium.org, Mar 22 2017

Issue description

Comment 1 by lunalu@chromium.org, Mar 24 2017

[LegacyUnenumerableNamedProperties] extended attribute appears on a interface that supports named properties, it indicates that all the interface’s named properties are unenumerable."

Comment 2 by lunalu@chromium.org, Mar 24 2017

Blocking: 701561
TODO: should update the getter namedItem in MimeTypeArray | PluginArray | Plugin once this is supported. 
Labels: -Type-Bug Type-Feature
Status: Available (was: Untriaged)
Implementing this should also help clean up the bindings code: some getters/setters have [NotEnumerable], which does not make sense for indexed properties and which should be replaced by [LegacyUnenumerableNamedProperties] (in the interface declaration) for named properties.
Owner: raphael....@intel.com
Status: Started (was: Available)
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 20 2017

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

commit 4d299da2bb35b40e9cc2a64a8e273d8301d234a3
Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Date: Thu Jul 20 15:30:07 2017

bindings: Return the right property descriptors for legacy platform objects.

The existing implementation of named and indexed properties in IDL
interfaces did not return the right property descriptor values according to
https://heycam.github.io/webidl/#LegacyPlatformObjectGetOwnProperty:
* Indexed properties always had [[Writable]] set to true (its value must
  depend on the presence of an indexed property setter) and [[Enumerable]]
  set to false (due to V8's default value when a handler does not set either
  a query or descriptor callback; it must always be true).
* Named properties always set [[Writable]] to true (its value must depend on
  the presence of a named property setter).

Since we initialize both |NamedPropertyHandlerConfiguration| and
|IndexedPropertyHandlerConfiguration| in our code with different
constructors, the fix for the two cases above is not "symmetrical":
* |IndexedPropertyHandlerConfiguration| is constructed with a callback for
  [[DefineOwnProperty]], so we cannot specify an
  |IndexedPropertyQueryCallback| like |NamedPropertyHandlerConfiguration|
  does (see https://codereview.chromium.org/2311873002). In this case, we
  add an |IndexedPropertyDescriptorCallback| that is responsible for
  implementing the steps from the WebIDL spec and returning a v8::Object
  with the properties a property descriptor must have.
* |NamedPropertyHandlerConfiguration|'s case is easier because we can just
  fix its existing |GenericNamedPropertyQueryCallback| and return either
  v8::None or v8::ReadOnly depending on the presence of a named callback
  setter.

It is also important to note that when an IDL interface is declared with
[LegacyUnenumerableNamedProperties] its named properties should not be
enumerable; we still need to add support for that extended attribute to the
bindings code though, so Object.keys() or Object.values() on e.g.
HTMLCollection still return more entries than they should.

Bug:  703990 ,  719703 
Change-Id: I772f92a377a1f5013f4f78e5d361a43974378d1a
Reviewed-on: https://chromium-review.googlesource.com/576183
Commit-Queue: Raphael Kubo da Costa (rakuco) <raphael.kubo.da.costa@intel.com>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488252}
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/LayoutTests/external/wpt/WebIDL/ecmascript-binding/legacy-platform-object-expected.txt
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/LayoutTests/external/wpt/WebIDL/ecmascript-binding/legacy-platform-object.html
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/LayoutTests/external/wpt/dom/collections/HTMLCollection-supported-property-indices-expected.txt
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/LayoutTests/external/wpt/dom/nodes/Document-getElementsByTagName-expected.txt
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/LayoutTests/external/wpt/dom/nodes/Element-getElementsByTagName-expected.txt
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/LayoutTests/external/wpt/dom/nodes/attributes-expected.txt
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/LayoutTests/external/wpt/html/browsers/the-window-object/window-indexed-properties-expected.txt
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/LayoutTests/external/wpt/html/semantics/forms/the-form-element/form-indexed-element-expected.txt
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/LayoutTests/fast/js/getOwnPropertyDescriptor-expected.txt
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/LayoutTests/fast/js/resources/getOwnPropertyDescriptor.js
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/core/v8/V8BindingForCore.h
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/templates/interface.cpp.tmpl
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/templates/interface.h.tmpl
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/tests/idls/core/TestSpecialOperationsNotEnumerable.idl
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/tests/idls/modules/TestInterface5.idl
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/tests/results/core/V8TestIntegerIndexed.cpp
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/tests/results/core/V8TestIntegerIndexed.h
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/tests/results/core/V8TestIntegerIndexedGlobal.cpp
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/tests/results/core/V8TestIntegerIndexedGlobal.h
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/tests/results/core/V8TestIntegerIndexedPrimaryGlobal.cpp
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/tests/results/core/V8TestIntegerIndexedPrimaryGlobal.h
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface.cpp
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface.h
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface2.cpp
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface2.h
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface3.cpp
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface3.h
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/tests/results/core/V8TestObject.cpp
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/tests/results/core/V8TestObject.h
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/tests/results/core/V8TestSpecialOperations.cpp
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/tests/results/core/V8TestSpecialOperationsNotEnumerable.cpp
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/tests/results/core/V8TestSpecialOperationsNotEnumerable.h
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/tests/results/modules/V8TestInterface5.cpp
[modify] https://crrev.com/4d299da2bb35b40e9cc2a64a8e273d8301d234a3/third_party/WebKit/Source/bindings/tests/results/modules/V8TestInterface5.h

Blockedon: v8:6627
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 22 2017

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

commit cc9ff6517e7746ddba70e2eade794968cd6d4287
Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Date: Tue Aug 22 16:37:24 2017

bindings: Implement support for [LegacyUnenumerableNamedProperties].

This interface extended attribute, inherited automatically by all child
interfaces, determines whether named properties in legacy platform
objects (such as HTMLCollection) should be enumerable or not; so far, named
properties were always enumerable unless the getter operation was marked
with the non-standard [NotEnumerable] extended attribute in the IDL file.

This CL only adds the required infrastructure to the bindings scripts and
templates; converting existing IDL files will be done separately as some of
the changes have user-visible impact.

Compared to using [NotEnumerable] in a named getter, [LUNP] makes
controlling whether a property should be returned or not more fine-grained.
[NotEnumerable] simply creates the v8::NamedPropertyHandlerConfiguration
responsible for handling named property access without the query and
enumerator callbacks (in this case, the named properties are marked as not
enumerable simply because V8 marks them as such when no query callback is
set). [LegacyUnenumerableNamedProperties], on the other hand, defines query
and enumerator callbacks, which must be implemented on the C++ side and
correspond to the concept of "supported property names" of WebIDL, and uses
the query callback to mark a property as not enumerable.

In practice, both [NotEnumerable] and [LUNP] work identically with for-in
loops, Object.keys() and Object.getOwnPropertyDescriptor(). However,
[NotEnumerable] omits named properties even when it must not, as is the
case with both Object.getOwnPropertyNames() and
Object.getOwnPropertyDescriptors().

Bug:  703990 
Change-Id: I1a4be4bd46fb9638d8875da56ccace3fcdd8da2e
Reviewed-on: https://chromium-review.googlesource.com/580990
Commit-Queue: Raphael Kubo da Costa (rakuco) <raphael.kubo.da.costa@intel.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496328}
[modify] https://crrev.com/cc9ff6517e7746ddba70e2eade794968cd6d4287/third_party/WebKit/Source/bindings/IDLExtendedAttributes.md
[modify] https://crrev.com/cc9ff6517e7746ddba70e2eade794968cd6d4287/third_party/WebKit/Source/bindings/IDLExtendedAttributes.txt
[modify] https://crrev.com/cc9ff6517e7746ddba70e2eade794968cd6d4287/third_party/WebKit/Source/bindings/scripts/compute_interfaces_info_overall.py
[modify] https://crrev.com/cc9ff6517e7746ddba70e2eade794968cd6d4287/third_party/WebKit/Source/bindings/scripts/idl_definitions.py
[modify] https://crrev.com/cc9ff6517e7746ddba70e2eade794968cd6d4287/third_party/WebKit/Source/bindings/scripts/v8_interface.py
[modify] https://crrev.com/cc9ff6517e7746ddba70e2eade794968cd6d4287/third_party/WebKit/Source/bindings/templates/interface.cpp.tmpl
[modify] https://crrev.com/cc9ff6517e7746ddba70e2eade794968cd6d4287/third_party/WebKit/Source/bindings/tests/idls/core/TestSpecialOperationsNotEnumerable.idl
[add] https://crrev.com/cc9ff6517e7746ddba70e2eade794968cd6d4287/third_party/WebKit/Source/bindings/tests/idls/modules/TestInheritedLegacyUnenumerableNamedProperties.idl
[modify] https://crrev.com/cc9ff6517e7746ddba70e2eade794968cd6d4287/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface.cpp
[modify] https://crrev.com/cc9ff6517e7746ddba70e2eade794968cd6d4287/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface2.cpp
[modify] https://crrev.com/cc9ff6517e7746ddba70e2eade794968cd6d4287/third_party/WebKit/Source/bindings/tests/results/core/V8TestObject.cpp
[modify] https://crrev.com/cc9ff6517e7746ddba70e2eade794968cd6d4287/third_party/WebKit/Source/bindings/tests/results/core/V8TestSpecialOperations.cpp
[modify] https://crrev.com/cc9ff6517e7746ddba70e2eade794968cd6d4287/third_party/WebKit/Source/bindings/tests/results/core/V8TestSpecialOperationsNotEnumerable.cpp
[modify] https://crrev.com/cc9ff6517e7746ddba70e2eade794968cd6d4287/third_party/WebKit/Source/bindings/tests/results/core/V8TestSpecialOperationsNotEnumerable.h
[add] https://crrev.com/cc9ff6517e7746ddba70e2eade794968cd6d4287/third_party/WebKit/Source/bindings/tests/results/modules/V8TestInheritedLegacyUnenumerableNamedProperties.cpp
[add] https://crrev.com/cc9ff6517e7746ddba70e2eade794968cd6d4287/third_party/WebKit/Source/bindings/tests/results/modules/V8TestInheritedLegacyUnenumerableNamedProperties.h
[modify] https://crrev.com/cc9ff6517e7746ddba70e2eade794968cd6d4287/third_party/WebKit/Source/bindings/tests/results/modules/V8TestInterface5.cpp

Labels: Hotlist-Interop
Labels: M-63
I think this is going to be M63 material given M62 is close to branching and should already be in feature freeze mode.

The infrastructure required to support [LUNP] has landed, but nothing is currently using it yet. Converting existing IDLs as suggested in comments #1 and #2 depends on https://bugs.chromium.org/p/v8/issues/detail?id=6627 being fixed again after its CLs were reverted.
Project Member

Comment 11 by bugdroid1@chromium.org, Sep 8 2017

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

commit 783087dbdf840f4222b7ef3fa5b462dcb49dbaec
Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Date: Fri Sep 08 09:32:08 2017

plugins: Convert IDLs to [LegacyUnenumerableNamedProperties].

Start using the [LegacyUnenumerableNamedProperties] extended attribute in
the interface declarations for MimeTypeArray, Plugin and PluginArray and
drop the non-standard [NotEnumerable] attribute from the named getter.

Doing so also allows us to get rid of the duplicate namedItem() + anonymous
named getter combination in the IDLs in favor of just making namedItem()
itself the named getter as described in the spec.

In pratice, the user-visible differences created by this patch are subtle,
as [NotEnumerable] was already being used to make named properties not
enumerable. Using [LegacyUnenumerableNamedProperties] improves conformance
by making them be listed by e.g. Object.getOwnPropertyDescriptors() and
Object.getOwnPropertyNames() while working correctly with for-in loops and
Object.keys() just like before.

Bug:  703990 
Change-Id: I1bfdd2be18405f3e003c0026da56ae3e4c2ec752
Reviewed-on: https://chromium-review.googlesource.com/626597
Commit-Queue: Raphael Kubo da Costa (rakuco) <raphael.kubo.da.costa@intel.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500543}
[modify] https://crrev.com/783087dbdf840f4222b7ef3fa5b462dcb49dbaec/third_party/WebKit/LayoutTests/external/wpt/html/webappapis/system-state-and-capabilities/the-navigator-object/navigator-pluginarray.html
[modify] https://crrev.com/783087dbdf840f4222b7ef3fa5b462dcb49dbaec/third_party/WebKit/Source/modules/plugins/DOMMimeTypeArray.cpp
[modify] https://crrev.com/783087dbdf840f4222b7ef3fa5b462dcb49dbaec/third_party/WebKit/Source/modules/plugins/DOMMimeTypeArray.h
[modify] https://crrev.com/783087dbdf840f4222b7ef3fa5b462dcb49dbaec/third_party/WebKit/Source/modules/plugins/DOMPlugin.cpp
[modify] https://crrev.com/783087dbdf840f4222b7ef3fa5b462dcb49dbaec/third_party/WebKit/Source/modules/plugins/DOMPlugin.h
[modify] https://crrev.com/783087dbdf840f4222b7ef3fa5b462dcb49dbaec/third_party/WebKit/Source/modules/plugins/DOMPluginArray.cpp
[modify] https://crrev.com/783087dbdf840f4222b7ef3fa5b462dcb49dbaec/third_party/WebKit/Source/modules/plugins/DOMPluginArray.h
[modify] https://crrev.com/783087dbdf840f4222b7ef3fa5b462dcb49dbaec/third_party/WebKit/Source/modules/plugins/MimeTypeArray.idl
[modify] https://crrev.com/783087dbdf840f4222b7ef3fa5b462dcb49dbaec/third_party/WebKit/Source/modules/plugins/Plugin.idl
[modify] https://crrev.com/783087dbdf840f4222b7ef3fa5b462dcb49dbaec/third_party/WebKit/Source/modules/plugins/PluginArray.idl
[modify] https://crrev.com/783087dbdf840f4222b7ef3fa5b462dcb49dbaec/third_party/WebKit/Source/platform/plugins/PluginData.h

Project Member

Comment 12 by bugdroid1@chromium.org, Sep 8 2017

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

commit 482ec872f8974245c8649ac2fecf60782f907b0e
Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Date: Fri Sep 08 10:54:59 2017

NamedNodeMap: Convert to [LegacyUnenumerableNamedProperties].

Start using the [LegacyUnenumerableNamedProperties] extended attribute in
the interface declaration and drop the non-standard [NotEnumerable]
attribute from the named getter. Doing so also allows us to get rid of the
duplicate getNamedItem() + anonymous named getter combination in the IDLs in
favor of just making getNamedItem() itself the named getter as mandated by
the spec.

In pratice, the user-visible differences created by this patch are subtle,
as [NotEnumerable] was already being used to make named properties not
enumerable.

for-in loops and Object.keys() were already working as expected before; with
this CL, we make everything work consistently, including
Object.getOwnPropertyDescriptors() and Object.getOwnPropertyNames(), by
implementing query and enumerator callbacks that correspond to the
description of "supported property names" in the spec.

Bug:  703990 
Change-Id: I01f9486cdc81c8be6ba2ca28ae45bb7153ba183d
Reviewed-on: https://chromium-review.googlesource.com/654041
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Hayato Ito <hayato@chromium.org>
Commit-Queue: Raphael Kubo da Costa (rakuco) <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/master@{#500561}
[delete] https://crrev.com/cf340a2eb631728a4ce6cbb11c060e8a72e1ae34/third_party/WebKit/LayoutTests/external/wpt/dom/collections/namednodemap-supported-property-names-expected.txt
[delete] https://crrev.com/cf340a2eb631728a4ce6cbb11c060e8a72e1ae34/third_party/WebKit/LayoutTests/external/wpt/dom/nodes/attributes-expected.txt
[modify] https://crrev.com/482ec872f8974245c8649ac2fecf60782f907b0e/third_party/WebKit/Source/core/dom/NamedNodeMap.cpp
[modify] https://crrev.com/482ec872f8974245c8649ac2fecf60782f907b0e/third_party/WebKit/Source/core/dom/NamedNodeMap.h
[modify] https://crrev.com/482ec872f8974245c8649ac2fecf60782f907b0e/third_party/WebKit/Source/core/dom/NamedNodeMap.idl

I2I mail sent to change HTMLCollection and HTMLAllCollection: https://groups.google.com/a/chromium.org/d/msg/blink-dev/YbsMn7DL1cg/rdkA-It1AgAJ because they currently don't declare their named properties unenumerable via [NotEnumerable].
@Raphael, the asymmetry between Object.keys()/Object.values() for the Storage interface will be fixed in this issue?

localStorage.setItem(0, 0)
Object.getOwnPropertyDescriptor(localStorage,0).enumerable // false
Object.keys(localStorage).length //1
Object.values(localStorage).length // 0


Storage (from which WindowLocalStorage inherits) is not declared with [LegacyUnenumerableNamedProperties]: https://html.spec.whatwg.org/multipage/webstorage.html#the-storage-interface

So I suggest you file a separate bug about that.
Correction: it's possible that either my commit from comment #6 or the V8 changes from bug that was blocking this one have fixed your issue. I've tried content_shell with Chromium master and got the right values from localStorage.
Could you please double-check that? I tried the latest Chromium build before I post comment #14.
Hmm, it looks like numeric keys are still buggy. In any case, this is unrelated to [LegacyUnenumerableNamedProperties], so can you file a separate bug and CC me there?
> can you file a separate bug and CC me there?

Actually, I've got a fix here. I'll reference  bug 719703  instead, as it's an edge case for the change I added in comment #6.
Project Member

Comment 20 by bugdroid1@chromium.org, Sep 19 2017

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

commit 231585efb676b7d99748e29f9049b29bf1f84f67
Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Date: Tue Sep 19 00:42:47 2017

Make HTMLAllCollection and HTMLCollection's named properties not enumerable.

Add the [LegacyUnenumerableNamedProperties] extended attribute to both IDL
files so that they follow what the DOM and HTML specs mandate. Since the
attribute is inherited, it also applies to both HTMLOptionsCollection and
HTMLFormControlsCollection.

Both HTMLAllCollection and HTMLCollection already implemented the
required enumeration and query callbacks (in other words, they already
implemented the specs' definitions of "supported property names" for each
interface).

The user-visible difference is that all named properties were previously
enumerable, while with this CL they are not: they are no longer returned by
Object.keys() or iterated by for-in loops, but Object.getOwnPropertyNames()
and Object.getOwnPropertyDescriptors() still include them. The value of the
"enumerable" property of the property descriptors returned is now false.

WebKit did the same thing in r210667 from January 2017 (was already marking
the properties as not enumerable before), and Gecko did the same in bug
1270349 in May 2016 (the properties were already not enumerable before that,
as explained in the Intent to Implement email).

Some tests in fast/dom were removed, as they only served to test the
previous behavior and the new, expected behavior is coverted by
web-platform-tests.

Intent to Implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/YbsMn7DL1cg/rdkA-It1AgAJ

Bug:  703990 ,  719703 
Change-Id: Id6c899c1715f22a426f24c30f0ad3f527beb14d5
Reviewed-on: https://chromium-review.googlesource.com/654837
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502733}
[modify] https://crrev.com/231585efb676b7d99748e29f9049b29bf1f84f67/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
[delete] https://crrev.com/9e319197e596a8b57465591c2ded1afac2ab484d/third_party/WebKit/LayoutTests/external/wpt/WebIDL/ecmascript-binding/legacy-platform-object-expected.txt
[modify] https://crrev.com/231585efb676b7d99748e29f9049b29bf1f84f67/third_party/WebKit/LayoutTests/external/wpt/dom/collections/HTMLCollection-supported-property-names-expected.txt
[delete] https://crrev.com/9e319197e596a8b57465591c2ded1afac2ab484d/third_party/WebKit/LayoutTests/external/wpt/dom/nodes/Document-getElementsByTagName-expected.txt
[delete] https://crrev.com/9e319197e596a8b57465591c2ded1afac2ab484d/third_party/WebKit/LayoutTests/external/wpt/dom/nodes/Element-children-expected.txt
[delete] https://crrev.com/9e319197e596a8b57465591c2ded1afac2ab484d/third_party/WebKit/LayoutTests/external/wpt/dom/nodes/Element-getElementsByTagName-expected.txt
[delete] https://crrev.com/9e319197e596a8b57465591c2ded1afac2ab484d/third_party/WebKit/LayoutTests/external/wpt/html/dom/documents/dom-tree-accessors/document.forms-expected.txt
[delete] https://crrev.com/9e319197e596a8b57465591c2ded1afac2ab484d/third_party/WebKit/LayoutTests/fast/dom/htmlallcollection-enumerated-properties-expected.txt
[delete] https://crrev.com/9e319197e596a8b57465591c2ded1afac2ab484d/third_party/WebKit/LayoutTests/fast/dom/htmlallcollection-enumerated-properties.html
[delete] https://crrev.com/9e319197e596a8b57465591c2ded1afac2ab484d/third_party/WebKit/LayoutTests/fast/dom/htmlcollection-enumerated-properties-expected.txt
[delete] https://crrev.com/9e319197e596a8b57465591c2ded1afac2ab484d/third_party/WebKit/LayoutTests/fast/dom/htmlcollection-enumerated-properties.html
[delete] https://crrev.com/9e319197e596a8b57465591c2ded1afac2ab484d/third_party/WebKit/LayoutTests/fast/dom/htmlformcontrolscollection-enumerated-properties-expected.txt
[delete] https://crrev.com/9e319197e596a8b57465591c2ded1afac2ab484d/third_party/WebKit/LayoutTests/fast/dom/htmlformcontrolscollection-enumerated-properties.html
[delete] https://crrev.com/9e319197e596a8b57465591c2ded1afac2ab484d/third_party/WebKit/LayoutTests/fast/dom/htmloptionscollection-enumerated-properties-expected.txt
[delete] https://crrev.com/9e319197e596a8b57465591c2ded1afac2ab484d/third_party/WebKit/LayoutTests/fast/dom/htmloptionscollection-enumerated-properties.html
[modify] https://crrev.com/231585efb676b7d99748e29f9049b29bf1f84f67/third_party/WebKit/Source/core/html/HTMLAllCollection.idl
[modify] https://crrev.com/231585efb676b7d99748e29f9049b29bf1f84f67/third_party/WebKit/Source/core/html/HTMLCollection.idl

Status: Fixed (was: Started)

Sign in to add a comment