Bug 121543 – [GTK] Some of DOM bindings macros are misnamed

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Tomas Popela]

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.

[Carlos Garcia Campos]

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.

[Carlos Garcia Campos]

(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.

[Martin Robinson]

(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.

[Gustavo Noronha (kov)]

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.

[Tomas Popela]

Created attachment 233599 [details]
Patch

[Carlos Garcia Campos]

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.

[Tomas Popela]

(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.

[Tomas Popela]

Created attachment 233602 [details]
Patch

[Martin Robinson]

Comment on attachment 233602 [details]
Patch

I haven't reviewed this, but I support making the names consistent.

[Carlos Garcia Campos]

Comment on attachment 233602 [details]
Patch

Ok

[WebKit Commit Bot]

Comment on attachment 233602 [details]
Patch

Clearing flags on attachment: 233602

Committed r170422: <http://trac.webkit.org/changeset/170422>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.