[webkit-reviews] review denied: [Bug 6458] Make the "khtml/rendering" directory compile on Win32 : [Attachment 5585] Patch to make the rendering dir build on Windows

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Tue Jan 10 06:55:35 PST 2006

Darin Adler <darin at apple.com> has denied Dave Hyatt <hyatt at apple.com>'s request
for review:
Bug 6458: Make the "khtml/rendering" directory compile on Win32

Attachment 5585: Patch to make the rendering dir build on Windows

------- Additional Comments from Darin Adler <darin at apple.com>
The changes in dom_stringimpl.cpp look wrong. I don't understand what the FIXME
is for, and (bool)-1 is just a synonym for true. I think that "return -1"
should actually be a "return false".

I think that it would be slightly bether to, rather than putting #if __APPLE__
around all the uses of KJS::Bindings::Instance, put this at the top of

    #if __APPLE__
    #include <JavaScriptCore/runtime.h>
    namespace KJS { namespace Bindings { class Instance; } }

I think the include of <math.h> in font.h does *not* need to be inside a WIN32

I think that in KWQDateTime.h you should just have defined an inline function
named CFAbsoluteTimeGetCurrent() rather than adding a GET_CURRENT_TIME macro,
like this:

    inline CFAbsoluteTime CFAbsoluteTimeGetCurrent() { return 0; } // FIXME:
Temporary until we port this class.

Otherwise, looks good.

I think that, even though I marked this review-, if you address what I
mentioned above you can land the patch without me reviewing again.

More information about the webkit-reviews mailing list