[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]

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
http://bugzilla.opendarwin.org/show_bug.cgi?id=6458

Attachment 5585: Patch to make the rendering dir build on Windows
http://bugzilla.opendarwin.org/attachment.cgi?id=5585&action=edit

------- 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
html_objectimpl.h:

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

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

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.