[webkit-reviews] review denied: [Bug 121212] WinLauncher needs back and forward buttons : [Attachment 211399] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has denied Alex Christensen
<alex.christensen at flexsim.com>'s request for review:
Bug 121212: WinLauncher needs back and forward buttons
https://bugs.webkit.org/show_bug.cgi?id=121212

Attachment 211399: Patch
https://bugs.webkit.org/attachment.cgi?id=211399&action=review

------- Additional Comments from Brent Fulgham <bfulgham at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=211399&action=review


Thanks for doing this. It will be ice to have these in the UI. r- so you don't
delete hooks to some things I'm trying to get landed. :)

> Tools/WinLauncher/WinLauncher.cpp:90
> +LRESULT CALLBACK    MyReloadButtonProc(HWND, UINT, WPARAM, LPARAM);

Can we please call these something else? The 'My...' Naming makes it seem like
a programming class project! :)

> Tools/WinLauncher/WinLauncher.cpp:379
> +		       hInst, 0);

Please format these normally with 4-space indents. One or two lines are better
than these large blocks.

> Tools/WinLauncher/WinLauncher.cpp:394
> +    DefButtonProc =
reinterpret_cast<WNDPROC>(GetWindowLongPtr(hBackButtonWnd, GWLP_WNDPROC));

Honestly, I'm surprised that the GetWindowLongPtr isn't defined as a 32/64 type
based on compiler settings. Can you double check that this is necessary? I sort
of think it could just be the ...Ptr version.

> Tools/WinLauncher/WinLauncher.cpp:-685
> -

Please don't delete this. It's WIP.

> Tools/WinLauncher/WinLauncher.cpp:-730
> -	       break;

Leave in please.

> Tools/WinLauncher/WinLauncher.vcxproj/WinLauncherLib.rc:-52
> -    END

Leave in please.