Bug 216533 – [WebIDL] %Interface%.prototype.constructor should be defined on [[Set]] receiver

• Status: RESOLVED
• Resolution: FIXED
• Component: Bindings

```
var p = Object.create(HTMLElement.prototype);
p.constructor = "foo";
console.log("!!!" + HTMLElement.prototype.constructor)
```

For Safari it prints:
```
!!!foo
```

For Chrome it prints:
```
!!!function HTMLElement() { [native code] }
```

For non HTML elements it works as expected.

[Radar WebKit Bug Importer]

<rdar://problem/69023868>

[Alexey Shvayka]

Created attachment 409037 [details]
WIP

[Alexey Shvayka]

Created attachment 409041 [details]
WIP

[Alexey Shvayka]

Created attachment 411057 [details]
Patch

[EWS Watchlist]

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

[Darin Adler]

Comment on attachment 411057 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=411057&action=review

> Source/JavaScriptCore/runtime/CustomGetterSetter.h:79
> +JS_EXPORT_PRIVATE Optional<bool> callCustomSetter(JSGlobalObject*, CustomGetterSetter::CustomSetter, bool isAccessor, JSObject* slotBase, JSValue thisValue, JSValue);

Sam Weinig has argued that Optional<bool> is too confusing a type to ever be used for anything. In this case, I must admit it’s not clear to me what the three different return values are and what they mean. Could just return a custom enumeration to make that clearer?

[Alexey Shvayka]

Created attachment 411796 [details]
Patch

Use TriState instead of Optional<bool>.

[Alexey Shvayka]

(In reply to Darin Adler from comment #6)

Thank you for review, Darin!

> Sam Weinig has argued that Optional<bool> is too confusing a type to ever be
> used for anything. In this case, I must admit it’s not clear to me what the
> three different return values are and what they mean. Could just return a
> custom enumeration to make that clearer?

Does TriState make it clearer?
It's nice that, unlike custom enumeration, it leaves the knowledge on how to handle missing CustomValue setter out of callCustomSetter().

[Alexey Shvayka]

(In reply to Alexey Shvayka from comment #8)
> It's nice that, unlike custom enumeration, it leaves the knowledge on how to
> handle missing CustomValue setter out of callCustomSetter().

This can also be achieved by passing something like CustomGetterSetter::Result::MissingCustomValueSetter.

[Darin Adler]

Comment on attachment 411796 [details]
Patch

Yes, I think the use of TriState makes this clearer. Not as elegant as Optional<bool> but clearer.

[EWS]

Committed r268710: <https://trac.webkit.org/changeset/268710>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 411796 [details].

[Alexey Shvayka]

(In reply to EWS from comment #11)
> Committed r268710: <https://trac.webkit.org/changeset/268710>

This change reduced WebCore --release binary size by 0.36% (296 KB), and also fixed DontEnum to be preserved when reassigning "constructor":

    HTMLElement.constructor = function() {};
    HTMLElement.propertyIsEnumerable("constructor"); // now: false (correct), was: true

https://bugs.webkit.org/show_bug.cgi?id=217916 adds a test case for this, and also fixes 2 super minor regressions introduced in r268710:

    1. When CustomValue structure property is reassigned, PropertyAttribute::CustomValue is not unset.
       This is unobservable since JSC relies on value being a CustomGetterSetter rather than checking attributes.
    2. When non-reified static setter-less CustomValue of a non-extensible structure is reassigned, TypeError is erroneously thrown.
       There are no such cases in the wild: "constructor" properties are always reified.