Bug 117513 – [Windows] Implement 'attributeValue' accessor to support test infrastructure

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[Brent Fulgham]

Several accessibility tests assume that a custom accessibility attribute called "AXDRTElementIdAttribute" can be used to access the element ID of the underlying DOM element for a given accessibility object.  This does not exist in the Windows implementation, preventing many accessibility tests to fail or crash.

This bug implements the accessor for this element.  Most tests still fail for other reasons, but this is the first step towards getting them properly functional.

[Radar WebKit Bug Importer]

<rdar://problem/14117373>

[Brent Fulgham]

Created attachment 204347 [details]
Patch

[Brent Fulgham]

Created attachment 204348 [details]
Revised to get rid of horrid Visual Studio project cruft.

[Anders Carlsson]

Comment on attachment 204348 [details]
Revised to get rid of horrid Visual Studio project cruft.

View in context: https://bugs.webkit.org/attachment.cgi?id=204348&action=review

> Source/WebKit/win/AccessibleBase.cpp:849
> +    *value = ::SysAllocStringLen(reinterpret_cast<const wchar_t*>(valueAtomic.characters()), valueAtomic.length());

This could use a BString together with .release().

> Tools/DumpRenderTree/win/AccessibilityControllerWin.cpp:88
> +    COMPtr<IAccessibleComparable>comparable = comparableObject(serviceProvider);

Missing space.

> Tools/DumpRenderTree/win/AccessibilityControllerWin.cpp:107
> +

No need for a newline here.

> Tools/DumpRenderTree/win/AccessibilityControllerWin.cpp:122
> +    BSTR idAttribute = JSStringCopyBSTR(id);

I think you can use a BString here - just use BString::adopt. (or better yet, add an adoptBString function just like we have for RetainPtr).

[Brent Fulgham]

(In reply to comment #4)
> (From update of attachment 204348 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=204348&action=review
> 
> > Source/WebKit/win/AccessibleBase.cpp:849
> > +    *value = ::SysAllocStringLen(reinterpret_cast<const wchar_t*>(valueAtomic.characters()), valueAtomic.length());
> 
> This could use a BString together with .release().

Done!

> > Tools/DumpRenderTree/win/AccessibilityControllerWin.cpp:88
> > +    COMPtr<IAccessibleComparable>comparable = comparableObject(serviceProvider);
> 
> Missing space.

Done!

> > Tools/DumpRenderTree/win/AccessibilityControllerWin.cpp:107
> > +
> 
> No need for a newline here.

Done!

> > Tools/DumpRenderTree/win/AccessibilityControllerWin.cpp:122
> > +    BSTR idAttribute = JSStringCopyBSTR(id);
> 
> I think you can use a BString here - just use BString::adopt. (or better yet, add an adoptBString function just like we have for RetainPtr).

I can't do this one.  We are not supposed to use internal WebCore types in 'client' code (like this test harness).  :-(

[Brent Fulgham]

Committed r151459: <http://trac.webkit.org/changeset/151459>