Class string of default iterator object seems to have been accidentally changed?

[TimothyGu]

Commit d433e0a (#198) has a commit message of the following:

commit d433e0a6ae4f81b35364e0890694a8b094a5f669
Author:     Tobie Langel <[email protected]>
AuthorDate: Wed Oct 26 09:13:35 2016 +0200
Commit:     GitHub <[email protected]>
CommitDate: Wed Oct 26 09:13:35 2016 +0200

    Use Bikeshed’s algorithm construct. (#198)
    
    * Marks up all algorithms with <div algorithm>.
    * Gives a name to algorithms which don't include a <dfn>.
    * Fix algorithms that generate Bikeshed warnings, notably by:
        * Replacing prose calls to [[Call]] method by Call(args).
        * Fixing incorrectly named variables.
        * Adding |T|, |U|, |A|, |map|, |object|, |I| and |id| to Ignored Vars.

However, it contains the following normative change (white space differences ignored):

 When a [=default iterator object=] is first created,
 its index is set to 0.
 
 The [=class string=] of a [=default iterator object=] for a given [=interface=]
 is the result of concatenting the [=identifier=] of the [=interface=]
-and the string “ Iterator”.
+and the string “Iterator”.
 
 
 <h5 id="es-iterator-prototype-object">Iterator prototype object</h5>

It seems to have been accidental, but there are already some implementations using the new changed spec (admittedly, most of which were written by me).

/cc @tobie

[tobie]

[annevk]

Maybe best to replace with concatenating X, U+0020 SPACE, and Y rather than X and " Y" for clarity.

[tobie]

(Mis)read your message before morning coffee and thought I had introduced the whitespace. Sorry.

Removing the whitespace was a deliberate fix, as no other class string contained whitespace (including the class string of the iterator prototype object), and I assumed it was a typo.

I think we should leave it as such unless there's a good reason not to.

(As an aside, this should probably have been in a PR of its own or at least be mentioned in the commit message.)

[tobie]

Mmm. On further inspection, the whitespace seems to have been originally deliberate given the SessionManager example we (still) have in the spec. :-(

Sounds like we need to figure out what browsers are doing, figure out what we want to do and if we add back the whitespace, do as Anne suggests above (plus ideally add a note to explain why we have a convention departure for that particular case).

[tobie]

The plot thickens. Originally, "Iterator" was used for both the class strings of the iterator prototype object and the default iterator object: e74d135a

It was subsequently changed to " Iterator" for default iterator object only in: 1bc1e29. This is also the commit where the example was changed from SessionManagerIterator to SessionManager Iterator.

[TimothyGu]

In ES there is a space, e.g.Map Iterator

[tobie]

Guess I should have looked there before "fixing" this. :-/

[tobie]

So this is predictably messy.

Given:

const it = new URLSearchParams().values();
const itProto = Object.getPrototypeOf(it);

	FF Nightly	Chrome Canary	WebKit Nightly
it[Symbol.toStringTag];	undefined	"Iterator"	undefined
Object.prototype.toString.apply(it);	[object URLSearchParamsIterator]	[object Iterator]	[object URLSearchParams Iterator]
itProto[Symbol.toStringTag];	undefined	"Iterator"	undefined
Object.prototype.toString.apply(itProto);	[object URLSearchParamsIteratorPrototype]	[object Iterator]	[object URLSearchParams Iterator]

In contrast, given:

const m = new Map().values();
const mProto = Object.getPrototypeOf(m);

	FF Nightly	Chrome Canary	WebKit Nightly
m[Symbol.toStringTag];	"Map Iterator"	"Map Iterator"	"Map Iterator"
Object.prototype.toString.apply(m);	[object Map Iterator]	[object Map Iterator]	[object Map Iterator]
mProto[Symbol.toStringTag];	"Map Iterator"	"Map Iterator"	"Map Iterator"
Object.prototype.toString.apply(mProto);	[object Map Iterator]	[object Map Iterator]	[object Map Iterator]

[TimothyGu]

I'd like a resolution on this soon as Web compatibility concerns have not yet kicked in. I am of the opinion that adding the space back in is the way to go here, for consistency with ES. I remember @bzbarsky was of the opinion that prototype objects should always have different class strings as instances but I could well be misremembering (thus looping him in). I do have to say though that "URLSearchParams IteratorPrototype" looks a bit odd since %IteratorPrototype% is something else.

FWIW Node.js uses "URLSearchParamsIterator" but we have flexibility for change.

[tobie]

I'd like a resolution on this soon as Web compatibility concerns have not yet kicked in.

Agreed.

I am of the opinion that adding the space back in is the way to go here, for consistency with ES.
I remember @bzbarsky was of the opinion that prototype objects should always have different class strings as instances but I could well be misremembering (thus looping him in).

As far as WebIDL is concerned, this is only specified for interface prototype objects. So technically, the iterator object's prototype, which isn't an interface prototype object, shouldn't be suffixed with "Prototype".

I do have to say though that "URLSearchParams IteratorPrototype" looks a bit odd since %IteratorPrototype% is something else.

Yeah. Overall, we should strive for more consistency with ES here, imho. This includes figuring out what to do with the "Prototype" suffix.

[tobie]

Additionally, see #54 and https://www.w3.org/Bugs/Public/show_bug.cgi?id=28244.

[domenic]

I think that even though in the general case @bzbarsky may not agree to the proposed solution to #54, i.e. #357, hopefully for iterator objects, which are explicitly designed to be compatible with ES, we can go the same way they do, and install a single symbol on the prototype with no "Prototype" suffix.

[tobie]

The proposed resolution would be the following behavior, right?

const it = new URLSearchParams().values();
it[Symbol.toStringTag];
// --> "URLSearchParams Iterator"
Object.prototype.toString.apply(it);
// --> "[object URLSearchParams Iterator]"
Object.getPrototypeOf(it)[Symbol.toStringTag];
// --> "URLSearchParams Iterator"
Object.prototype.toString.apply(Object.getPrototypeOf(it));
// --> "[object URLSearchParams Iterator]"

[domenic]

Yep!

[bzbarsky]

I can live with that solution for iterators, yes. In practice, getting their prototypes is enough of a PITA that people won't be passing them around, I hope and hence won't run into the branding issues that somewhat worry me for interface prototype objects.

Please make sure to file bugs on browsers when this lands?

[bzbarsky]

Did test/implementation bugs get filed?