[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