Allow constructing and subclassing EventTarget #467
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
domenic
added
needs implementer interest
|
What is the current API proposal? No parent chain? In general I'm supportive, just need to think a bit the API so that it fulfills the most common use cases. |
|
Yes; as noted in the pull request, +<p class="note">In the future we could allow custom <a>get the parent</a> algorithms. Let us know
+if this would be useful for your programs. For now, all author-created {{EventTarget}}s do not
+participate in a tree structure.</p> |
domenic
added a commit
to web-platform-tests/wpt
that referenced
this pull request
Jun 21, 2017
domenic
removed
the
needs tests
domenic
added a commit
to web-platform-tests/wpt
that referenced
this pull request
Jun 22, 2017
annevk
reviewed
Jun 28, 2017
dom.bs
Outdated
| @@ -944,6 +944,11 @@ and a <dfn export for=EventTarget>legacy-canceled-activation behavior</dfn> algo | |||
| are not to be used for anything else. [[!HTML]] | |||
|
|
|||
| <dl class=domintro> | |||
| <dt><code><var>target</var> = new EventTarget();</code> | |||
There was a problem hiding this comment.
We should link EventTarget here.
annevk
reviewed
Jun 28, 2017
dom.bs
Outdated
| must run these steps: | ||
|
|
||
| <ol> | ||
| <li>Return a new {{EventTarget}} object, using the default <a>get the parent</a> algorithm, and |
There was a problem hiding this comment.
<li><p> or maybe inline it in the previous step. It seems that you get a default get the parent and no behavior should maybe be a note as it's just a statement of fact.
annevk
approved these changes
Jun 28, 2017
|
Got tests, got 1 implementer support; waiting for a second. |
|
@annevk given #441 (comment), do you think this is mergeable? (Also want to review/merge the tests.) |
|
I confirmed on IRC that it's all good. I also reviewed the tests. |
|
For next time, it's worth noting the tests in the commit message I think. Also, did any browser bugs get filed? |
|
I was going to but then the build failed so I didn't have a spec to point them to. That should be fixed now so I can file Monday. Will update the thread then. |
|
Re-built the spec and it's showing up now, yay. Bugs:
|
bertogg
pushed a commit
to Igalia/webkit
that referenced
this pull request
Feb 17, 2020
https://bugs.webkit.org/show_bug.cgi?id=174313 Reviewed by Darin Adler. LayoutTests/imported/w3c: * web-platform-tests/WebIDL/ecmascript-binding/constructors-expected.txt: * web-platform-tests/dom/events/Event-subclasses-constructors-expected.txt: * web-platform-tests/dom/events/EventTarget-constructible.any-expected.txt: * web-platform-tests/dom/events/EventTarget-constructible.any.worker-expected.txt: * web-platform-tests/dom/events/event-global-extra.window-expected.txt: * web-platform-tests/dom/idlharness.any.worker-expected.txt: * web-platform-tests/dom/idlharness.window-expected.txt: Source/WebCore: Currently, EventTarget can't be directly constructed or be subclassed in JavaScript. The spec for EventTarget was updated (whatwg/dom#467) to allow constructing and subclassing EventTarget. This feature was shipped in Chrome 64 and Firefox 59. This patch introduces EventTargetConcrete class, a user-constructable version of EventTarget, exposed as "EventTarget" to JavaScript. We don't use EventTarget directly because it is an abstract class and making it non-abstract is unfavorable due to size increase of EventTarget and all of its subclasses with code that is mostly unnecessary for them, resulting in a performance decrease. To prevent definition of specific to EventTargetConcrete `toJS` and `toJSNewlyCreated` functions, we don't define EventTargetConcrete interface type, but rather tweak make_event_factory.pl to default to base interface (like it does for Event). To allow subclassing of all DOM constructors, non-custom ones replace structures of newly created wrapper objects with ones returned by InternalFunction::createSubclassStructure. Per WebIDL spec [1], `setSubclassStructureIfNeeded` helper uses realm of `newTarget` for default prototypes. This approach was chosen because a) detecting [[Construct]] with callFrame->newTarget() is unreliable outside constructor, and b) passing `newTarget` down to `createWrapper` via `toJSNewlyCreated` is quite awkward and would result in massive code change. [1] https://heycam.github.io/webidl/#internally-create-a-new-object-implementing-the-interface (step 3.3.2) Tests: fast/dom/dom-constructors.html imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/constructors.html imported/w3c/web-platform-tests/dom/events/Event-subclasses-constructors.html imported/w3c/web-platform-tests/dom/events/EventTarget-constructible.any.html imported/w3c/web-platform-tests/dom/idlharness.window.html * Headers.cmake: * Sources.txt: * WebCore.xcodeproj/project.pbxproj: * bindings/js/JSDOMWrapperCache.h: (WebCore::setSubclassStructureIfNeeded): * bindings/js/JSEventTargetCustom.cpp: (WebCore::toJSNewlyCreated): * bindings/scripts/CodeGeneratorJS.pm: (GenerateConstructorDefinition): * bindings/scripts/InFilesCompiler.pm: (generateInterfacesHeader): * bindings/scripts/test/JS/*: Adjust bindings expectations. * dom/EventTarget.cpp: (WebCore::EventTarget::create): * dom/EventTarget.h: * dom/EventTarget.idl: * dom/EventTargetConcrete.cpp: Added. * dom/EventTargetConcrete.h: Added. * dom/make_event_factory.pl: (generateImplementation): LayoutTests: * fast/dom/dom-constructors-expected.txt: * fast/dom/dom-constructors.html: * platform/ios/imported/w3c/web-platform-tests/dom/events/Event-subclasses-constructors-expected.txt: Removed. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@256716 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #441. See also https://www.w3.org/Bugs/Public/show_bug.cgi?id=16487.
Tests: web-platform-tests/wpt#6306
Chrome is interested in implementing this; anyone else? @cdumez @smaug---- @travisleithead
Preview | Diff