1317658 - [HTMLConstructor] Fallback to the interface prototype object from newTarget's realm if cannot get prototype from newTarget

Status: RESOLVED FIXED

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

Description

[Edgar Chen [:edgar]]

+++ This bug was initially created as a follow-up of Bug #1274159 comment #47 +++

>+                  JS::Rooted<JSObject*> newTargetUnwrapped(cx, js::CheckedUnwrap(newTarget));
>+                  { // enter newTarget's compartment
>+                    JSAutoCompartment ac(cx, newTargetUnwrapped);
>+                    desiredProto = GetProtoObjectHandle(cx);

This doesn't actually implement what the spec says to do.  The spec says to use GetFunctionRealm() [1], which is not the same thing as what you have here (e.g. in the case of scripted callable proxies whose target is not same-compartment with the proxy, or bound functions, etc).

[1] See step 7.1 of https://html.spec.whatwg.org/multipage/dom.html#htmlconstructor

Comment 1

[Till Schneidereit [:till]]

Bug 962053 is only about the content-visible reflection of realms, not the concept itself, so it doesn't block this.

Comment 2

[Boris Zbarsky [:bzbarsky]]

I don't think this depends on bug 1317416 either, really.

What this does need to depend on is SpiderMonkey having API that does GetFunctionRealm.  Right now there is no such thing; presumably this needs a SpiderMonkey bug?

Comment 3

[André Bargull [:anba]]

bug 1317416 is the same kind of issue for JavaScript functions and built-ins. So when bug 1317416 gets fixed in SpiderMonkey, we can provide an API for GetFunctionRealm/GetPrototypeFromConstructor at the same time. (Strictly speaking it's not necessary to wait for bug 1317416 before adding GetFunctionRealm/GetPrototypeFromConstructor to JSAPI, though.)

Comment 4

[Edgar Chen [:edgar]]

I guess I misunderstood bug 962053.

(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #2)
> I don't think this depends on bug 1317416 either, really.
> 
> What this does need to depend on is SpiderMonkey having API that does
> GetFunctionRealm.  Right now there is no such thing; presumably this needs a
> SpiderMonkey bug?

Exactly, what this needs is a SpiderMonkey API for GetFunctionRealm [1].

[1] https://tc39.github.io/ecma262/#sec-getfunctionrealm

Hi Till, is there any existing bug for this?

Comment 5

[Andrew Overholt [:overholt]]

John, do we need to do something here to ship our first implementation of Custom Elements?

Comment 6

[John Dai[:jdai]]

It’s a corner case to use Custom Elements. The bug depends on SpiderMonkey API for GetFunctionRealm, I guess we can't do anything in here.

Comment 7

[Hsin-Yi Tsai (she/her) [:hsinyi]]

My interpretation of comment 6 is this is not a blocker for shipping 1st impl. of Custom elements, moving to P3.

Comment 8

[Boris Zbarsky [:bzbarsky]]

I agree: this should not block shipping custom elements.

Specifically, this is only a problem when the newtarget (i.e. the custom element constructor) is one of the things mentioned in comment 0: a scriptable Proxy for a function from a different global, or a bound function created by calling bind() in a different global than the underlying function.  While it's possible to write testcases like this, no one in their right mind using custom elements would ever create their constructors this way, I expect....

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1317658 part 1. Remove unnecessary isConstructing check from GetDesiredProto, now that all callers ensure that we are in fact constructing. r=edgar

Binding constructors started doing that in bug 979591.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1317658 part 2. Pass enough information to bindings' GetDesiredProto to allow it to return the prototype the spec says it should. r=edgar

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/018c39d438c5
part 1.  Remove unnecessary isConstructing check from GetDesiredProto, now that all callers ensure that we are in fact constructing.  r=edgar
https://hg.mozilla.org/integration/autoland/rev/4a8a36c44e0f
part 2.  Pass enough information to bindings' GetDesiredProto to allow it to return the prototype the spec says it should.  r=edgar

Comment 12

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/018c39d438c5
https://hg.mozilla.org/mozilla-central/rev/4a8a36c44e0f

Comment 13

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/16701 for changes under testing/web-platform/tests