Remove legacy callers #412
Conversation
|
@bzbarsky, are you OK with this? I believe last time we discussed document.all we found out Firefox is the only one using actual IDL to fully define its behavior, so for other browsers I think they will be fine with custom bindings in the HTML spec. But I want to check with you from Firefox's perspective. I think this is a good change from a spec point of view, myself. |
|
(I think @domenic knows, but we probably need to ping again in two weeks unless bz manages to not miss a single email once he gets back.) |
|
In practice this probably doesn't matter too much; Gecko would just keep supporting legacycaller and using IDL for this. If no one else is using IDL here, then clearly there's no implementation change involved, so this is just spec bookkeeping.... |
WebKit also implements and uses legacycaller for its HTMLAllCollection implementation. |
|
@samweinig are you OK with removing legacycaller from IDL and just having a one-off [[Call]] defined in HTML, like in whatwg/html#2932 ? I think it'd be best to leave features used in only one, legacy place out of IDL. |
|
@domenic yeah, I'm not sure it makes much difference. I'll either just keep our legacycaller stuff (or perhaps simplify it since it supports things like overloading) or change the syntax to an extended attribute. I'm not 100% I see the value of removing it. I get that we don't want more specs using it, but it seems like having as much of this legacy stuff in the same place has some value. I also have some concern that the more we specify exotic behavior (like a custom [[Call]]) the more it might encourage other specs to do that, kind of moving the problem around. That said, it seems ok. |
|
@samweinig note that we already have legacy stuff in HTML for |
|
@annevk that's a good point, and ultimately why it seems fine. |
|
@TimothyGu mind rebasing? |
TimothyGu
force-pushed
the
legacycaller
branch
from
1b6bb36
to
d8f79db
Compare
|
@domenic Done. |
Turns out there is only one interface in the Platform using legacy callers (HTMLAllCollection). Define the exotic behaviors there instead. Fixes whatwg#407. Fixes whatwg#408.
TimothyGu
force-pushed
the
legacycaller
branch
from
d8f79db
to
5ee55c8
Compare
|
Freshly rebased. |
|
I went ahead and merged this to avoid another merge conflict. Hope that's okay. |
https://bugs.webkit.org/show_bug.cgi?id=220246 Reviewed by Sam Weinig. Before this change, [LegacyCaller] implementation was very complex yet versatile, handling overloads and multiple callers via operation cloning. This patch removes [LegacyCaller], instead of simplifying it, and leverages [CustomGetCallData] to implement HTMLAllCollection's [[Call]] method for a few reasons: 1. Legacy callers were removed from the WebIDL spec [1], with `document.all` being the only use case; callable objects won't ever be introduced. 2. To closely match the HTML spec [2] by returning early rather than passing nullish AtomString. 3. To make getCallData() override more obvious to a reader unfamiliar with legacy callers. 4. To maximize the amount of code removed from the generator. No new tests, no behavior change. [1] whatwg/webidl#412 [2] https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#HTMLAllCollection-call * Sources.txt: * WebCore.xcodeproj/project.pbxproj: * bindings/js/JSHTMLAllCollectionCustom.cpp: Added. (WebCore::JSC_DEFINE_HOST_FUNCTION): (WebCore::JSHTMLAllCollection::getCallData): * bindings/scripts/CodeGeneratorJS.pm: (GenerateInterface): (GenerateGetCallData): (InstanceOverridesGetCallData): (AddLegacyCallerOperationIfNeeded): Deleted. (GeneratePluginCall): Deleted. (GenerateLegacyCallerDefinitions): Deleted. (GenerateLegacyCallerDefinition): Deleted. * bindings/scripts/IDLAttributes.json: * bindings/scripts/IDLParser.pm: (cloneArgument): Deleted. (cloneOperation): Deleted. * bindings/scripts/test/JS/JSTestObj.cpp: (WebCore::JSTestObjDOMConstructor::construct): (WebCore::callJSTestObj1): Deleted. (WebCore::callJSTestObj2): Deleted. (WebCore::callJSTestObj3): Deleted. (WebCore::JSTestObj::getCallData): Deleted. (WebCore::jsTestObjPrototypeFunction_legacyCallerNamedBody): Deleted. * bindings/scripts/test/JS/JSTestObj.h: * bindings/scripts/test/TestObj.idl: * html/HTMLAllCollection.idl: Canonical link: https://commits.webkit.org/232734@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@271137 268f45cc-cd09-0410-ab3c-d52691b4dbfc
https://bugs.webkit.org/show_bug.cgi?id=220246 Reviewed by Sam Weinig. Before this change, [LegacyCaller] implementation was very complex yet versatile, handling overloads and multiple callers via operation cloning. This patch removes [LegacyCaller], instead of simplifying it, and leverages [CustomGetCallData] to implement HTMLAllCollection's [[Call]] method for a few reasons: 1. Legacy callers were removed from the WebIDL spec [1], with `document.all` being the only use case; callable objects won't ever be introduced. 2. To closely match the HTML spec [2] by returning early rather than passing nullish AtomString. 3. To make getCallData() override more obvious to a reader unfamiliar with legacy callers. 4. To maximize the amount of code removed from the generator. No new tests, no behavior change. [1] whatwg/webidl#412 [2] https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#HTMLAllCollection-call * Sources.txt: * WebCore.xcodeproj/project.pbxproj: * bindings/js/JSHTMLAllCollectionCustom.cpp: Added. (WebCore::JSC_DEFINE_HOST_FUNCTION): (WebCore::JSHTMLAllCollection::getCallData): * bindings/scripts/CodeGeneratorJS.pm: (GenerateInterface): (GenerateGetCallData): (InstanceOverridesGetCallData): (AddLegacyCallerOperationIfNeeded): Deleted. (GeneratePluginCall): Deleted. (GenerateLegacyCallerDefinitions): Deleted. (GenerateLegacyCallerDefinition): Deleted. * bindings/scripts/IDLAttributes.json: * bindings/scripts/IDLParser.pm: (cloneArgument): Deleted. (cloneOperation): Deleted. * bindings/scripts/test/JS/JSTestObj.cpp: (WebCore::JSTestObjDOMConstructor::construct): (WebCore::callJSTestObj1): Deleted. (WebCore::callJSTestObj2): Deleted. (WebCore::callJSTestObj3): Deleted. (WebCore::JSTestObj::getCallData): Deleted. (WebCore::jsTestObjPrototypeFunction_legacyCallerNamedBody): Deleted. * bindings/scripts/test/JS/JSTestObj.h: * bindings/scripts/test/TestObj.idl: * html/HTMLAllCollection.idl: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@271137 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Turns out there is only one interface in the Platform using legacy callers (
HTMLAllCollection). Define the exotic behaviors there instead. PR for that is pending.Fixes #407.
Fixes #408.
Preview | Diff