[webkit-reviews] review denied: [Bug 25413] [GTK] Please expose the level of headings. : [Attachment 41355] proposed patch - take 1

[bugzilla-daemon at webkit.org]

Xan Lopez <xan.lopez at gmail.com> has denied  review:
Bug 25413: [GTK] Please expose the level of headings.
https://bugs.webkit.org/show_bug.cgi?id=25413

Attachment 41355: proposed patch - take 1
https://bugs.webkit.org/attachment.cgi?id=41355&action=review

------- Additional Comments from Xan Lopez <xan.lopez at gmail.com>
This patch is amazing, great job!

A few small comments:

> +static AtkAttributeSet* addAttributeToSet(AtkAttributeSet* attributeSet,
const char* name, const char* value)
> +{
> +    AtkAttribute* attribute = (AtkAttribute*)g_malloc(sizeof(AtkAttribute));


WebKit style guide tells us to use C++ style castings, so you need to do
something like:

AtkAttribute* attribute =
static_cast<AtkAttribute>(g_malloc(sizeof(AtkAttribute)));


> +    attribute->name = g_strdup(name);
> +    attribute->value = g_strdup(value);
> +    attributeSet = g_slist_prepend(attributeSet, attribute);
> +
> +    return attributeSet;
> +}
> +
> +static AtkAttributeSet* webkit_accessible_get_attributes(AtkObject* object)
> +{
> +    AtkAttributeSet* attributeSet = NULL;
> +
> +    int headingLevel = core(object)->headingLevel();
> +    if (headingLevel) {
> +	   String value = String::number(headingLevel);
> +	   attributeSet = addAttributeToSet(attributeSet, (const char*)"level",
returnString(value));

- Is the (const char*) casting for "level" really needed? I think it should be
const char* already.
- Using returnString() here is a tiny hack, since the function was really
intended for return values in public APIs implementations. Do thinks work if
you pass value.utf8().data() to the function?

I'm marking it as r- while we work on those final details.