[webkit-reviews] review denied: [Bug 15891] [GTK] Javascript console and dialogs are not implemented : [Attachment 17236] Better signal names. (fixed comment)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Nov 13 13:01:10 PST 2007


Alp Toker <alp at atoker.com> has denied Christian Dywan
<christian at twotoasts.de>'s request for review:
Bug 15891: [GTK] Javascript console and dialogs are not implemented
http://bugs.webkit.org/show_bug.cgi?id=15891

Attachment 17236: Better signal names. (fixed comment)
http://bugs.webkit.org/attachment.cgi?id=17236&action=edit

------- Additional Comments from Alp Toker <alp at atoker.com>
>Index: WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp
>===================================================================
>--- WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp	(Revision 27755)
>+++ WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp	(Arbeitskopie)
>@@ -1,5 +1,6 @@
> /*
>  * Copyright (C) 2007 Holger Hans Peter Freyther
>+ * Copyright (C) 2007 Christian Dywan <christian at twotoasts.de>
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -195,40 +196,35 @@
> 
> void ChromeClient::addMessageToConsole(const WebCore::String& message,
unsigned int lineNumber, const WebCore::String& sourceId)
> {
>-    CString messageString = message.utf8();
>-    CString sourceIdString = sourceId.utf8();
>-
>-    WEBKIT_PAGE_GET_CLASS(m_webPage)->java_script_console_message(m_webPage,
messageString.data(), lineNumber, sourceIdString.data());
>+    gboolean retval;
>+    g_signal_emit_by_name(m_webPage, "console-message",
message.utf8().data(), lineNumber, sourceId.utf8().data(), &retval);
> }
> 
> void ChromeClient::runJavaScriptAlert(Frame* frame, const String& message)
> {
>-    CString messageString = message.utf8();
>-    WEBKIT_PAGE_GET_CLASS(m_webPage)->java_script_alert(m_webPage,
kit(frame), messageString.data());
>+    gboolean retval;
>+    g_signal_emit_by_name(m_webPage, "script-alert", kit(frame),
message.utf8().data(), &retval);
> }
> 
> bool ChromeClient::runJavaScriptConfirm(Frame* frame, const String& message)
> {
>-    CString messageString = message.utf8();
>-    return WEBKIT_PAGE_GET_CLASS(m_webPage)->java_script_confirm(m_webPage,
kit(frame), messageString.data());
>+    gboolean retval;
>+    gboolean didConfirm;
>+    g_signal_emit_by_name(m_webPage, "script-confirm", kit(frame),
message.utf8().data(), &didConfirm, &retval);
>+    return didConfirm == TRUE;
> }
> 
> bool ChromeClient::runJavaScriptPrompt(Frame* frame, const String& message,
const String& defaultValue, String& result)
> {
>-    CString messageString = message.utf8();
>-    CString defaultValueString = defaultValue.utf8(); 
>-
>-    gchar* cresult =
WEBKIT_PAGE_GET_CLASS(m_webPage)->java_script_prompt(m_webPage,
>-									       
kit(frame),
>-									       
messageString.data(),
>-									       
defaultValueString.data());
>-    if (!cresult)
>-	  return false;
>-    else {
>-	  result = String::fromUTF8(cresult);
>-	  g_free(cresult);
>+    gboolean retval;
>+    gchar* value = 0;
>+    g_signal_emit_by_name(m_webPage, "script-prompt", kit(frame),
message.utf8().data(), defaultValue.utf8().data(), &value, &retval);
>+    if (value) {
>+	  result = String::fromUTF8(value);
>+	  g_free(value);
>	  return true;
>     }
>+    return false;
> }
> 
> void ChromeClient::setStatusbarText(const String& string)
>Index: WebKit/gtk/ChangeLog
>===================================================================
>--- WebKit/gtk/ChangeLog	(Revision 27758)
>+++ WebKit/gtk/ChangeLog	(Arbeitskopie)
>@@ -1,3 +1,18 @@
>+2007-11-13  Christian Dywan  <christian at twotoasts.de>
>+
>+	  Reviewed by NOBODY (OOPS!).
>+
>+	  Implement signals for script dialogs and console messages.
>+
>+	  * Api/webkitgtk-marshal.list:
>+	  * Api/webkitgtkpage.cpp:
>+	  * Api/webkitgtkpage.h:
>+	  * WebCoreSupport/ChromeClientGtk.cpp:
>+	  (WebKit::ChromeClient::addMessageToConsole):
>+	  (WebKit::ChromeClient::runJavaScriptAlert):
>+	  (WebKit::ChromeClient::runJavaScriptConfirm):
>+	  (WebKit::ChromeClient::runJavaScriptPrompt):
>+
> 2007-11-11  Alp Toker  <alp at atoker.com>
> 
>	  Reviewed by Anders.
>Index: WebKit/gtk/Api/webkitgtk-marshal.list
>===================================================================
>--- WebKit/gtk/Api/webkitgtk-marshal.list	(Revision 27755)
>+++ WebKit/gtk/Api/webkitgtk-marshal.list	(Arbeitskopie)
>@@ -1,3 +1,7 @@
> VOID:STRING,STRING
> VOID:OBJECT,BOOLEAN
> VOID:OBJECT,OBJECT
>+BOOLEAN:STRING,INT,STRING
>+BOOLEAN:OBJECT,STRING
>+BOOLEAN:OBJECT,STRING,BOOLEAN
>+BOOLEAN:OBJECT,STRING,STRING,STRING
>Index: WebKit/gtk/Api/webkitgtkpage.cpp
>===================================================================
>--- WebKit/gtk/Api/webkitgtkpage.cpp	(Revision 27755)
>+++ WebKit/gtk/Api/webkitgtkpage.cpp	(Arbeitskopie)
>@@ -1,5 +1,6 @@
> /*
>  * Copyright (C) 2007 Holger Hans Peter Freyther
>+ * Copyright (C) 2007 Christian Dywan <christian at twotoasts.de>
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -64,6 +65,10 @@
>     STATUS_BAR_TEXT_CHANGED,
>     ICOND_LOADED,
>     SELECTION_CHANGED,
>+    CONSOLE_MESSAGE,
>+    SCRIPT_ALERT,
>+    SCRIPT_CONFIRM,
>+    SCRIPT_PROMPT,
>     LAST_SIGNAL
> };
> 
>@@ -215,33 +220,105 @@
>     return g_strdup(old_name);
> }
> 
>-static void webkit_page_real_java_script_alert(WebKitPage*, WebKitFrame*,
const gchar*)
>+typedef enum {
>+    WEBKIT_SCRIPT_DIALOG_ALERT,
>+    WEBKIT_SCRIPT_DIALOG_CONFIRM,
>+    WEBKIT_SCRIPT_DIALOG_PROMPT
>+ } WebKitJavaScriptDialogType;

Please rename this enumeration to WebKitScriptDialogType.

>+
>+static gboolean webkit_page_script_dialog(WebKitPage* page, WebKitFrame*
frame, const gchar* message, WebKitJavaScriptDialogType type, const gchar*
defaultValue, gchar** value)
> {
>-    notImplemented();
>+    GtkMessageType messageType;
>+    GtkButtonsType buttons;
>+    gint defaultResponse;
>+    GtkWidget* window;
>+    GtkWidget* dialog;
>+    GtkWidget* entry;
>+    gboolean didConfirm;
>+
>+    switch (type) {
>+
>+    case WEBKIT_SCRIPT_DIALOG_ALERT:
>+	  messageType = GTK_MESSAGE_WARNING;
>+	  buttons = GTK_BUTTONS_CLOSE;
>+	  defaultResponse = GTK_RESPONSE_CLOSE;
>+	  break;
>+
>+    case WEBKIT_SCRIPT_DIALOG_CONFIRM:
>+	  messageType = GTK_MESSAGE_QUESTION;
>+	  buttons = GTK_BUTTONS_YES_NO;
>+	  defaultResponse = GTK_RESPONSE_YES;
>+	  break;
>+
>+    case WEBKIT_SCRIPT_DIALOG_PROMPT:
>+	  messageType = GTK_MESSAGE_QUESTION;
>+	  buttons = GTK_BUTTONS_OK_CANCEL;
>+	  defaultResponse = GTK_RESPONSE_OK;
>+    }
>+
>+    window = gtk_widget_get_toplevel(GTK_WIDGET(page));
>+    dialog = gtk_message_dialog_new(
>+     GTK_WIDGET_TOPLEVEL(window) ? GTK_WINDOW(window) : 0
>+     , GTK_DIALOG_DESTROY_WITH_PARENT, messageType, buttons, message);
>+    gchar* title = g_strconcat("JavaScript - ",
webkit_frame_get_location(frame), 0);
>+    gtk_window_set_title(GTK_WINDOW(dialog), title);
>+    g_free(title);
>+
>+    if (type == WEBKIT_SCRIPT_DIALOG_PROMPT) {
>+	  entry = gtk_entry_new();
>+	  gtk_entry_set_text(GTK_ENTRY(entry), defaultValue);
>+	  gtk_container_add(GTK_CONTAINER(GTK_DIALOG(dialog)->vbox), entry);
>+	  gtk_entry_set_activates_default(GTK_ENTRY(entry), TRUE);
>+	  gtk_widget_show(entry);
>+    }
>+
>+    gtk_dialog_set_default_response(GTK_DIALOG(dialog), defaultResponse);
>+    gint response = gtk_dialog_run(GTK_DIALOG(dialog));
>+
>+    switch (response) {
>+
>+    case GTK_RESPONSE_YES:
>+	  didConfirm = TRUE;
>+	  break;
>+
>+    case GTK_RESPONSE_NO:
>+    case GTK_RESPONSE_CANCEL:
>+	  didConfirm = FALSE;
>+	  break;
>+
>+    case GTK_RESPONSE_OK:
>+	  didConfirm = TRUE;
>+	  *value = g_strdup(gtk_entry_get_text(GTK_ENTRY(entry)));
>+    }
>+    gtk_widget_destroy(GTK_WIDGET(dialog));
>+    return didConfirm;
> }
> 
>-static gboolean webkit_page_real_java_script_confirm(WebKitPage*,
WebKitFrame*, const gchar*)
>+static gboolean webkit_page_real_script_alert(WebKitPage* page, WebKitFrame*
frame, const gchar* message)
> {
>-    notImplemented();
>-    return FALSE;
>+    webkit_page_script_dialog(page, frame, message,
WEBKIT_SCRIPT_DIALOG_ALERT, 0, 0);
>+    return TRUE;
> }
> 
>-/**
>- * WebKitPage::java_script_prompt
>- *
>- * @return: NULL to cancel the prompt
>- */
>-static gchar* webkit_page_real_java_script_prompt(WebKitPage*, WebKitFrame*,
const gchar*, const gchar* defaultValue)
>+static gboolean webkit_page_real_script_confirm(WebKitPage* page,
WebKitFrame* frame, const gchar* message, gboolean* didConfirm)
> {
>-    notImplemented();
>-    return g_strdup(defaultValue);
>+    *didConfirm = webkit_page_script_dialog(page, frame, message,
WEBKIT_SCRIPT_DIALOG_CONFIRM, 0, 0);
>+    return TRUE;
> }
> 
>-static void webkit_page_real_java_script_console_message(WebKitPage*, const
gchar*, unsigned int, const gchar*)
>+static gboolean webkit_page_real_script_prompt(WebKitPage* page, WebKitFrame*
frame, const gchar* message, const gchar* defaultValue, gchar** value)
> {
>-    notImplemented();
>+    if (!webkit_page_script_dialog(page, frame, message,
WEBKIT_SCRIPT_DIALOG_PROMPT, defaultValue, value))
>+	  *value = g_strdup(defaultValue);
>+    return TRUE;
> }
> 
>+static gboolean webkit_page_real_console_message(WebKitPage* page, const
gchar* message, unsigned int line, const gchar* sourceId)
>+{
>+    LOG("console-message: %s@%d: %s\n", sourceId, line, message);
>+    return TRUE;
>+}
>+
> static void webkit_page_finalize(GObject* object)
> {
>     webkit_page_stop_loading(WEBKIT_PAGE(object));
>@@ -354,17 +431,97 @@
>	      g_cclosure_marshal_VOID__VOID,
>	      G_TYPE_NONE, 0);
> 
>+    /**
>+     * WebKitPage::console-message
>+     * @page: the object on which the signal is emitted
>+     * @message: the message text
>+     * @line: the line where the error occured
>+     * @source_id: the source id
>+     * @return: TRUE to stop other handlers from being invoked for the event.
FALSE to propagate the event further.
>+     *
>+     * A JavaScript console message was created.
>+     */
>+    webkit_page_signals[CONSOLE_MESSAGE] = g_signal_new("console_message",
>+	      G_TYPE_FROM_CLASS(pageClass),
>+	      (GSignalFlags)(G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION),
>+	      G_STRUCT_OFFSET(WebKitPageClass, console_message),
>+	      g_signal_accumulator_true_handled,
>+	      NULL,
>+	      webkit_marshal_BOOLEAN__STRING_INT_STRING,
>+	      G_TYPE_BOOLEAN, 3,
>+	      G_TYPE_STRING, G_TYPE_INT, G_TYPE_STRING);
> 
>+    /**
>+     * WebKitPage::script-alert
>+     * @page: the object on which the signal is emitted
>+     * @frame: the relevant frame
>+     * @message: the message text
>+     * @return: TRUE to stop other handlers from being invoked for the event.
FALSE to propagate the event further.
>+     *
>+     * A JavaScript alert dialog was created.
>+     */
>+    webkit_page_signals[SCRIPT_ALERT] = g_signal_new("alert",
>+	      G_TYPE_FROM_CLASS(pageClass),
>+	      (GSignalFlags)(G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION),
>+	      G_STRUCT_OFFSET(WebKitPageClass, script_alert),
>+	      g_signal_accumulator_true_handled,
>+	      NULL,
>+	      webkit_marshal_BOOLEAN__OBJECT_STRING,
>+	      G_TYPE_BOOLEAN, 2,
>+	      G_TYPE_OBJECT, G_TYPE_STRING);

You forgot to rename "alert" here, so it doesn't work. Should be
"script_alert"?


More information about the webkit-reviews mailing list