[webkit-reviews] review denied: [Bug 61136] [Windows] Scrollbars Transparent in Windows XP if WebKit is using Layered Window : [Attachment 95160] format code

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue May 31 20:06:29 PDT 2011


Brent Fulgham <bfulgham at webkit.org> has denied	review:
Bug 61136: [Windows] Scrollbars Transparent in Windows XP if WebKit is using
Layered Window
https://bugs.webkit.org/show_bug.cgi?id=61136

Attachment 95160: format code
https://bugs.webkit.org/attachment.cgi?id=95160&action=review

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

This looks great!  Nice job getting the rect math cleaned up in this last
patch.

A couple of general comments:
1. Please provide a changelog entry for your modifications.
2. Please move the 'setRGBABitmapAlpha' implementation to the DIBPixelData cpp
file, and declare it as a static function in the WebCore namespace.
3. Please don't bother making whitespace changes to areas of the code you are
not modifying.	You can always create a second patch to handle whitespace if
you wish.

Rejecting because there is no changelog entry.	Otherwise, looks great!

> Source/WebCore/rendering/RenderThemeWin.cpp:618
> +void setRGBABitmapAlpha(HDC hdc, const IntRect& dstRect, unsigned char
level)

Please make this a static method in namespace WebCore in the DIBPixelData
header/implementation.	Then we can get rid of the extern declaration and have
both files #include the DIBPixelData.h header.


More information about the webkit-reviews mailing list