Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enumeration order of interface members #432

Open
tobie opened this issue Sep 1, 2017 · 30 comments
Open

Enumeration order of interface members #432

tobie opened this issue Sep 1, 2017 · 30 comments

Comments

@tobie
Copy link
Collaborator

tobie commented Sep 1, 2017

Spec both says:

The order that members appear [on an interface] has significance for property enumeration in the ECMAScript binding.

and

The order of appearance of an interface definition and any of its partial interface definitions does not matter.

This seems contradictory. Which one is it? And more precisely, how is enumeration order defined across partials if at all.

@annevk
Copy link
Member

annevk commented Sep 1, 2017

It's important to some extent for compatibility. E.g., we cannot change the order of some members on the 2D API as that would break sites. How much order is important in general is unknown and proposals have come up for randomizing it and seeing what breaks...

@tobie
Copy link
Collaborator Author

tobie commented Sep 1, 2017

So enumeration order matters between some members defined within the same interface or partial interface declaration. I see.

@domenic domenic mentioned this issue Oct 3, 2017
7 tasks
@tobie
Copy link
Collaborator Author

tobie commented Oct 4, 2017

So if we want to define this precisely, here's a proposal so there's some basis for discussion:

  1. Let |members| be a new [=ordered set=].
  2. Let |stack| be the result of invoking [=create an inheritance stack=] for [=interface=] |I|.
  3. [=iteration/While=] |stack| is not [=stack/empty=]:
    1. Let |interface| be the result of [=stack/popping=] from |stack|.
    2. [=set/Append=] [=For each|each=] |member| of |interface|'s
      [=interface member|members=] to |members|.
    3. [=For each=] |partialInterface| of |interface| declared in the same specification as |interface|,
      in the order in which it appears in the specification:
      1. [=set/Append=] [=For each|each=] |member| of |partialInterface|'s
        [=interface member|members=] to |members|.
    4. [=For each=] |specification| that declares a [=partial interface=] of |interface|,
      lexicographically ordered by |specification|'s short name:
      1. [=For each=] |partialInterface| of |interface| declared in |specification|,
        in the order in which it appears in |specification|:
        1. [=set/Append=] [=For each|each=] |member| of |partialInterface|'s
          [=interface member|members=] to |members|.
    5. [=For each=] |mixin| [=included=] by |interface|, declared in the same specification as |interface|,
      in the order in which it appears in the specification:
      1. [=set/Append=] [=For each|each=] |member| of |mixin|'s
        [=mixin member|members=] to |members|.
      2. [=For each=] |partialMixin| of |mixin| declared in the same specification as |mixin|,
        in the order in which it appears in the specification:
        1. [=set/Append=] [=For each|each=] |member| of |partialMixin|'s
          [=mixin member|members=] to |members|.
      3. [=For each=] |specification| that declares a [=partial mixin=] of |mixin|,
        lexicographically ordered by |specification|'s short name:
        1. [=For each=] |partialMixin| of |mixin| declared in |specification|,
          in the order in which it appears in |specification|:
          1. [=set/Append=] [=For each|each=] |member| of |partialMixin|'s
            [=mixin member|members=] to |members|.
    6. [=For each=] |specification| that declares a [=mixin=] [=included=] by |interface|,
      lexicographically ordered by |specification|'s short name:
      1. [=For each=] |mixin| [=included=] by |interface| in |specification|,
        in the order in which it appears in |specification|:
        1. [=For each=] |member| of |mixin|'s [=mixin member|members=]:
          1. [=set/Append=] |member| to |members|.
        2. [=For each=] |partialMixin| of |mixin| declared in the same specification as |mixin|,
          in the order in which it appears in the specification:
          1. [=set/Append=] [=For each|each=] |member| of |mixin|'s
            [=mixin member|members=] to |members|.
        3. [=For each=] |specification| that declares a [=partial mixin=] of |mixin|,
          lexicographically ordered by |specification|'s short name:
          1. [=For each=] |partialMixin| of |mixin| declared in |specification|,
            in the order in which it appears in |specification|:
            1. [=set/Append=] [=For each|each=] |member| of |partialMixin|'s
              [=mixin member|members=] to |members|.
  4. Return |members|.

@bzbarsky
Copy link
Collaborator

bzbarsky commented Oct 4, 2017

Interfaces don't really have a concept of which specification they're declared in attached to them. And in particular:

  1. On a text level which specification is mutable, and sometimes there are multiple specifications that do it at once. Right now that's all OK as long as they all agree.
  2. Right now, just concatenating all IDL together is a reasonable (and possibly desirable) thing to do.

Likewise "order within a specification" may not be a very well defined concept...

It's not clear to me that "|specification|'s short name" is either stable or unique....

In general, other than the arc/arcTo thing I'm not obviously aware of people depending on the order, fwiw.

@tobie
Copy link
Collaborator Author

tobie commented Oct 4, 2017

So are you suggesting we should just no spec this for now?

@TimothyGu
Copy link
Member

Or we can just enforce the relative order of members declared in the definition of an interface in a single IDL fragment, though that does cause questions like this:

interface A {
  DOMString a();
  DOMString b();
  DOMString a(DOMString arg);
};

@tobie
Copy link
Collaborator Author

tobie commented Oct 4, 2017

Or we can just enforce the relative order of members declared in the definition of an interface in a single IDL fragment, though that does cause questions like this:

That doesn't define clearly what "in order" means in step 2 of collect attribute values.

@TimothyGu
Copy link
Member

@tobie Well, can't we leave inter-IDL fragment order undefined/implementation-defined? Like isn't relative order within the IDL fragment all we really care about in real life?

@tobie
Copy link
Collaborator Author

tobie commented Oct 4, 2017

Well, can't we leave inter-IDL fragment order undefined/implementation-defined?

I don't really have a strong opinion on this beyond wanting to resolve the current contradiction in the spec and needing to specify mixin member order in host interfaces.

Like isn't relative order within the IDL fragment all we really care about in real life?

For testing JSON order, that's sort of painful, but maybe nobody really cares about JSON order.

@bzbarsky
Copy link
Collaborator

bzbarsky commented Oct 4, 2017

For the case @TimothyGu raises (overloads not next to each other), all hope is lost, kinda, because they could in fact be in different IDL fragments too. We could order them by "which one appears first in the global ordering we haven't figured out how to establish"....

I would probably be OK with ensuring things in a given IDL fragment stay in order. As for JSON... people shouldn't care about order there, but who knows whether they do. :(

@annevk
Copy link
Member

annevk commented Oct 5, 2017

Couple ideas:

  1. A simple global ordering could be lexicographical with the key being the contents of each of the fragments that end up being combined into one. It's definitely not great and if we organize specifications differently there might be some observable side effects.
  2. Another thing we could do is require random order between fragments, but specified order for a given fragment. That complicates testing and implementation more than the former strategy I think.
  3. We could continue to not care and hope the problem doesn't spread.

@bzbarsky
Copy link
Collaborator

bzbarsky commented Oct 5, 2017

So in practice, how do web browsers get these IDL fragments into their IDL parser?

Presumably, they put them into files somehow. Is this a manual or automated process? Is the "fragment identity" preserved in any way during this process?

That is, having ordering depend on the concept of "IDL fragment" assumes that this concept even exists in practice in implementations. If it doesn't and would have to be painstakingly maintained by hand, then chances are it wouldn't get maintained....

@tobie
Copy link
Collaborator Author

tobie commented Oct 5, 2017

Here's what I think Web developers might reasonably expect order to be:

  1. members are ordered alphabetically (case insensitive), just like in dev tools,
  2. members are ordered lexicographically, and
  3. members are ordered as listed in the spec (without really thinking that some APIs are split-up through partials, mixins, and different documents).

I have no idea what the perf cost of (1) and (2) would be. I find them sort of attractive if they're not too expensive.

The problem with (3) is that we're unclear what it means precisely. Could we settle on something were we say:

  • order within a single definition is respected,
  • order in between definition is vendor-specific,
  • overloads across multiple definitions might show up with any of those in a vendor specific way.

@bzbarsky
Copy link
Collaborator

bzbarsky commented Oct 5, 2017

I have no idea what the perf cost of (1) and (2) would be.

I can't speak to other implementations, but in Gecko this would be a compile-time cost, not a runtime cost. This is also how dictionary members are already ordered.

Though, actually, that's not quite true. Gecko sets up properties on prototypes in "batches", operations separately from attributes, for various reasons. So right now in Gecko ordering is IDL order (ignoring the multiple fragment thing) within each of operations and attributes, but operations always come before attributes. That said, ordering within each of those lexicographically or so would be easy.

The problem is that the arc/arcTo thing we ran into in https://bugzilla.mozilla.org/show_bug.cgi?id=623437 needed arcTo to come before arc.

@littledan
Copy link
Collaborator

What are the next steps to make progress on this issue? The question came up again in #675. @bzbarsky The issues you point out with the above proposals seem pretty significant; it doesn't sound like WebIDL fragments are preserved contiguous in general, or that we have well-defined short names for specifications. Any more ideas?

How should we proceed with defining new "partial" constructs in WebIDL? Should we hold off, due to the ambiguity, until we have a solution to this issue?

@annevk
Copy link
Member

annevk commented Mar 4, 2019

I'm leaning toward:

  1. Order within a block (interface / partial interface) is preserved.
  2. We compute an order key for each block (e.g., concatenate all member names?).
  3. We order the blocks and then combine them in order.

@littledan
Copy link
Collaborator

littledan commented Mar 4, 2019

@annevk That plan sounds good to me. I like how it's insensitive to implementation-specific extended attributes, whitespace differences, etc., and how it's easy to run in your head. I don't think people will run into the theoretical ambiguity (when static members may share names with non-static members).

To spell it out a bit further, the order could be:

  1. The main interface (or namespace or module)
  2. Partial interfaces (sorted by the concatenation of names)
  3. Included interface mixins (sorted by the name of the interface mixin), closing Order of includes statements is not a well-defined concept #473

Seems like we have room to change things here, as browsers disagree with each other in enumeration order already. For example, in Document.prototype, Chromium and Webkit put implementation first, whereas Gecko puts getElementsByTagName first--and this is just in the "easy case" of things declared in the interface itself.

@Ms2ger
Copy link
Member

Ms2ger commented Mar 4, 2019

Fwiw, I'm pretty sure Gecko defines methods and attributes separately, so we'd have to look at the relative order within those groups.

@domenic
Copy link
Member

domenic commented Mar 4, 2019

I think the spec also defines methods and attributes separately!

@littledan
Copy link
Collaborator

littledan commented Mar 4, 2019

Yeah, looks like attributes come before operations, per step 10 of https://heycam.github.io/webidl/#dfn-interface-prototype-object , so I guess the spec disagrees with Firefox here. Was this intentional? I wonder if this text was written before we cared about enumeration order.

I would've expected that things are interspersed. In a JavaScript class, with methods, getters and setters, they would be.

@domenic
Copy link
Member

domenic commented Mar 4, 2019

The history here is, to my recollection, that originally all attributes and operations were added declaratively, in no specified order. I introduced the first imperative installation of properties with namespaces, and encouraged @tobie to modernize other constructs in that way. Recently @Ms2ger has pushed that work much further. (Perhaps finished it?) I don't recall us ever stopping to consider enumeration order.

I think the extra thing to consider here is implementation complexity or precedent. I think many implementations may have monomorphic, type-by-type loops like the spec does, instead of for (member of members) { member.define(theClass) where .define() is polymorphic on the type of member.

@Ms2ger
Copy link
Member

Ms2ger commented Mar 4, 2019

Gecko's implementation has been doing methods, then attributes (and then constants) since first using WebIDL in 2012 (https://hg.mozilla.org/mozilla-central/rev/1bdb337e3136#l27.69; I don't think we cared about enumeration order at the time.

@bzbarsky
Copy link
Collaborator

bzbarsky commented Mar 4, 2019

Switching the relative order of attributes and methods easy in Gecko.

Ordering each of those sets in some order like what #432 (comment) describes is work (we'd have to have a lot more complexity and state in the "build the interface member list" code than we do now), but pretty doable.

Having to have an order that interleaves attributes and methods (whether that's on a per-source basis or within sources, where a source is a mixin or partial interface definition) would be a big pain implementation-wise.

I guess mixins can't include other mixins, right? That simplifies things a bit.

Within a mixin, presumably partials are also handled by the same approach as for interfaces, but affecting the overall order of mixins?

Concatenation of names does not provide unique sort keys. Consider:

  partial interface A {
    readonly attribute long a;
    readonly attribute long bc;
  };
  partial interface A {
    readonly attribute long ab;
    readonly attribute long c;
  };

What we could do instead if sort by just first member name, which should be unique. Especially if we prefix the name with "Regular", "Static", "Both" (for constants) or something, to deal with the static and non-static sharing names case.

One thing worth thinking about here: Ideally the order of partials would not change when new things are added to the partials. I think we get that if we use the first member as key, as long as people add things to the end...

@s0a9d4i2jia5

This comment has been minimized.

@bathos
Copy link
Contributor

bathos commented Oct 19, 2022

Does anyone know if there’s been any further “news” on this front since this issue last got discussed?

It's important to some extent for compatibility. E.g., we cannot change the order of some members on the 2D API as that would break sites.

Spot checking, I haven’t turned up any prototype with more than two IDL-defined properties whose relative order is the same in both Blink and Gecko. I’m not sure if “2D” refers to CanvasRenderingContext2D or not, but if it does...

Side by side of Edge & Firefox devtools showing very different enumeration order for properties of CanvasRenderingContext2D.prototype

...then it looks like code that works in one of those browsers now but wouldn’t if the order changed would also be code that already doesn’t work in the other browser (unless it’s branching on user agent sniffing or just very “lucky”?)

It seems like any observable runtime behavior that would arise from use of partials is at odds with their “specification editorial aide” role. It also seems undesirable for mixin usage and the “Web IDL source text order” of members of any of these constructs to have runtime consequences. The current cross-browser chaos seems like a substantial source of entropy that could be avoided.

If there were a canonical order for “flattened” named members, lexicographic or otherwise, I don’t think anything about the imperative steps would need to change. A fully-defined order that doesn’t teleport artifacts of editorial decisions, organizational structure, and the directory and file naming of a browser’s source text into the runtime seems very worthwhile regardless of what that specific order ends up being, so e.g. “attributes come before operations” doesn’t matter cause they always would.

Cases where web-compat demands a unique order seem very [LegacySomething] ... unless such cases are a lot more common than I’d have hoped?

@annevk
Copy link
Member

annevk commented Oct 19, 2022

Not sure. You can search for ARC-ORDER in the HTML standard's source and find https://bugzilla.mozilla.org/show_bug.cgi?id=623437 which points to http://js1k.com/2010-xmas/demo/865. ARC-ORDER was added in whatwg/html@771f591. That demo no longer seems to work in every browser.

https://hg.mozilla.org/mozilla-central/rev/0399ff8f614c verifies that arcTo comes first although it wrongly says "Should see arc before arcTo".

(And in whatwg/html@e25d3f9#diff-41cf6794ba4200b839c53531555f0f3998df4cbb01a4d5cb0b94e3ca5e23947d ARC-ORDER got added to a few more lines, presumably due to copypasta.)

@bathos
Copy link
Contributor

bathos commented Oct 19, 2022

Nice, thanks for the links/context!

That demo no longer seems to work in every browser.

Yep, sadly I got TypeError: fl is not a function in both Edge and FF nightly, but it did at least reach the code where the order-sensitive stuff comes from, which is this: for($ in a)a[$[0]+($[6]||$[2])]=a[$];. We can ungolf this to...

for (let originalPropertyKey in renderingContext) {
  let charA = originalPropertyKey[0];
  let charB = originalPropertyKey[6] || originalPropertyKey[2];
  let newImprovedPropertyKey = charA + charB;

  renderingContext[newImprovedPropertyKey] = renderingContext[originalPropertyKey];
}

Though order-sensitive, order doesn’t seem to describe the expectation it depended on. It depended on no new member ever being added to CanvasRenderingContext2D whose name’s first and seventh-or-third characters were the same as those of select members which existed at the time it was written.

I’m kinda surprised that “if a property name’s first character is a and its third character is r, any new property added to the same object with a name for which that’s also true will be enumerated before the original ar member” was deemed an implicit guarantee of the web platform 😅!

As the issue wasn’t really ordering itself, it’s not surprising that it happened again. The reason fl is not a function is that the filter attribute (whose value is a string) “broke the invariant” that the fill method was the only-or-last fl :)

@annevk
Copy link
Member

annevk commented Oct 20, 2022

Ooh nice work, I guess that means we should give up on ARC-ORDER at least and in theory could think of ways to resolve this issue.

Even more attractive than #432 (comment) would be lexicographical order I think.

@domenic
Copy link
Member

domenic commented Oct 20, 2022

Things that might be hard for implementations:

  • Interleaving attributes, operations, and constants
  • Interleaving different partials and mixins

So I think this gives a few proposals:

  • Total lexicographical order: direct interface members + mixins' members + partials' members, all mixed together, without regard for type, get sorted lexicographically and installed in order.
  • Split by type: direct interface attributes + mixins' attributes + partials' attributes all mixed together and sorted lexicographically, then the same for constants, then the same for operations.
  • Split by type and then source: something like
    • First direct interface attributes, ordered lexicographically
    • Then mixins' attributes, ordered lexicographically within each mixin and with the mixins ordered lexicographically by mixin name
    • Then partials attributes, ordered lexicographically within each mixin and... lexicographically within each partial by the first attribute or first operation in the partial?
    • Same process for constants
    • Same process for operations
  • Split by source and then type
    • First direct interface attributes, ordered lexicographically
    • Then direct interface constants, ordered lexicographically,
    • ... etc.

(Note: I picked the ordering attributes, constants, operations arbitrarily. Gecko apparently does operations, attributes, constants and Chromium does attributes, constants, operations.)

Implementer input welcome on any preference among these. Based on some quick testing of Reflect.ownKeys(Element.prototype), it seems like Chromium does something like "split by type and then source", although ordering within each source or among sources does not seem lexicographical. Gecko seems to mix together partials but not mixins...

@bathos
Copy link
Contributor

bathos commented Oct 20, 2022

@annevk Just to be safe since it’s so close to Halloween, I made a page which should keep the ARC-ORDER demo from returning from the dead, at least if I recited the hex correctly.

@domenic I’d put in a bid for type+lexicographic. (I’m not an implementor in the browser sense, but I do work on Web IDL stuff where this would probably end up being the most straightforward change.) I also don’t think the last two options are great because it seems to me that use of mixins and partials shouldn’t have runtime consequences if possible — e.g. if an editor chooses to move some members into a partial in order to interleave definitions with prose text, it’s odd/brittle for that to alter observable aspects of what’s being specified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

10 participants