[webkit-reviews] review granted: [Bug 30937] MSAA: Accessibility of headings is not correct : [Attachment 42174] DRT changes, layout test

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has granted Jon Honeycutt
<jhoneycutt at apple.com>'s request for review:
Bug 30937: MSAA: Accessibility of headings is not correct
https://bugs.webkit.org/show_bug.cgi?id=30937

Attachment 42174: DRT changes, layout test
https://bugs.webkit.org/attachment.cgi?id=42174&action=review

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
> +    ASSERT(V_VT(&vRole) == VT_I4 || V_VT(&vRole) == VT_BSTR);
> +
> +    wstring result;
> +    if (V_VT(&vRole) == VT_I4) {
> +	   TCHAR roleText[64] = {0};
> +	   ::GetRoleText(V_I4(&vRole), roleText, ARRAYSIZE(roleText));
> +	   result = roleText;

How do we know 64 characters is enough?

> +    } else if (V_VT(&vRole) == VT_BSTR)
> +	   result = wstring(V_BSTR(&vRole), ::SysStringLen(V_BSTR(&vRole)));
> +
> +    return JSStringCreateWithCharacters(result.data(), result.length());

As David says, you're leaking a BSTR here.

r=me if you fix the leak.