Intent to Implement and Ship: make HTMLCollection and HTMLAllCollection's named properties not enumerable

[Raphael Kubo da Costa]

Intent to Implement/Ship
========================
Make the named properties in HTMLCollection and HTMLAllCollection (as
well as IDL interfaces that inherit from them, namely
HTMLOptionsCollection and HTMLFormControlsCollection) not enumerable.

Contact emails
==============
raphael.ku...@intel.com

Specs
=====
https://dom.spec.whatwg.org/#interface-htmlcollection
https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#the-htmlallcollection-interface

Summary
=======
The [LegacyUnenumerableNamedProperties] extended attribute was added to
WebIDL in early 2016 with https://github.com/heycam/webidl/pull/91.

It was then added to HTMLCollection and HTMLAllCollection in the
following commits:
https://github.com/whatwg/dom/commit/742e8c15312ae447580d2fd5498a02d0902d5ff3
https://github.com/whatwg/html/commit/07f6c7f058c57d582db1aced6741cea1ed63ccf7

Even before that, those interfaces' named properties were declared not
enumerable back in 2014:
https://github.com/whatwg/dom/commit/c812a0aafaa92460ffdd333418e03f2790fa742a
https://github.com/whatwg/html/commit/c1de4765a8ba3671cbb70a34cf5bdf3ee93572c6

Note that these commits also add the [LUNP] extended attribute to other
interfaces; I'm tackling them separately and not including them in this
CL because we were already trying to achieve the same thing there with
the non-standard [NotEnumerable] attribute. HTML{All}Collection are the
exceptions that need an Intent thread.

When used in an IDL interface, it makes all its named properties
unenumerable. User-visible effects of this change include:
- These properties are skipped by calls to Object.keys(),
Object.values() and Object.entries() as well as for-in loops.
- Calling Object.getOwnPropertyDescriptor() will return a descriptor
with its "enumerable" property set to false.

Object.getOwnPropertyDescriptors() and Object.getOwnPropertyNames()
continue to include these named properties just like before, as they are
not affected by the enumerability of a given property.

Motivation
==========
Interoperability with other engines and spec compliance. Like I said
above, some of our IDL interfaces (NamedNodeMap, for example) already
behaved almost like the spec mandated while others such as
HTMLCollection did not, which leads to unnecessary confusion.

Interoperability risk
=====================
Edge:
- EdgeHTML 14.14393 still marks HTMLCollection's named properties as
enumerable. According to wpt.fyi, this hasn't changed in Edge 15. It
is unclear if there is work on it.

Firefox:
- Has shipped with [LegacyUnenumerableNamedProperties] since Firefox 49.
https://bugzilla.mozilla.org/show_bug.cgi?id=1270349
- Made HTMLAllCollection's properties unenumerable in Firefox 32 (at
least).
https://bugzilla.mozilla.org/show_bug.cgi?id=874212
- Made HTMLCollection's properties unenumerable in Firefox 31.
https://bugzilla.mozilla.org/show_bug.cgi?id=843840

WebKit:
- Added [LegacyUnenumerableNamedProperties] to its IDLs in Jan 2017.
https://trac.webkit.org/changeset/210667/webkit
- I don't have a Mac at hand to properly test this right now, but I can
see some code making named properties default to not enumerable from
2015 and 2013 (my Perl- and JSC-fu aren't very good though).
https://trac.webkit.org/changeset/154253/webkit
https://trac.webkit.org/changeset/188663/webkit

Compatibility risk
==================
On the one hand, there are no measurements in place to check if anyone's
relying on those named properties being enumerable even though they
should not have been enumerable since 2014, which can bring another
flood of angry emails like the XHR.getAllResponseHeaders() issue we had
with M60.

On the other hand, it looks like both Gecko and WebKit have done this
for years now, in a way or another.

Will this feature be supported on all six Blink platforms?
==========================================================
Yes.

Is this feature fully tested by web-platform-tests?
===================================================
Yes.

Tracking bug(s)
===============
https://bugs.chromium.org/p/chromium/issues/detail?id=703990
https://bugs.chromium.org/p/chromium/issues/detail?id=719703

Requesting approval to ship?
============================
Yes, hopefully in M63; I'll also add a chromestatus.com entry about this
if this intent's approved.

[Raphael Kubo Da Costa]

ping?

On Friday, September 8, 2017 at 1:08:59 PM UTC+2, Raphael Kubo Da Costa wrote:

Intent to Implement/Ship
========================
Make the named properties in HTMLCollection and HTMLAllCollection (as
well as IDL interfaces that inherit from them, namely
HTMLOptionsCollection and HTMLFormControlsCollection) not enumerable.

Contact emails
==============

raphael.kubo.da.costa@intel.com

[PhistucK]

After testing Safari using BrowserStack, it does not seem to have ever been a WebKit issue.

Chrome started making it enumerable in Chrome 34 (2014).

Test case -

data:text/html,<img src="" name="crap"/><script>for (x in document.images) alert(x)</script>

Is it possible add a use counter for cases where this change will reduce the number of for-in iterations?

☆PhistucK

On Wed, Sep 13, 2017 at 11:53 AM, Raphael Kubo Da Costa <raphael.ku...@intel.com> wrote:

ping?

On Friday, September 8, 2017 at 1:08:59 PM UTC+2, Raphael Kubo Da Costa wrote:

Intent to Implement/Ship
========================
Make the named properties in HTMLCollection and HTMLAllCollection (as
well as IDL interfaces that inherit from them, namely
HTMLOptionsCollection and HTMLFormControlsCollection) not enumerable.

Contact emails
==============

raphael.ku...@intel.com

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/dfba1256-ab97-4263-8099-2be72711d533%40chromium.org.

[Kubo Da Costa, Raphael]

On Wed, 2017-09-13 at 14:34 +0300, PhistucK wrote:

Is it possible add a use counter for cases where this change will reduce the number of for-in iterations?

The for-in handling code is all in V8; all we do is return an integer that corresponds to a v8::PropertyAttribute, so I think the most we could do either log all calls to the query callback we generate in the bindings or add some (possibly ugly) plumbing to make each interface track this on the C++ side.

[PhistucK]

Sorry, I shortened the question a bit, I guess.

Is it possible to add use counter for cases where the this would affect the number of enumerated properties?

☆PhistucK

[Kubo Da Costa, Raphael]

On Wed, 2017-09-13 at 17:22 +0300, PhistucK wrote:

Sorry, I shortened the question a bit, I guess.

Is it possible to add use counter for cases where the this would affect the number of enumerated properties?

I think the problem's still the same: the finest glanularity we could use is "calls to the query callback for HTMLCollection and the others", as all the rest is handled by V8. While part of that number is indeed the number of cases the properties would no longer be enumerated, the same callback is used in calls to e.g. Object.getOwnPropertyDescriptors(), so there'd definitely be noise in the data.

[Chris Harrelson]

LGTM1

--

You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/1505313046.3821.12.camel%40intel.com.

[Dimitri Glazkov]

LGTM2

On Wed, Sep 13, 2017 at 10:54 AM Chris Harrelson <chri...@chromium.org> wrote:

LGTM1

On Wed, Sep 13, 2017 at 7:30 AM, Kubo Da Costa, Raphael <raphael.ku...@intel.com> wrote:

On Wed, 2017-09-13 at 17:22 +0300, PhistucK wrote:

Sorry, I shortened the question a bit, I guess.

Is it possible to add use counter for cases where the this would affect the number of enumerated properties?

I think the problem's still the same: the finest glanularity we could use is "calls to the query callback for HTMLCollection and the others", as all the rest is handled by V8. While part of that number is indeed the number of cases the properties would no longer be enumerated, the same callback is used in calls to e.g. Object.getOwnPropertyDescriptors(), so there'd definitely be noise in the data.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/1505313046.3821.12.camel%40intel.com.

--

You received this message because you are subscribed to the Google Groups "blink-dev" group.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAOMQ%2Bw_k%2Bv6oidFwkqurSu57iTazO0tsjpRik0hjWexD%2BGXr-w%40mail.gmail.com.

[Jochen Eisinger]

lgtm3

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAOtFfx4EjiWUr6T34vCcxfABt1gEp%2B59NEXrmHE9EpxNYXyOuA%40mail.gmail.com.