Bug 117513 - [Windows] Implement 'attributeValue' accessor to support test infrastructure
Summary: [Windows] Implement 'attributeValue' accessor to support test infrastructure
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows 7
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-06-11 10:56 PDT by Brent Fulgham
Modified: 2013-06-11 12:53 PDT (History)
9 users (show)

See Also:


Attachments
Patch (981.52 KB, patch)
2013-06-11 11:45 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Revised to get rid of horrid Visual Studio project cruft. (13.53 KB, patch)
2013-06-11 12:15 PDT, Brent Fulgham
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2013-06-11 10:56:29 PDT
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.
Comment 1 Radar WebKit Bug Importer 2013-06-11 10:57:29 PDT
<rdar://problem/14117373>
Comment 2 Brent Fulgham 2013-06-11 11:45:52 PDT
Created attachment 204347 [details]
Patch
Comment 3 Brent Fulgham 2013-06-11 12:15:40 PDT
Created attachment 204348 [details]
Revised to get rid of horrid Visual Studio project cruft.
Comment 4 Anders Carlsson 2013-06-11 12:22:18 PDT
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).
Comment 5 Brent Fulgham 2013-06-11 12:46:13 PDT
(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).  :-(
Comment 6 Brent Fulgham 2013-06-11 12:53:56 PDT
Committed r151459: <http://trac.webkit.org/changeset/151459>