[Webkit-unassigned] [Bug 119829] IAccessibleText/2 implementation for AppleWindows port

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=119829





--- Comment #4 from Brent Fulgham <bfulgham at webkit.org>  2013-08-15 20:33:48 PST ---
(From update of attachment 208876)
View in context: https://bugs.webkit.org/attachment.cgi?id=208876&action=review

I think this looks great!  Sorry this turned into a larger project than I had originally envisioned. :(

> Source/WebKit/win/AccessibleText.cpp:63
> +HRESULT AccessibleText::attributes(long offset, long *startOffset, long *endOffset, BSTR *textAttributes)

The asterisk is part of the type! Please write long* startOffset, etc.

> Source/WebKit/win/AccessibleText.cpp:73
> +HRESULT AccessibleText::caretOffset(long *offset)

Ditto

> Source/WebKit/win/AccessibleText.cpp:76
> +        return E_FAIL;

We usually check that the pointer argument is non-null and return E_POINTER if we are called without a valid pointer. This applies to the other methods as well.

> Source/WebKit/win/AccessibleText.cpp:205
> +    int previous_pos = std::max(0, (int)(offset-1));

Need a C++ cast here...

> Source/WebKit/win/AccessibleText.cpp:528
> +    else

No need for this else case. Just return offset.

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