[heycam/webidl] Named properties visibility algorithm leads to ES invariant violation (#152)

Consider the testcase at <https://bug1297411.bmoattachments.org/attachment.cgi?id=8784910>.  This does the following:

1. Define a non-configurable non-writable property on an [OverrideBuiltins] interface.
2. Modify the object so it would have a named prop with the same name (e.g. add an input to a form).

Per the letter of the IDL spec right now, this leads to an ES invariant violation: getOwnPropertyDescriptor after step 2 lands at http://heycam.github.io/webidl/#named-properties-object-getownproperty and then ends up in http://heycam.github.io/webidl/#dfn-named-property-visibility which returns true in step 4, so the returned descriptor ends up claiming a configurable property whose value is the input element.

Current browser behavior is as follows:

1. Safari  more or less follows this broken spec as written (except it claims the named prop is not configurable and not writable, but has its value change anyway).
2. WebKit nightly follows the spec exactly for forms but has the Safari behavior for document.
3. Chrome matches WebKit nightly for forms but preserves the invariant for document in a way I'll describe below.
4. Gecko preserves the invariant in a way I'll describe below.
5. Edge both preserves the invariant for document in a way I'll describe below and violates it for forms in exactly the way Safari does.

I see two conceivable ways of preserving the invariant here:

1. Switch the order of steps 4 and 5 in <http://heycam.github.io/webidl/#dfn-named-property-visibility> so that named props don't shadow own properties even for [OverrideBuiltins] interfaces.  This is the approach Gecko takes for both form and document and seems to be the approach Edge takes for document.  If this is done, then I believe steps 1 and 2 of the algorithm become unnecessary, by the way, but only because we never have objects with onforgeable properties that are being set up after they have named props...
2. Replace steps 1 and 2 with a check for an own non-configurable property with the given name and return false if found.  This seems to be the approach Blink takes for document.  Again, this is ok because we never have objects with unforgeable properties that are being set up after they have named props.  If we wanted to have this be conceptually sound in general we'd need to leave steps 1 and 2 (which might be combinable?) and add a step between the current steps 2 and 3.

The two options lead to quite different behavior on the testcase at https://bugzilla.mozilla.org/attachment.cgi?id=8784908 -- option 1 has the expando shadow the named prop, and option 2 has the named prop shadow the expando.

In practice, I expect both options are web compatible enough and I think option 1 is simpler to both specify and implement.  But I would be interested in hearing what other people think.

@cdumez, @domenic, @travisleithead, thoughts?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/heycam/webidl/issues/152