[Webkit-unassigned] [Bug 42232] Make changing Cursors work in WebKit2.

[bugzilla-daemon at webkit.org]

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


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #61533|review?                     |review+
               Flag|                            |




--- Comment #12 from Darin Adler <darin at apple.com>  2010-07-14 11:01:49 PST ---
(From update of attachment 61533)
> +    switch (type) {
> +        case Cursor::Pointer:
> +            return pointerCursor();

Wrong indentation style.

> +const char* nameForCursorType(Cursor::Type type)
> +{
> +    switch (type) {
> +        case Cursor::Pointer:
> +            return "Pointer";

Same.

> +const Cursor& pointerCursor()
> +{
> +    DEFINE_STATIC_LOCAL(Cursor, c, (Cursor::Pointer));
> +    return c;
> +}

How about using the word "cursor" or "staticLocalCursor" instead of "c"?

> +    switch (m_type) {
> +        case Cursor::Pointer:
> +            m_platformCursor = HardRetain([NSCursor arrowCursor]);
> +            break;

Switch style again.

> +            m_platformCursor = HardRetain(leakNamedCursor("crossHairCursor", 11, 11));

Given the word leak in the name of this function, it seems wrong or unnecessary to do another HardRetain.

> +    switch (m_type) {
> +        case Cursor::Pointer:

Another switch statement.

> +    LRESULT onSetCursor(HWND hWnd, UINT message, WPARAM, LPARAM, bool& handled);

Why does the HWND argument need names in all of these?

Otherwise looks good.

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