[Webkit-unassigned] [Bug 77878] [gtk] Accessibility: use find funtion in vector instead of for.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Feb 7 04:16:10 PST 2012


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





--- Comment #2 from Mario Sanchez Prada <msanchez at igalia.com>  2012-02-07 04:16:10 PST ---
(From update of attachment 125658)
View in context: https://bugs.webkit.org/attachment.cgi?id=125658&action=review

Thanks for the patch, Frederik. Just informally reviewing now. See some comments below...

> Source/WebCore/ChangeLog:9
> +        find function was basically reimplemented here.

No, there was no good reason for implementing that part that way, other than probably not realizing of the find() function being there. 

Anyway, I'm not sure this "but I was wondering why" part is something that should be in the ChangeLog. I would probably leave it as "This is only a minor cleanup, as the find function was basically being reimplemented." (I'd avoid "here" unless you're using this sentence below, in the part of changelog where the function name is explicitly listed.

> Source/WebCore/accessibility/gtk/WebKitAccessibleWrapperAtk.cpp:399
> +    return parent->children().find(coreObject);

I'm fine with the change. Just a minor nitpick: WTF::Vector::find() returns notFound (from NotFound.h) instead of plainly returning -1 so, even if it's actually the same thing in practise, I think it'd be better to replace this with something like this:

    [...]
    size_t indexFound = parent->children().find(coreObject);
    if (indexFound == WTF::notFound)
      return -1;

    return indexFound;
  }

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