Summary: | [GTK] Some of DOM bindings macros are misnamed | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tomas Popela <tpopela> | ||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | alecflett, cdumez, cgarcia, commit-queue, eric.carlson, glenn, gustavo, jer.noble, jsbell, mrobinson, philipj, sergio, xan.lopez | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 133724 | ||||||||
Attachments: |
|
Description
Tomas Popela
2013-09-18 00:37:15 PDT
We have some exceptions in the function decamelize, but not all cases are covered. While working on bug #121538 I also noticed an inconsistency in the macros for the WebKitDOMObject and all others, see: WEBKIT_TYPE_DOM_OBJECT WEBKIT_IS_DOM_OBJECT WEBKIT_IS_DOM_OBJECT_CLASS While all other generated objects have WEBKIT_DOM as the prefix, for example: WEBKIT_TYPE_DOM_ATTR WEBKIT_DOM_IS_ATTR WEBKIT_DOM_IS_ATTR_CLASS So, we are always using WEBKIT as the prefix for TYPE macros, but WEBKIT_DOM for IS macros except for WebKitDOMObject that we use WEBKIT. I think we should fix all these inconsistencies and provide backwards compatibility macros in WebKitDOMCustom. (In reply to comment #1) > We have some exceptions in the function decamelize, but not all cases are covered. While working on bug #121538 I also noticed an inconsistency in the macros for the WebKitDOMObject and all others, see: > > WEBKIT_TYPE_DOM_OBJECT > WEBKIT_IS_DOM_OBJECT > WEBKIT_IS_DOM_OBJECT_CLASS > > While all other generated objects have WEBKIT_DOM as the prefix, for example: > > WEBKIT_TYPE_DOM_ATTR > WEBKIT_DOM_IS_ATTR > WEBKIT_DOM_IS_ATTR_CLASS > > So, we are always using WEBKIT as the prefix for TYPE macros, but WEBKIT_DOM for IS macros except for WebKitDOMObject that we use WEBKIT. > > I think we should fix all these inconsistencies and provide backwards compatibility macros in WebKitDOMCustom. To make it clear, since we are using WebKitDOM as our namespace I think we should rename all TYPE macros as WEBKIT_DOM_TYPE_FOO and WebKitDOMObject macros as well. Unless I'm wrong and the intention was not to use WebKitDOM as the namespace. Maybe Gustavo or Martin know it. (In reply to comment #2) > To make it clear, since we are using WebKitDOM as our namespace I think we should rename all TYPE macros as WEBKIT_DOM_TYPE_FOO and WebKitDOMObject macros as well. Unless I'm wrong and the intention was not to use WebKitDOM as the namespace. Maybe Gustavo or Martin know it. I cannot remember why it was designed this way. I think originally the namespace was supposed to be WebKit only, even for the DOM bindings, but in practice it ended up being WebKitDOM, so I think this fix would make sense. Created attachment 233599 [details]
Patch
Comment on attachment 233599 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=233599&action=review Great job! Thank you very much. This looks good to me. Martin, Gustavo, any objection? > Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:1419 > - return WEBKIT_DOM_${clsCaps}(g_object_new(WEBKIT_TYPE_DOM_${clsCaps}, "core-object", coreObject, NULL)); > + return WEBKIT_DOM_${clsCaps}(g_object_new(WEBKIT_DOM_TYPE_${clsCaps}, "core-object", coreObject, NULL)); We can take advantage we are changing this line to use nullptr instead of NULL. (In reply to comment #6) > (From update of attachment 233599 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=233599&action=review > > Great job! Thank you very much. This looks good to me. Martin, Gustavo, any objection? > > > Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:1419 > > - return WEBKIT_DOM_${clsCaps}(g_object_new(WEBKIT_TYPE_DOM_${clsCaps}, "core-object", coreObject, NULL)); > > + return WEBKIT_DOM_${clsCaps}(g_object_new(WEBKIT_DOM_TYPE_${clsCaps}, "core-object", coreObject, NULL)); > > We can take advantage we are changing this line to use nullptr instead of NULL. No problem. Updated patch will follow. Created attachment 233602 [details]
Patch
Comment on attachment 233602 [details]
Patch
I haven't reviewed this, but I support making the names consistent.
Comment on attachment 233602 [details]
Patch
Ok
Comment on attachment 233602 [details] Patch Clearing flags on attachment: 233602 Committed r170422: <http://trac.webkit.org/changeset/170422> All reviewed patches have been landed. Closing bug. |