[Webkit-unassigned] [Bug 33785] [Gtk] AtkHyperlink needs to be implemented

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=33785


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #71301|review?                     |review-
               Flag|                            |




--- Comment #11 from Martin Robinson <mrobinson at webkit.org>  2010-10-20 21:07:57 PST ---
(From update of attachment 71301)
View in context: https://bugs.webkit.org/attachment.cgi?id=71301&action=review

> WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1940
> +static AtkHyperlink* webkitAccessibleHypertextGetLink(AtkHypertext* hypertext, gint index)

I think you can change the index parameter type to size_t and avoid the cast below.

> WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1943
> +    AccessibilityObject::AccessibilityChildrenVector children = coreObject->children();

Could this just core(hypertext->children();

> WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1966
> +static gint webkitAccessibleHypertextGetNLinks(AtkHypertext* hypertext)

Avoid abbreviations here. webkitAccessibleHypertextGetLinkCount or webkitAccessibleHypertextGetNumberOfLinks. The method could also return a size_t.

> WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1972
> +    for (unsigned i = 0; i < children.size(); i++) {

I think you should use size_t here instead of unsigned.

> WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1985
> +    gint nLinks = webkitAccessibleHypertextGetNLinks(hypertext);
> +    if (!nLinks)
> +        return -1;

Should be numberOfLinks or perhaps linksCount. Avoid abbreviations. The type should also probably be size_t.

> WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1987
> +    for (gint i = 0; i < nLinks; i++) {

size_t again.

> WebCore/accessibility/gtk/WebKitAccessibleHyperlink.cpp:102
> +static gint webkitAccessibleHyperlinkActionGetNActions(AtkAction* action)

Avoid the abbreviation here as well.

> WebCore/accessibility/gtk/WebKitAccessibleHyperlink.cpp:165
> +static gchar* webkitAccessibleHyperlinkGetUri(AtkHyperlink* link, gint index)

Should be webkitAccessibleHyperlinkGetURI

> WebCore/accessibility/gtk/WebKitAccessibleHyperlink.cpp:168
> +    // FIXME Do NOT support more than one instance of an AtkObject

Very small nit! FIXME -> FIXME:

> WebCore/accessibility/gtk/WebKitAccessibleHyperlink.cpp:303
> +GType
> +webkitAccessibleHyperlinkGetType(void)

Don't use a newline after GType here, if it can be avoided remove the void.

> WebCore/accessibility/gtk/WebKitAccessibleHyperlink.cpp:342
> +    g_hash_table_remove(hashTable, (void*)linkImpl);

I could be wrong, but I don't think you need to cast to void*. If you do, you should use a static_cast<void*>().

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.