Hide urlbar contents on about:home

VERIFIED FIXED

Status

VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: mbrubeck, Assigned: mbrubeck)

Tracking

Trunk
x86
Linux

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

9 years ago
Fennec currently hides the URI and title on "about:blank".  This patch gives the same treatment to the start page (about:home).  This has the following advantages:

- The start page can now have a real page title, instead of "Enter Search or Address" (an attempt to mimic the urlbar's emptytext, which appears in the wrong color compared to real emptytext).

- "about:home" does not appear in the urlbar the first time the user touches it, potentially confusing some users.

- The patch also replaces several hard-coded "about:blank" special cases into a descriptively-named function.
(Assignee)

Comment 1

9 years ago
Created attachment 437054 [details] [diff] [review]
patch
Attachment #437054 - Flags: feedback?(mark.finkle)
Comment on attachment 437054 [details] [diff] [review]
patch

>diff -r 9f05fbde842c chrome/content/aboutHome.xhtml

>-  <title>&urlbar.emptytext;</title>
>+  <title>&homepage.default;</title>

In this case, we really do want the "Enter Search or Address" text here. It's more important to show the hint than to show a real page title.


I wonder if we should put _hideUrlbarText in Util, but rename to isURIEmpty(aURI)

The patch isn't bad otherwise.
Attachment #437054 - Flags: feedback?(mark.finkle) → feedback-
(Assignee)

Comment 3

9 years ago
(In reply to comment #2)
> In this case, we really do want the "Enter Search or Address" text here. It's
> more important to show the hint than to show a real page title.

Mark, the other changes in this patch cause the real hint to always show on about:home regardless of the page title (just as it does on about:blank).  Users won't see "Firefox Start" in the urlbar, but they will see it in the awesomebar if they bookmark the page or if we decide to include "about:" URIs in history.
Comment on attachment 437054 [details] [diff] [review]
patch

OK, I see how the <title> is handled now. Pulling in preferences.dtd isn't my favorite thing, but being able to bookmark the page with a good title is nice.

I do think the Util function will serve us better.
(Assignee)

Comment 5

9 years ago
There's also a regression caused by this patch, something to do with bug 521828 - the <title> doesn't display on the first pageload in some cases.  I'll try to fix that.
(Assignee)

Updated

9 years ago
Assignee: nobody → mbrubeck
Status: NEW → ASSIGNED
Summary: [patch] Hide urlbar contents on about:home → Hide urlbar contents on about:home
(Assignee)

Comment 6

9 years ago
Created attachment 437400 [details] [diff] [review]
revised patch

Here's a version of the patch that incorporates feedback and fixes the problems mentioned above.  This patch now depends on a separate patch from bug 521828 (attachment 437399 [details] [diff] [review]).
Attachment #437054 - Attachment is obsolete: true
Attachment #437400 - Flags: feedback?(mark.finkle)
(Assignee)

Comment 7

9 years ago
Created attachment 437401 [details] [diff] [review]
patch v3

Argh, a few lines got into that patch that weren't supposed to.  Fixed.
Attachment #437400 - Attachment is obsolete: true
Attachment #437401 - Flags: feedback?(mark.finkle)
Attachment #437400 - Flags: feedback?(mark.finkle)
Attachment #437401 - Flags: feedback?(mark.finkle) → feedback-
Comment on attachment 437401 [details] [diff] [review]
patch v3

>diff -r 067718acefeb chrome/content/Util.js

>+  /** Don't display anything in the urlbar for these special URIs. */
>+  isURIEmpty: function isURIEmpty(aURI) {
>+    return (!aURI || aURI == "about:blank" || aURI == "about:home");
>+  },

Let's name this isURLEmpty. We should try to separate the naming of URL and URI a bit more. I think a good convention would be: use "URI" if the variable or function uses (or returns) a nsIURI. Otherwise, use "URL" if the variable or function uses (or returns) a string.

We haven't been good at doing this separation in the past.

>diff -r 067718acefeb chrome/content/browser-ui.js

>   _titleChanged : function(aDocument) {

>-    var caption = aDocument.title;
>-    if (!caption) {
>-      caption = this.getDisplayURI(browser);
>-      if (caption == "about:blank")
>-        caption = "";
>-    }
>+    var uri = this.getDisplayURI(browser);
>+    var caption = aDocument.title || uri;
>+
>+    if (Util.isURIEmpty(uri))
>+      caption = "";

Can we keep the logic the way it was, or change it so getDisplayURI is not called unless we need it to be called.

>   _networkStart: function _networkStart(aRequest) {
>-    let uri = aRequest.QueryInterface(Ci.nsIChannel).URI.spec;
>+    // For chrome URIs especially, we want the urlbar during loading to use the
>+    // "original" URI (about:home), not a rewritten one (jar:file:///...).
>+    let uri = aRequest.QueryInterface(Ci.nsIChannel).originalURI.spec;
>     this._tab.startLoading(uri);

I don't think we need this part
(Assignee)

Comment 9

9 years ago
Created attachment 437956 [details] [diff] [review]
patch v4

Updated patch:

* Renamed function to isURLEmpty.
* Removed changes to _networkStart.

This still relies on nsIChannel.originalURI.  This is slightly ugly, but I think it's the right thing here - it also fixes a regression from bug 521828 where about:config displays a jar: URL in the urlbar.

(in response to comment #8)
> Can we keep the logic the way it was, or change it so
> getDisplayURI is not called unless we need it to be called.

Sorry, I need to call getDisplayURI every time to decide whether to show the title.
Attachment #437401 - Attachment is obsolete: true
Attachment #437956 - Flags: review?(mark.finkle)
Attachment #437956 - Flags: review?(mark.finkle) → review+
pushed:
http://hg.mozilla.org/mobile-browser/rev/3ef5eb570bc2
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
verified FIXED On builds:

Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2.4pre) Gecko/20100412 Namoroka/3.6.4pre Fennec/1.1a2pre

and

Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.3a4pre) Gecko/20100412 Namoroka/3.7a4pre Fennec/1.1a2pre
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.