Description
Maxime Simon
2009-07-03 02:44:41 PDT
Created attachment 32229 [details]
Patch to add ChromeClient for Haiku WebCore support.
Created attachment 32230 [details]
Patch to add ContextMenuClient for Haiku WebCore support.
Created attachment 32231 [details]
Patch to add DragClient for Haiku WebCore support.
Created attachment 32232 [details]
Patch to add EditorClient for Haiku WebCore support.
Created attachment 32233 [details]
Patch to add FrameLoaderClient for Haiku WebCore support.
Created attachment 32234 [details]
Patch to add InspectorClient for Haiku WebCore support.
Comment on attachment 32231 [details]
Patch to add DragClient for Haiku WebCore support.
Seems like you probably wanted DragDestinationActionNone, but OK.
Looks OK.
Comment on attachment 32234 [details]
Patch to add InspectorClient for Haiku WebCore support.
Looks OK. We've often used TemporaryLinkStubs for such things, but this is also fine.
Comment on attachment 32229 [details]
Patch to add ChromeClient for Haiku WebCore support.
addMessageToConsole and canRunBeforeUnloadConfirmPanel definitions are not correctly indented
Comment on attachment 32233 [details]
Patch to add FrameLoaderClient for Haiku WebCore support.
FrameLoaderClientHaiku::saveDocumentViewToCachedPage shouldn't be present if its commented out. But aside from that i'd prefer it if brady or antti reviewed this patch.
Created attachment 32858 [details]
Patch to add ChromeClient for Haiku WebCore support.
Created attachment 32860 [details]
Patch to add FrameLoaderClient for Haiku WebCore support.
(In reply to comment #10) > (From update of attachment 32233 [details]) > FrameLoaderClientHaiku::saveDocumentViewToCachedPage shouldn't be present if > its commented out. But aside from that i'd prefer it if brady or antti > reviewed this patch. Since I commited the old patch I made some changes. So I commited a new patch. There is no more commented out method. Comment on attachment 32230 [details] Patch to add ContextMenuClient for Haiku WebCore support. Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebKit/ChangeLog A WebKit/haiku/WebCoreSupport/ContextMenuClientHaiku.cpp A WebKit/haiku/WebCoreSupport/ContextMenuClientHaiku.h Committed r46013 A WebKit/haiku/WebCoreSupport/ContextMenuClientHaiku.h A WebKit/haiku/WebCoreSupport/ContextMenuClientHaiku.cpp M WebKit/ChangeLog r46013 = c289b08492c0915116b42e31e5a6a33201fabcd2 (trunk) No changes between current HEAD and refs/remotes/trunk Resetting to the latest refs/remotes/trunk http://trac.webkit.org/changeset/46013 Comment on attachment 32231 [details] Patch to add DragClient for Haiku WebCore support. Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebKit/ChangeLog A WebKit/haiku/WebCoreSupport/DragClientHaiku.cpp A WebKit/haiku/WebCoreSupport/DragClientHaiku.h Committed r46014 A WebKit/haiku/WebCoreSupport/DragClientHaiku.cpp A WebKit/haiku/WebCoreSupport/DragClientHaiku.h M WebKit/ChangeLog r46014 = 1f86110d811f8df279aa86457ba1ae333c89d41e (trunk) No changes between current HEAD and refs/remotes/trunk Resetting to the latest refs/remotes/trunk http://trac.webkit.org/changeset/46014 Comment on attachment 32232 [details] Patch to add EditorClient for Haiku WebCore support. Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebKit/ChangeLog A WebKit/haiku/WebCoreSupport/EditorClientHaiku.cpp A WebKit/haiku/WebCoreSupport/EditorClientHaiku.h Committed r46015 A WebKit/haiku/WebCoreSupport/EditorClientHaiku.cpp A WebKit/haiku/WebCoreSupport/EditorClientHaiku.h M WebKit/ChangeLog r46015 = 38130bab20f4c45ac67252185d757cf5bfc243b3 (trunk) No changes between current HEAD and refs/remotes/trunk Resetting to the latest refs/remotes/trunk http://trac.webkit.org/changeset/46015 Comment on attachment 32234 [details] Patch to add InspectorClient for Haiku WebCore support. Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebKit/ChangeLog A WebKit/haiku/WebCoreSupport/InspectorClientHaiku.cpp A WebKit/haiku/WebCoreSupport/InspectorClientHaiku.h Committed r46016 A WebKit/haiku/WebCoreSupport/InspectorClientHaiku.cpp A WebKit/haiku/WebCoreSupport/InspectorClientHaiku.h M WebKit/ChangeLog r46016 = adf9bbe9de1e7ae895f888409449f552d061b9d8 (trunk) No changes between current HEAD and refs/remotes/trunk Resetting to the latest refs/remotes/trunk http://trac.webkit.org/changeset/46016 All reviewed patches landed, closing. Again, bugzilla-tool is too eager about closing bugs. :) Comment on attachment 32858 [details]
Patch to add ChromeClient for Haiku WebCore support.
Is it a deliberate choice to leave the printf's in the ChromeClientHaiku::runJavaScript* methods? Other than those this patch seems good but i'd like to confirm that those are/are not deliberate
Created attachment 33037 [details]
Patch to add ChromeClient for Haiku WebCore support.
Indeed, it wasn't my intention to leave the printf's in the runJavaScript[...] methods.
Comment on attachment 32860 [details] Patch to add FrameLoaderClient for Haiku WebCore support. Just a few issues to address. > diff --git a/WebKit/haiku/WebCoreSupport/FrameLoaderClientHaiku.cpp b/WebKit/haiku/WebCoreSupport/FrameLoaderClientHaiku.cpp > +/* > + * Copyright (C) 2006 Don Gibson <dgibson77@gmail.com> > + * Copyright (C) 2006 Zack Rusin <zack@kde.org> > + * Copyright (C) 2006 Apple Computer, Inc. All rights reserved. > + * Copyright (C) 2007 Trolltech ASA > + * Copyright (C) 2007 Ryan Leavengood <leavengood@gmail.com> > + * > + * All rights reserved. This is an odd location for "All rights reserved." It typically follows the copyright lines. > +#include "config.h" > +#include "FrameLoaderClientHaiku.h" > + > +#include "DocumentLoader.h" > +#include "FrameLoader.h" > +#include "FrameTree.h" > +#include "Frame.h" > +#include "FrameView.h" > +#include "HTMLFrameOwnerElement.h" > +#include "Page.h" > +#include "PlatformString.h" > +#include "ResourceRequest.h" > + > +#include "NotImplemented.h" This appears out of sort order. > +void FrameLoaderClientHaiku::setWebView(WebView* webview) webView > +bool FrameLoaderClientHaiku::hasWebView() const > +{ > + return m_webView != NULL; Avoid NULL (use 0), but also avoid comparisons to 0/NULL. return m_webView; > +void FrameLoaderClientHaiku::dispatchWillPerformClientRedirect(const KURL&, > + double interval, > + double fireDate) Feel free to unwrap these parameters into one line. > +void FrameLoaderClientHaiku::didChangeTitle(DocumentLoader *docLoader) "*" in wrong location. (DocumentLoader* docLoader) > + if (m_firstData) { > + FrameLoader *frameLoader = loader->frameLoader(); "*" in wrong location. > +{ > + // FIXME: Indentation is off. > diff --git a/WebKit/haiku/WebCoreSupport/FrameLoaderClientHaiku.h b/WebKit/haiku/WebCoreSupport/FrameLoaderClientHaiku.h > @@ -0,0 +1,248 @@ > +/* > + * Copyright (C) 2006 Zack Rusin <zack@kde.org> > + * Copyright (C) 2006 Apple Computer, Inc. All rights reserved. > + * Copyright (C) 2007 Ryan Leavengood <leavengood@gmail.com> > + * > + * All rights reserved. Same comment as before. > +#ifndef FrameLoaderClientHaiku_H Use lowercase h like this: FrameLoaderClientHaiku_h > +#include "FrameLoaderClient.h" > +#include "FrameLoader.h" Out of sort order. > + class NavigationAction; > + class String; > + class ResourceLoader; Out of sort order. > + void setFrame(Frame *frame); Remove parameter names if they don't add any information. This should be "void setFrame(Frame*);" > + void setWebView(WebView *webview); Remove parameter names if they don't add any information. > + virtual void dispatchDecidePolicyForMIMEType(FramePolicyFunction function, Remove parameter names if they don't add any information. > + virtual void dispatchDecidePolicyForNewWindowAction(FramePolicyFunction function, Remove parameter names if they don't add any information. > + virtual void dispatchDecidePolicyForNavigationAction(FramePolicyFunction function, Remove parameter names if they don't add any information. > + // FIXME: This should probably not be here, but it's needed for the tests currently Add "." > + virtual PassRefPtr<Frame> createFrame(const KURL& url, const String& name, > + HTMLFrameOwnerElement* ownerElement, Remove parameter names if they don't add any information: ownerElement. > + virtual PassRefPtr<Widget> createPlugin(const IntSize&, HTMLPlugInElement*, const KURL&, > + const Vector<String>&, const Vector<String>&, const String&, indentation is off. > + bool loadManually) ; > + > + virtual PassRefPtr<Widget> createJavaAppletWidget(const IntSize&, HTMLAppletElement*, > + const KURL& baseURL, const Vector<String>& paramNames, indentation is off. > + const Vector<String>& paramValues); > + Frame *m_frame; "*" in wrong location. > + WebView *m_webView; "*" in wrong location. Comment on attachment 33037 [details] Patch to add ChromeClient for Haiku WebCore support. Just a few comments to address and this will be ready to land. > diff --git a/WebKit/haiku/WebCoreSupport/ChromeClientHaiku.cpp b/WebKit/haiku/WebCoreSupport/ChromeClientHaiku.cpp > @@ -0,0 +1,360 @@ > +/* > + * Copyright (C) 2006 Zack Rusin <zack@kde.org> > + * Copyright (C) 2006 Apple Computer, Inc. All rights reserved. > + * Copyright (C) 2007 Ryan Leavengood <leavengood@gmail.com> > + * > + * All rights reserved. Looks odd as before. > + > +#include "Frame.h" > +#include "FrameView.h" > +#include "FrameLoadRequest.h" > +#include "PlatformString.h" > +#include "HitTestResult.h" > + > +#include "NotImplemented.h" Sort all of these. > diff --git a/WebKit/haiku/WebCoreSupport/ChromeClientHaiku.h b/WebKit/haiku/WebCoreSupport/ChromeClientHaiku.h > + * Copyright (C) 2006 Zack Rusin <zack@kde.org> > + * Copyright (C) 2007 Ryan Leavengood <leavengood@gmail.com> > + * > + * All rights reserved. Ditto :) > +#ifndef ChromeClientHaiku_H Use "_h" instead of "_H" Created attachment 33607 [details]
Patch to add ChromeClient for Haiku WebCore support.
Created attachment 33608 [details]
Patch to add FrameLoaderClient for Haiku WebCore support.
Comment on attachment 33607 [details] Patch to add ChromeClient for Haiku WebCore support. > + > + #if ENABLE(OFFLINE_WEB_APPLICATIONS) > + virtual void reachedMaxAppCacheSize(int64_t spaceNeeded); > + #endif The #if (and #endif) shouldn't be indented. |