[Webkit-unassigned] [Bug 14678] [gdk] API Drafting

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=14678


aroben at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #15629|review?                     |review+
               Flag|                            |




------- Comment #11 from aroben at apple.com  2007-07-22 17:53 PDT -------
(From update of attachment 15629)
+    EditorClientGdk* editorClient = new EditorClientGdk;
+    private_data->page = new Page(new ChromeClientGdk(page), new
ContextMenuClientGdk, editorClient, 0, new InspectorClientGdk);
+    editorClient->setPage(private_data->page);

Seems like EditorClientGdk should take a WebKitGtkPage* argument in its
constructor. Just a FIXME is good enough for now.

+     * FIXME: Will be different with multiple frames

It would be good to say what/how/why this will be different so that you don't
forget it and other people understand.

+    PassRefPtr<SharedBuffer> sharedBuffer = new SharedBuffer(strdup(content),
strlen(content));    
+    SubstituteData substituteData(sharedBuffer, String(content_mime_type),
String(content_encoding), KURL("about:blank"), url);

I believe the correct idiom here is to make sharedBuffer a RefPtr (not
PassRefPtr), then pass `sharedBuffer.release()` into the SubstituteData
constructor.

Nothing in WebKit/gtk should be in the WebCore namespace (though I realize that
right now many things are in WebCore that should be in WebKit/gtk). I think you
can move everything in webkitgtkprivate.cpp out of the WebCore namespace. Would
be good to move ChromeClientGdk, too, though that could be a separate patch.

+FloatRect ChromeClientGdk::windowRect() {

+bool ChromeClientGdk::scrollbarsVisible() {

Brace should be on the next line.

+float ChromeClientGdk::scaleFactor()
+{
+    notImplemented();
+    return 1.0;
+}

Should be `1.0f` to avoid double -> float conversion.

+    if (!cresult)
+        return false;
+    else {
+        result = UTF8Encoding().decode(cresult, strlen(cresult));
+        g_free(cresult);
+        return true;
+    }

Don't need to say `else` after the return.

+    for my $i (0 .. $#ARGV) {
+        my $opt = $ARGV[$i];

Since you don't use $i anywhere else, you should just do:

foreach my $opt (@ARGV) {
}

r=me, though I think it would be nice to get someone with GTK+ experience to
have a look at it (maybe Alp Toker?). You should also keep in mind what Maciej
said about the benefits of keeping the API relatively close to the Mac/Windows
API.

It's great to see all those WebCore headers gone from GdkLauncher!


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.