Bug 98350

Created attachment 208804 [details] WIP patch It depends on addNotificationListener implementation.

Comment on attachment 208804 [details] WIP patch View in context: https://bugs.webkit.org/attachment.cgi?id=208804&action=review > LayoutTests/ChangeLog:7 > + Reviewed by NOBODY (OOPS!). > + > + * platform/gtk/TestExpectations: You need a small description here > Source/WebCore/ChangeLog:13 > + * accessibility/atk/AXObjectCacheAtk.cpp: > + (WebCore::AXObjectCache::postPlatformNotification): > + * accessibility/atk/WebKitAccessibleWrapperAtk.cpp: > + (webkitAccessibleGetAttributes): I know not everyone does it (even though it's in the guidelines), but I normally like to have shorts descriptions for every function touched in the ChangeLog. > Source/WebCore/accessibility/atk/AXObjectCacheAtk.cpp:169 > + } else if (notification == AXInvalidStatusChanged) { > + g_signal_emit_by_name(axObject, "state-change", "invalid-entry", coreObject->invalidStatus() != "false"); > } You do not need { } if it's a one line branch > Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:898 > + > + // In case of 'aria-invalid' when the attribute empty or has "false" for ATK > + // according to http://www.w3.org/WAI/PF/aria-implementation/#mapping attribute > + // is not mapped but layout tests will expect 'false'. > + if (attributeValue.isEmpty() && atkAttributeName == "aria-invalid") > + return JSStringCreateWithUTF8CString("false"); > + You probably need to add this aria-invalid specific mapping to WKTR too

Comment on attachment 208804 [details] WIP patch View in context: https://bugs.webkit.org/attachment.cgi?id=208804&action=review >> LayoutTests/ChangeLog:7 >> + * platform/gtk/TestExpectations: > > You need a small description here ok >> Source/WebCore/ChangeLog:13 >> + (webkitAccessibleGetAttributes): > > I know not everyone does it (even though it's in the guidelines), but I normally like to have shorts descriptions for every function touched in the ChangeLog. ok >> Source/WebCore/accessibility/atk/AXObjectCacheAtk.cpp:169 >> } > > You do not need { } if it's a one line branch ok >> Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:898 >> + > > You probably need to add this aria-invalid specific mapping to WKTR too will do

Created attachment 209982 [details] Patch

Comment on attachment 209982 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=209982&action=review (In reply to comment #4) > Created an attachment (id=209982) [details] > Patch Looks good to me now (and after SVN revision r154697 the addNotificationListener implementation is no longer an issue in DRT. However, I believe you will need to keep the aria-invalid test skipped for WK2 only since we still don't have addNotificationListener for AccessibilityUIElement in WKTR, hence it will still timeout there. > LayoutTests/platform/gtk/TestExpectations:-783 > -webkit.org/b/98350 accessibility/aria-invalid.html [ Timeout ] I would suggest to move this line to the TestExpectations file in gtk-wk2 and reference it against bug 120421 (which tracks the missing implementation of addNotificationListner in WKTR

Created attachment 210286 [details] Patch

Comment on attachment 210286 [details] Patch Thanks for the quick response. LGTM now.

Comment on attachment 210286 [details] Patch Clearing flags on attachment: 210286 Committed r154960: <http://trac.webkit.org/changeset/154960>

All reviewed patches have been landed. Closing bug.