Bug 34219 – [Gtk] Server message not shown on http authentication

• Component: WebKitGTK
• Status: RESOLVED FIXED

[José Millán Soto]

Server message is not shown on http authentication dialog.

Epiphany bug report: https://bugzilla.gnome.org/show_bug.cgi?id=604179

[José Millán Soto]

Created attachment 47546 [details]
Patch to make server message visible in http authentication dialog

This patch makes server message visible in authentication dialog.

[WebKit Review Bot]

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.

[Gustavo Noronha (kov)]

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.

[José Millán Soto]

Created attachment 47723 [details]
 Patch to make server message visible in http authentication dialog

This patch solves the problems commented in comment 3.

[WebKit Review Bot]

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.

[Xan Lopez]

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.

[José Millán Soto]

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.

[WebKit Review Bot]

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.

[José Millán Soto]

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.

[José Millán Soto]

Created attachment 47947 [details]
Patch to make server message visible in http authentication dialog

[WebKit Review Bot]

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.

[Xan Lopez]

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 ""?

[José Millán Soto]

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)

[WebKit Review Bot]

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.

[Xan Lopez]

Comment on attachment 49098 [details]
Patch to make server message visible in http authentication dialog

Looks good to me.

[WebKit Commit Bot]

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>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.