[Webkit-unassigned] [Bug 77864] [Qt] Initial implementation of accessibility support

[bugzilla-daemon at webkit.org]

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





--- Comment #35 from Simon Hausmann <hausmann at webkit.org>  2013-01-09 07:16:50 PST ---
(From update of attachment 181907)
View in context: https://bugs.webkit.org/attachment.cgi?id=181907&action=review

Some early comments

> Source/WebCore/accessibility/qt/AccessibilityObjectWrapperQt.cpp:51
> +    Q_ASSERT(m_document);
> +    Q_ASSERT(m_id);

We use WTF/WebKit types and macros usually, so in this case just ASSERT() :)

> Source/WebCore/accessibility/qt/AccessibilityObjectWrapperQt.cpp:102
> +    if (accessibilityObject()) {
> +        const AccessibilityObjectWrapper* childWrapper = static_cast<const AccessibilityObjectWrapper*>(childIface);
> +        if (AccessibilityObject* childObject = childWrapper->accessibilityObject()) {
> +            AccessibilityObject* parentObject = childObject->parentObjectUnignored();
> +            if (parentObject)
> +                return parentObject->children().find(childObject);
> +        }
> +    }
> +    return -1;

I've become a fan of the early-return style of code formatting that reduces the nesting required. So

if (!accessibiltyObject())
   return -1;

<other code with one level of indentation less>

> Source/WebKit/qt/WidgetApi/qwebviewaccessible.cpp:64
> +    if (index)

Would it perhaps be more readable to write if (index != 0) ?

> Tools/qmake/mkspecs/features/features.prf:124
> +    contains(QT_CONFIG,accessibility) {

Yeah so it looks like here you need to do a Qt version check :)

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