After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 720876 - A few fixes for the location entry
A few fixes for the location entry
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: Interface
unspecified
Other All
: Normal normal
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
: 720869 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2013-12-21 14:22 UTC by William Jon McCann
Modified: 2014-01-07 23:09 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Don't show ephy-about: in the UI (1.82 KB, patch)
2013-12-21 14:22 UTC, William Jon McCann
committed Details | Review
Don't show null in page title for about:plugins (1.05 KB, patch)
2013-12-21 14:22 UTC, William Jon McCann
committed Details | Review
Always use a favicon in the drag surface (2.36 KB, patch)
2013-12-21 14:22 UTC, William Jon McCann
committed Details | Review
Center the favicon vertically on the drag surface (1.22 KB, patch)
2013-12-21 14:22 UTC, William Jon McCann
committed Details | Review
Add more padding around the drag surface (1.06 KB, patch)
2013-12-21 14:22 UTC, William Jon McCann
committed Details | Review
Increase the row padding in the completion menu (822 bytes, patch)
2013-12-21 14:22 UTC, William Jon McCann
committed Details | Review
Match spacing in current Adwaita (806 bytes, patch)
2013-12-21 14:22 UTC, William Jon McCann
committed Details | Review
Use a smaller text size for the url line in the completion menu (2.72 KB, patch)
2013-12-21 14:22 UTC, William Jon McCann
reviewed Details | Review
Use a smaller text size for the url line in the completion menu (2.85 KB, patch)
2014-01-07 22:43 UTC, William Jon McCann
none Details | Review
Use a smaller text size for the url line in the completion menu (2.53 KB, patch)
2014-01-07 23:07 UTC, William Jon McCann
committed Details | Review

Description William Jon McCann 2013-12-21 14:22:32 UTC
Here are a number of small fixes for the location entry, drag icon, and
completion list.
Comment 1 William Jon McCann 2013-12-21 14:22:34 UTC
Created attachment 264680 [details] [review]
Don't show ephy-about: in the UI
Comment 2 William Jon McCann 2013-12-21 14:22:38 UTC
Created attachment 264681 [details] [review]
Don't show null in page title for about:plugins

If for some reason an about: page doesn't load the next startup
will show one of the error screens. Those screens assume they
can get a hostname for the URL. There is no hostname for about:
schemes. So, in that case just display the entire URL.
Comment 3 William Jon McCann 2013-12-21 14:22:41 UTC
Created attachment 264682 [details] [review]
Always use a favicon in the drag surface
Comment 4 William Jon McCann 2013-12-21 14:22:43 UTC
Created attachment 264683 [details] [review]
Center the favicon vertically on the drag surface
Comment 5 William Jon McCann 2013-12-21 14:22:46 UTC
Created attachment 264684 [details] [review]
Add more padding around the drag surface
Comment 6 William Jon McCann 2013-12-21 14:22:49 UTC
Created attachment 264685 [details] [review]
Increase the row padding in the completion menu
Comment 7 William Jon McCann 2013-12-21 14:22:51 UTC
Created attachment 264686 [details] [review]
Match spacing in current Adwaita
Comment 8 William Jon McCann 2013-12-21 14:22:55 UTC
Created attachment 264687 [details] [review]
Use a smaller text size for the url line in the completion menu
Comment 9 William Jon McCann 2013-12-21 19:24:58 UTC
*** Bug 720869 has been marked as a duplicate of this bug. ***
Comment 10 Claudio Saavedra 2014-01-07 13:53:05 UTC
Review of attachment 264680 [details] [review]:

Marking as reviewed.

::: lib/widgets/ephy-location-entry.c
@@ +681,3 @@
+		*url = str;
+	}
+	/* Do not show internal ephy-about: protocol to users */

Not a big fan of the call by reference and posterior freeing, as it assumes memory is allocated and someone might misuse this.
Comment 11 Claudio Saavedra 2014-01-07 13:55:06 UTC
Review of attachment 264681 [details] [review]:

Good!
Comment 12 Claudio Saavedra 2014-01-07 13:59:03 UTC
Review of attachment 264682 [details] [review]:

Just a minor comment.

::: lib/widgets/ephy-location-entry.c
@@ +814,3 @@
 	g_free (title);
 	g_string_free (text, TRUE);
+	g_clear_object (&favicon);

I don't think you need g_clear_object() here, as it's a local variable. g_object_unref() should be enough.
Comment 13 Claudio Saavedra 2014-01-07 13:59:26 UTC
Review of attachment 264683 [details] [review]:

OK
Comment 14 Claudio Saavedra 2014-01-07 13:59:43 UTC
Review of attachment 264684 [details] [review]:

OK
Comment 15 Claudio Saavedra 2014-01-07 14:00:00 UTC
Review of attachment 264685 [details] [review]:

OK
Comment 16 Claudio Saavedra 2014-01-07 14:00:17 UTC
Review of attachment 264686 [details] [review]:

OK
Comment 17 Bastien Nocera 2014-01-07 14:12:44 UTC
Review of attachment 264687 [details] [review]:

::: lib/widgets/ephy-location-entry.c
@@ +1096,3 @@
 					     &color);
 
+		color_text = g_strdup_printf ("#%02X%02X%02X",

I'd split this into a rgba_to_hex() function. Also, as pango can quite "large" #ffffffffffff type hex values for colours (see pango_color_parse), I'd multiply by 65535 and drop the rounding.
Comment 18 William Jon McCann 2014-01-07 22:33:25 UTC
(In reply to comment #12)
> I don't think you need g_clear_object() here, as it's a local variable.
> g_object_unref() should be enough.

It is a convenient null check.
Comment 19 William Jon McCann 2014-01-07 22:43:11 UTC
Created attachment 265607 [details] [review]
Use a smaller text size for the url line in the completion menu
Comment 20 William Jon McCann 2014-01-07 23:07:24 UTC
Created attachment 265611 [details] [review]
Use a smaller text size for the url line in the completion menu
Comment 21 William Jon McCann 2014-01-07 23:09:17 UTC
Attachment 264680 [details] pushed as e504fb8 - Don't show ephy-about: in the UI
Attachment 264681 [details] pushed as fb0de41 - Don't show null in page title for about:plugins
Attachment 264682 [details] pushed as 4f76ca4 - Always use a favicon in the drag surface
Attachment 264683 [details] pushed as 0b1e5e0 - Center the favicon vertically on the drag surface
Attachment 264684 [details] pushed as 4fc7602 - Add more padding around the drag surface
Attachment 264685 [details] pushed as a22a8ae - Increase the row padding in the completion menu
Attachment 264686 [details] pushed as a576059 - Match spacing in current Adwaita
Attachment 265611 [details] pushed as a61723c - Use a smaller text size for the url line in the completion menu