If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Replace "about:home" with "Firefox Start Page" in preferences dialog

VERIFIED FIXED in Firefox 4.0b7

Status

()

Firefox
Preferences
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: beltzner, Assigned: Gavin)

Tracking

Trunk
Firefox 4.0b7
Points:
---

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

(Whiteboard: [about-home])

Attachments

(1 attachment, 5 obsolete attachments)

bug 590877 made "about:home" the default homepage URI for Firefox

In the preferences window, we should display "about:home" as "Firefox Start", instead, which is more human readable.
Sorry to file this so late, but pretty important polish IMO.
blocking2.0: --- → beta6+
Whiteboard: [strings]
Shorlander points out that this should also be the string used for the tooltip when you hover over the "Default" button.

Marco points out that if we do this, we'll have to add some checking to ensure that if the user edits the value, they end up with a properly formed URI or set of URIs. I propose that in cases where the user edits and leaves a bad value, we just revert to about:home for them :)
Marco is sick today, so if someone's bored and wants to take this, feel free!
Whiteboard: [strings] → [strings][needs patch]
I have a wip patch actually, so it could be attached in an hour or so...
Created attachment 474778 [details] [diff] [review]
pref part v1.0

this is the pref part... I thought of various ways like using stack or decks to show a fake inputfield, but this looks cleaner based on prefwindow APIs.

I don't have a patch for the home button tooltip yet, but I think having them separate and getting feedback earlier is better.
Feel free to make a modified patch from this as well, since I'm not 100% active today.
Attachment #474778 - Flags: review?(gavin.sharp)
Created attachment 474786 [details] [diff] [review]
tooltip part v1.0

and this could be the tooltip part.

With this I guess I'm givin up for today, hope it will be better tomorrow
Attachment #474786 - Flags: review?(gavin.sharp)
Comment on attachment 474786 [details] [diff] [review]
tooltip part v1.0

>diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js

>@@ -2979,8 +2979,7 @@ function openHomeDialog(aURL)

>-      var homeButton = document.getElementById("home-button");
>-      homeButton.setAttribute("tooltiptext", aURL);
>+      gHomeButton.updateTooltip();

I think you can just remove this entirely - the pref observer already handles updating the tooltip text.

>+      if (homePage == "about:home")

Hmm, was going to suggest comparing to the default pref value... But I guess that might not always be correct if someone ships a different default home page, so maybe this is better off as-is.
Attachment #474786 - Flags: review?(gavin.sharp) → review+
Given that we're re-using the existing about:home string, I think this may not actually be [strings]...
Created attachment 475168 [details] [diff] [review]
alternative patch

I was thinking we could just do something like this, which is somewhat simpler than your patch (though maybe not as user-friendly?).
Attachment #475168 - Flags: feedback?(mak77)
Whiteboard: [strings][needs patch] → [strings]
(In reply to comment #9)
> Created attachment 475168 [details] [diff] [review]
> alternative patch
> 
> I was thinking we could just do something like this, which is somewhat simpler
> than your patch (though maybe not as user-friendly?).

well it's pretty bad for the user because when he will start to edit that, he will end up editing something like "Firefox Sta" and he could end up saving that as homepage. I did the onfocus/onblur changes exactly to avoid putting them in a situation that could confuse them on what to do.
and at that point we should handle that case as Mike suggested, checking if the value is a valid url. but that is again tricky because that field can also contain multiple urls separated by "|".
And if we change the title of the page, we change the text the user has to enter. in syncToHomePref you compare to aboutHomeTitle but that's different based on the fact this is minefield, firefox or some other codename.
(In reply to comment #7)
> Comment on attachment 474786 [details] [diff] [review]
> tooltip part v1.0
> 
> >diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js
> 
> >@@ -2979,8 +2979,7 @@ function openHomeDialog(aURL)
> 
> >-      var homeButton = document.getElementById("home-button");
> >-      homeButton.setAttribute("tooltiptext", aURL);
> >+      gHomeButton.updateTooltip();
> 
> I think you can just remove this entirely - the pref observer already handles
> updating the tooltip text.

you are absolutely right, gHomeButton object does this by itself.

> >+      if (homePage == "about:home")
> 
> Hmm, was going to suggest comparing to the default pref value... But I guess
> that might not always be correct if someone ships a different default home
> page, so maybe this is better off as-is.

I think this code is too bound to our dtd and behavior to replace all default homepages to aboutHomeTitle string :)
(In reply to comment #10)
> well it's pretty bad for the user because when he will start to edit that, he
> will end up editing something like "Firefox Sta" and he could end up saving
> that as homepage.

It was already possible to enter invalid home page values - this only makes things slightly worse, because the field starts out containing non-URL text (which may tempt them to insert some other non-magical-non-URL string in the hopes of it working). If they do get into that state they can easily revert to default with the button, or just fix the entry manually again, though, so I'm not sure that's a blocking issue.

(In reply to comment #11)
> And if we change the title of the page, we change the text the user has to
> enter.

I don't think that's a problem - we're unlikely to change the title very often, and even if we do, we're not really trying to support the user entering this string manually, since there's a button that can be used to achieve the same effect. I included the onSyncToPreference just because it was easy to do.
(In reply to comment #13)
> I don't think that's a problem - we're unlikely to change the title very often,

it's a problem at least when moving between minefield and firefox for example, where titles are different
I'm not sure, it looks really unpolished to me without the focus/blur change, like if we were unable to make a temporary change and we just put some text in the field.
The UI here is really broken, but I guess we could do something like the following, which isn't too bad:* Default to "Firefox Start" as the value in the field, rendered as placeholder (gray) text.* If you focus the field and start typing, that will replace the placeholder text.* If you delete the string in the field, it reverts to "Firefox Start" as the placeholder text.* If you really want a blank page as your startup page, you would have to know that about:blank exists, but I really think this is an acceptable compromise, since there is a "Show a blank page" in the pulldown above ("When Firefox starts…[  |v] ).That way, Firefox Start is the implicit value of the field if you don't specify anything.I'd really like us to avoid having "magical strings" that mean a certain thing here. :)
Sorry, bugzilla broke my formatting :(

Again:

The UI here is really broken, but I guess we could do something like the
following, which isn't too bad:

* Default to "Firefox Start" as the value in the field, rendered as placeholder
(gray) text.

* If you focus the field and start typing, that will replace the placeholder
text.

* If you delete the string in the field, it reverts to "Firefox Start" as the
placeholder text.

* If you really want a blank page as your startup page, you would have to know
that about:blank exists, but I really think this is an acceptable compromise,
since there is a "Show a blank page" in the pulldown above ("When Firefox
starts…[  |v] ).

That way, Firefox Start is the implicit value of the field if you don't specify
anything.

I'd really like us to avoid having "magical strings" that mean a certain thing
here. :)
(In reply to comment #14)> it's a problem at least when moving between minefield and firefox for example,> where titles are differentWhy? The pref value remains about:home before and after, it's just the string that appears in the textbox that changes depending on the build you're running. "Reset to default" keeps working too. The only thing that doesn't work is manually entering "Firefox Start Page" in Minefield, and what I was trying to say in the second part of comment 13 is that we don't really care to support that.
oh and yes, this is not [strings] since we can reuse the text.
Whiteboard: [strings]
I still don't like the fact users have to type "Firefox Start Page" magic text in a url (plus weird "|" char) field (Apropos, you should compare lowercased values), that text changes based on locale and branding.

Limi's suggestion to use placeholder could work, we could even ideally remove the "restore default" button if users get used to clear the field to use default. It would practically work like my patch but without the focus/blur stuff.
Created attachment 475277 [details] [diff] [review]
patch for limi's suggestion (use placeholder text)

This turned out to be not so difficult... this patch:
- shows the about:home title as placeholder text if your home page is about:home
- if you click "reset to default", or empty the textbox manually, the home page is set to "about:home" and the textbox is empty (shows placeholder text)
- supports existing users that have the pref set to "" by showing about:blank for that case

This makes it impossible to actually set the pref to "" (which we treat the same way as about:blank) in the pref dialog, but I don't think that's really an issue.
Created attachment 475280 [details] [diff] [review]
patch for limi's suggestion (use placeholder text, cleaned up)
Attachment #475277 - Attachment is obsolete: true
Attachment #475280 - Flags: feedback?(mak77)
(In reply to comment #19)
> I still don't like the fact users have to type "Firefox Start Page" magic text
> in a url (plus weird "|" char) field (Apropos, you should compare lowercased
> values), that text changes based on locale and branding.

There's no reason that they would ever have to type it, though. There's a button that does it for them.
Attachment #475168 - Attachment is obsolete: true
Attachment #475168 - Flags: feedback?(mak77)
Good catch re: case-insensitive compares - would need to roll that into the patch as well.
Comment on attachment 475280 [details] [diff] [review]
patch for limi's suggestion (use placeholder text, cleaned up)


>diff --git a/browser/components/preferences/main.js b/browser/components/preferences/main.js
>--- a/browser/components/preferences/main.js
>+++ b/browser/components/preferences/main.js
>@@ -69,16 +69,46 @@ var gMainPane = {
>    *     2: the last page the user visited (DEPRECATED)
>    *     3: windows and tabs from the last session (a.k.a. session restore)
>    *
>    *   The deprecated option is not exposed in UI; however, if the user has it
>    *   selected and doesn't change the UI for this preference, the deprecated
>    *   option is preserved.
>    */
> 
>+  syncFromHomePref: function (textbox)
>+  {
>+    let aboutHomeTitle = textbox.getAttribute("abouthometitle");

this does not exist anymore in the patch

>+    let homePref = document.getElementById("browser.startup.homepage");
>+
>+    // If the pref is set to about:home, set the value to "" to show the
>+    // placeholder text (about:home title)

end comment with period

>+    if (homePref.value == "about:home")
>+      return "";
>+
>+    // If the pref is actually "", show about:blank - the actual home page
>+    // loading code treats them the same, and we don't want the placeholder text
>+    // showing.

replace - with comma and double spacing
", and..." should probably be "but we don't want to show the placeholder text for it."

>+    if (homePref.value == "")
>+      return "about:blank";
>+
>+    // Else show the actual pref value

end with comma

>+    return undefined;
>+  },
>+
>+  syncToHomePref: function (textbox)
>+  {
>+    // If the value is "", use about:home

ditto

>+    if (textbox.value == "")
>+      return "about:home";
>+
>+    // Else use the actual textbox value

ditto

should be fine, will test it briefly but I like it
Attachment #475280 - Flags: feedback?(mak77) → feedback+
Comment on attachment 474786 [details] [diff] [review]
tooltip part v1.0

Split this off to bug 596409.
Attachment #474786 - Attachment is obsolete: true
Attachment #474778 - Attachment is obsolete: true
Attachment #474778 - Flags: review?(gavin.sharp)
Created attachment 475301 [details] [diff] [review]
updated patch
Assignee: mak77 → gavin.sharp
Attachment #475280 - Attachment is obsolete: true
Status: NEW → ASSIGNED
blocking2.0: beta7+ → betaN+

Updated

7 years ago
Attachment #475301 - Flags: review+
Just to clarify; I didn't actually mean that the placeholder text should say "about:home", but that it should say "Firefox Start". What I didn't want was a "magical string" of any kind, but that probably wasn't what was proposed. :)

So, when there isn't any content in the string field, it says "Firefox Start" in the gray placeholder text.
(In reply to comment #27)
> So, when there isn't any content in the string field, it says "Firefox Start"
> in the gray placeholder text.

Yes, that's what the patch implements.
Comment on attachment 475301 [details] [diff] [review]
updated patch

Fine with alternate solution, yeah.
Attachment #475301 - Flags: ui-review+
http://hg.mozilla.org/mozilla-central/rev/ce5800998e6e
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b7

Comment 31

7 years ago
Verified fixed

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100916 Firefox/4.0b7pre
Status: RESOLVED → VERIFIED
Summary: Replace "about:home" with "Firefox Start" in preferences dialog → Replace "about:home" with "Firefox Start Page" in preferences dialog

Updated

7 years ago
Whiteboard: [about-home]
You need to log in before you can comment on or make changes to this bug.