1297411 - named properties should shadow expando (own) props on [OverrideBuiltins] interfaces

Status: RESOLVED INVALID

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

See discussion at bug 1297304 comment 3.

Peter, Cameron, thoughts?

Comment 1

[Boris Zbarsky [:bzbarsky]]

This applies to [[Get]] and [[GetOwnProperty]].

Comment 2

[Peter Van der Beken [:peterv]]

Yeah, looks like a bug. I think we might have tried to remain backwards compatible, but my memory is hazy.

Comment 3

[Boris Zbarsky [:bzbarsky]]

I need to carefully double-check behavior for all our OverrideBuiltins interfaces in Safari to make sure we're not going to break sites...

Comment 4

[Boris Zbarsky [:bzbarsky]]

Actually, we only need to check the ones that don't have a named creator, which means I can ignore DOMStringMap and only consider HTMLDocument and HTMLFormElement.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Simple testcase for the behavior

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Slightly more interesting testcase

I started looking at our code for this and rememebered why it is we have the behavior we do.  It makes it simple to not violate the ES invariants.

This testcase shows that Safari totally violates them for <form> and document, and Blink violates them for <form>.  For document it doesn't violate them, but that means that in practice sometimes the named prop shadows the expando prop and sometimes not, depending on whether the expando prop is defined as configurable.

Given that, I think we should change http://heycam.github.io/webidl/#dfn-named-property-visibility by switching steps 4 and 5 (which basically aligns with Edge and our current impl), mark this bug invalid, and reopen bug 1297304.

Comment 7

[Boris Zbarsky [:bzbarsky]]

The other option is to keep steps 4 and 5 in the current order, but change step 1 to consider all non-configurable properties or something; that's closer to what Blink does with document, I think...

Comment 8

[Boris Zbarsky [:bzbarsky]]

Filed https://github.com/heycam/webidl/issues/152 to get this sorted out.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Invalid per spec update in https://github.com/heycam/webidl/issues/152 and https://github.com/heycam/webidl/pull/174