[Webkit-unassigned] [Bug 47365] getTextAtOffset returns incorrect results if a link includes text and an image

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Oct 21 00:44:50 PDT 2010


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


Martin Robinson <mrobinson at webkit.org> changed:

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




--- Comment #3 from Martin Robinson <mrobinson at webkit.org>  2010-10-21 00:44:50 PST ---
(From update of attachment 71330)
View in context: https://bugs.webkit.org/attachment.cgi?id=71330&action=review

Looks good, but I have a few concerns.

> WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:866
> +    GString* str = g_string_new(0);

Please don't use an abbreviation here.

> WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:873
> +    for (RenderObject* obj = renderer->firstChild(); obj; obj = obj->nextSibling()) {
> +        if (obj->isBR()) {

Again, we've been trying to avoid abbreviations in new code.

> WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:936
> +        g_string_append(str, textForRenderer(accObject->renderer()));

Isn't this a memory leak? textForRenderer returns a newly allocated string.

> WebKit/gtk/tests/testatk.c:975
> +    webkit_web_view_load_string(webView, linksWithInlineImages, NULL, NULL, NULL);

We should follow WebKit style in the tests as much as possible, thus NULL => 0.

> WebKit/gtk/tests/testatk.c:980
> +    g_idle_add((GSourceFunc)bail_out, loop);
> +    g_main_loop_run(loop);
> +

Didn't we discover that this approach was unnecessary in a previous patch or does something about this require it?

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



More information about the webkit-unassigned mailing list