Bug 34219

Summary: [Gtk] Server message not shown on http authentication
Product: WebKit Reporter: José Millán Soto <jmillan>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, jmillan, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Patch to make server message visible in http authentication dialog
gustavo: review-
Patch to make server message visible in http authentication dialog
xan.lopez: review-
Patch to make server message visible in http authentication dialog
none
Patch to make server message visible in http authentication dialog
jmillan: review-
Patch to make server message visible in http authentication dialog
xan.lopez: review-
Patch to make server message visible in http authentication dialog none

Description José Millán Soto 2010-01-27 10:14:42 PST
Server message is not shown on http authentication dialog.

Epiphany bug report: https://bugzilla.gnome.org/show_bug.cgi?id=604179
Comment 1 José Millán Soto 2010-01-27 10:23:37 PST
Created attachment 47546 [details]
Patch to make server message visible in http authentication dialog

This patch makes server message visible in authentication dialog.
Comment 2 WebKit Review Bot 2010-01-27 10:29:32 PST
Attachment 47546 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKit/gtk/webkit/webkitsoupauthdialog.c:193:  Declaration has space between * and variable name in GtkWidget* serverMessageDescriptionLabel  [whitespace/declaration] [3]
WebKit/gtk/webkit/webkitsoupauthdialog.c:194:  Declaration has space between * and variable name in GtkWidget* serverMessageLabel  [whitespace/declaration] [3]
WebKit/gtk/webkit/webkitsoupauthdialog.c:195:  Declaration has space between * and variable name in GtkWidget* descriptionLabel  [whitespace/declaration] [3]
Total errors found: 3


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Gustavo Noronha (kov) 2010-01-28 12:32:36 PST
Comment on attachment 47546 [details]
Patch to make server message visible in http authentication dialog

 196     char *description;

This * is misplaced in comparison to the others.

 272     serverMessageLabel = gtk_label_new(_(soup_auth_get_realm(authData->auth)));

I don't think we want to translate the server's message.
Comment 4 José Millán Soto 2010-01-29 11:05:48 PST
Created attachment 47723 [details]
 Patch to make server message visible in http authentication dialog

This patch solves the problems commented in comment 3.
Comment 5 WebKit Review Bot 2010-01-29 11:10:44 PST
Attachment 47723 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKit/gtk/webkit/webkitsoupauthdialog.c:193:  Declaration has space between * and variable name in GtkWidget* serverMessageDescriptionLabel  [whitespace/declaration] [3]
WebKit/gtk/webkit/webkitsoupauthdialog.c:194:  Declaration has space between * and variable name in GtkWidget* serverMessageLabel  [whitespace/declaration] [3]
WebKit/gtk/webkit/webkitsoupauthdialog.c:195:  Declaration has space between * and variable name in GtkWidget* descriptionLabel  [whitespace/declaration] [3]
WebKit/gtk/webkit/webkitsoupauthdialog.c:196:  Declaration has space between * and variable name in char* description  [whitespace/declaration] [3]
Total errors found: 4


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Xan Lopez 2010-01-31 14:22:39 PST
Comment on attachment 47723 [details]
 Patch to make server message visible in http authentication dialog

I think the "Server message: " stuff should only be shown if there's actually a message, not unconditionally.
Comment 7 José Millán Soto 2010-02-02 09:53:27 PST
Created attachment 47943 [details]
Patch to make server message visible in http authentication dialog

This version of the patch checks if realm is an empty string, and if that's the case it doesn't show "Server Message:" label in authentication dialog.
Comment 8 WebKit Review Bot 2010-02-02 09:57:49 PST
Attachment 47943 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKit/gtk/webkit/webkitsoupauthdialog.c:193:  Declaration has space between * and variable name in GtkWidget* serverMessageDescriptionLabel  [whitespace/declaration] [3]
WebKit/gtk/webkit/webkitsoupauthdialog.c:194:  Declaration has space between * and variable name in GtkWidget* serverMessageLabel  [whitespace/declaration] [3]
WebKit/gtk/webkit/webkitsoupauthdialog.c:195:  Declaration has space between * and variable name in GtkWidget* descriptionLabel  [whitespace/declaration] [3]
WebKit/gtk/webkit/webkitsoupauthdialog.c:196:  Declaration has space between * and variable name in char* description  [whitespace/declaration] [3]
WebKit/gtk/webkit/webkitsoupauthdialog.c:273:  Should have a space between // and comment  [whitespace/comments] [4]
Total errors found: 5


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 José Millán Soto 2010-02-02 10:11:01 PST
Created attachment 47946 [details]
Patch to make server message visible in http authentication dialog

Solves an style problem that had last patch with one comment.
Comment 10 José Millán Soto 2010-02-02 10:13:56 PST
Created attachment 47947 [details]
Patch to make server message visible in http authentication dialog
Comment 11 WebKit Review Bot 2010-02-02 10:20:18 PST
Attachment 47947 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKit/gtk/webkit/webkitsoupauthdialog.c:193:  Declaration has space between * and variable name in GtkWidget* serverMessageDescriptionLabel  [whitespace/declaration] [3]
WebKit/gtk/webkit/webkitsoupauthdialog.c:194:  Declaration has space between * and variable name in GtkWidget* serverMessageLabel  [whitespace/declaration] [3]
WebKit/gtk/webkit/webkitsoupauthdialog.c:195:  Declaration has space between * and variable name in GtkWidget* descriptionLabel  [whitespace/declaration] [3]
WebKit/gtk/webkit/webkitsoupauthdialog.c:196:  Declaration has space between * and variable name in char* description  [whitespace/declaration] [3]
Total errors found: 4


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Xan Lopez 2010-02-04 14:04:15 PST
Comment on attachment 47947 [details]
Patch to make server message visible in http authentication dialog

A couple of things:

- You should probably create 2 or 3 rows depending or whether there's a realm or not, not always create 3 and then not use the first one.

- You are checking that the string is not NULL, but I guess you should also check that it's not ""?
Comment 13 José Millán Soto 2010-02-19 11:29:34 PST
Created attachment 49098 [details]
Patch to make server message visible in http authentication dialog

This one creates only three rows if there's a realm.

(In reply to comment #12)
> - You are checking that the string is not NULL, but I guess you should also
> check that it's not ""?
Actually, I was checking that if it was "" (by checking realm[0]) and I was not checking if it was NULL.

Now, I use strlen instead of doing realm[0] to check if it's an empty string and it's also checked if it's NULL (soup_auth_get_realm should not return NULL, but just in case)
Comment 14 WebKit Review Bot 2010-02-19 11:32:16 PST
Attachment 49098 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKit/gtk/webkit/webkitsoupauthdialog.c:193:  Declaration has space between * and variable name in GtkWidget* serverMessageDescriptionLabel  [whitespace/declaration] [3]
WebKit/gtk/webkit/webkitsoupauthdialog.c:194:  Declaration has space between * and variable name in GtkWidget* serverMessageLabel  [whitespace/declaration] [3]
WebKit/gtk/webkit/webkitsoupauthdialog.c:195:  Declaration has space between * and variable name in GtkWidget* descriptionLabel  [whitespace/declaration] [3]
WebKit/gtk/webkit/webkitsoupauthdialog.c:196:  Declaration has space between * and variable name in char* description  [whitespace/declaration] [3]
Total errors found: 4 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 Xan Lopez 2010-02-23 01:19:58 PST
Comment on attachment 49098 [details]
Patch to make server message visible in http authentication dialog

Looks good to me.
Comment 16 WebKit Commit Bot 2010-02-23 03:26:05 PST
Comment on attachment 49098 [details]
Patch to make server message visible in http authentication dialog

Clearing flags on attachment: 49098

Committed r55139: <http://trac.webkit.org/changeset/55139>
Comment 17 WebKit Commit Bot 2010-02-23 03:26:10 PST
All reviewed patches have been landed.  Closing bug.