Bug 121543

Summary: [GTK] Some of DOM bindings macros are misnamed
Product: WebKit Reporter: Tomas Popela <tpopela>
Component: WebKitGTKAssignee: 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 Flags
Patch
none
Patch none

Description Tomas Popela 2013-09-18 00:37:15 PDT
Some of DOM bindings macros are misnamed.

WEBKIT_DOM_HTMLBR_ELEMENT should be WEBKIT_DOM_HTML_BR_ELEMENT
WEBKIT_DOM_HTMLLI_ELEMENT should be WEBKIT_DOM_HTML_LI_ELEMENT
WEBKIT_DOM_HTMLHR_ELEMENT should be WEBKIT_DOM_HTML_HR_ELEMENT
WEBKIT_DOM_HTMLU_LIST_ELEMENT should be WEBKIT_DOM_HTML_U_LIST_ELEMENT
WEBKIT_DOM_HTMLD_LIST_ELEMENT should be WEBKIT_DOM_HTML_D_LIST_ELEMENT
WEBKIT_DOM_HTMLO_LIST_ELEMENT should be WEBKIT_DOM_HTML_O_LIST_ELEMENT

The same is for _IS_ macros (ie. WEBKIT_DOM_IS_HTML_BR_ELEMENT not HTMLBR_ELEMENT).

Looks like generator is doing something wrong with elements that have their tag name with two or less characters.

We should do something similar like in webkit_dom_element_get_id vs. webkit_dom_html_element_get_id case, because these macros are part of the public API.
Comment 1 Carlos Garcia Campos 2013-09-18 00:45:32 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.
Comment 2 Carlos Garcia Campos 2014-06-16 04:33:25 PDT
(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.
Comment 3 Martin Robinson 2014-06-16 08:49:31 PDT
(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.
Comment 4 Gustavo Noronha (kov) 2014-06-19 13:36:52 PDT
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.
Comment 5 Tomas Popela 2014-06-23 05:33:25 PDT
Created attachment 233599 [details]
Patch
Comment 6 Carlos Garcia Campos 2014-06-23 06:32:28 PDT
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.
Comment 7 Tomas Popela 2014-06-23 06:39:53 PDT
(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.
Comment 8 Tomas Popela 2014-06-23 06:42:42 PDT
Created attachment 233602 [details]
Patch
Comment 9 Martin Robinson 2014-06-24 11:59:49 PDT
Comment on attachment 233602 [details]
Patch

I haven't reviewed this, but I support making the names consistent.
Comment 10 Carlos Garcia Campos 2014-06-24 23:23:49 PDT
Comment on attachment 233602 [details]
Patch

Ok
Comment 11 WebKit Commit Bot 2014-06-24 23:55:13 PDT
Comment on attachment 233602 [details]
Patch

Clearing flags on attachment: 233602

Committed r170422: <http://trac.webkit.org/changeset/170422>
Comment 12 WebKit Commit Bot 2014-06-24 23:55:18 PDT
All reviewed patches have been landed.  Closing bug.