[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 #30 from Mario Sanchez Prada <mario at webkit.org>  2012-12-21 00:31:57 PST ---
(From update of attachment 180345)
View in context: https://bugs.webkit.org/attachment.cgi?id=180345&action=review

Thanks Frederik for working on this patch. Guess it's not coincidence the release of Qt5 happened two days ago, huh? :-)

In any case, overall the patch looks good to me providing it works (I have no much knowledge about Qt) and that you fix both those build issues spotted by the EWS and the things both Simon and me commented on.

> Source/WebCore/Target.pri:56
> +    accessibility/AXObjectCacheQt.cpp \

Why not placing AXObjectCacheQt.cpp inside of accessibility/qt, as other ports do?

> Source/WebCore/accessibility/AccessibilityObject.cpp:78
> +#if PLATFORM(GTK) || (PLATFORM(EFL) && HAVE(ACCESSIBILITY)) || (PLATFORM(QT) && HAVE(ACCESSIBILITY))

You could refactor this boolean condition to something shorter:

  #if PLATFORM(GTK) || ((PLATFORM(EFL) || PLATFORM(QT)) && HAVE(ACCESSIBILITY))

...or maybe just to leave it as "#if HAVE(ACCESSIBILITY)" (not 100% sure though)

>> Source/WebCore/accessibility/qt/AccessibilityObjectQt.cpp:40
>> +void AccessibilityObject::setWrapper(AccessibilityObjectWrapper *wrapper)
> 
> * placement

Misplaced "*"

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