606548 - Start page options > Custom vs Current pages is redundant

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Tony Chung [:tchung]]

Attachment: Custom Page screenshot

The workflow of the start page options (Custom vs. Current Page) is confusing. If i selected a "Use Current Page", it quickly displays "Custom Page" in the UI.   If you select the droplist, you are presented an option to choose "Use Current Page" or "Custom Page".checked

Use Current Page and Custom Page serves the same purpose in this case.   I suggest removing "Custom Page" option altogether

See screenshot

Repro:
1) install android trunk nightly: Mozilla/5.0 (Android; Linux armv71; rv:2.0b8pre) Gecko/20101022 Firefox/4.0b8pre Fennec/4.0b2pre	
2) visit any url thats not about:firefox
3) options > Prefs > select Start page droplist
4) Choose "Use Current Page"
5) Notice Button switches to "Custom Page" instead of Current page
6) Select droplist again, verify there's both use Current page and custom page options.  both select the same thing, so it seems custom page is redundant.

Expected:
- stick with one choice "Use Current Page", and no more options

Actual:
- see screenshot

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

We add "Custom Page" only if the user has previously selected a "Use Current Page" item.

We show "Custom Page" to let the user know the a custom web page is being used as the Start Page. This custom page might not be the current page shown in the browser, so "Custom Page" is not equal to "Use Current Page"

Comment 2

[Tony Chung [:tchung]]

okay, then maybe we should hide "Use Current Page" option if the custom page is already set to the current page.   Keep the display as it is now, to "Use current page", if the default page gets set to something else. 

Just seems odd to be show both options when Current Page == Custom page scenario

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Good thought. Cc'ong Madhava to OK.

Comment 4

[Madhava Enros [:madhava]]

ah, just when you're currently on the page that is the custom page. Yeah - OK.

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

The patch hide the "Use Current Page" option if the user homepage is the current page.

Please note that if the user has never made a choice and the browser is on the "Start Page" (about:home) it will be only two choices on the list :

 - Fennec Start Page
 - Blank Page

instead of the normally 3 choices:

 - Fennec Start Page
 - Blank Page
 - Use Current Page

Mark,
in the patch I'm hiding the item in the raw menupopup and add it to the displayed list, I'm a bit hesitant about that, should I simply ignore it? Not sure if we want to keep it for some extensions purpose (not this case but some others - but I have no ideas of them)

Comment 6

[Madhava Enros [:madhava]]

Hm - so there will be a slight feature-discoverability hit here (for users exploring, without ever having changed the start page). I hadn't thought of that.  Would it be worth including the option but disabling it instead of removing it altogether?

Comment 7

[Matt Brubeck (:mbrubeck)]

(In reply to comment #5)
> The patch hide the "Use Current Page" option if the user homepage is the
> current page.

It seems more informative to hide "Custom Page" in this case and have "Use Current Page" be the selected item.  For one thing, this would mean that when you select "Use Current Page" from the dialog, it would remain selected instead of changing instantly to "Custom Page".

Madhava, what do you think?

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #7)
> (In reply to comment #5)
> > The patch hide the "Use Current Page" option if the user homepage is the
> > current page.
> 
> It seems more informative to hide "Custom Page" in this case and have "Use
> Current Page" be the selected item.

If we do this and the user has a custom page already selected, how would they pick a new custom page?

Comment 9

[Matt Brubeck (:mbrubeck)]

(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #5)
> > > The patch hide the "Use Current Page" option if the user homepage is the
> > > current page.
> > 
> > It seems more informative to hide "Custom Page" in this case and have "Use
> > Current Page" be the selected item.
> 
> If we do this and the user has a custom page already selected, how would they
> pick a new custom page?

We would hide "Custom Page" only if the *current page* is already selected as the home page.  If the user navigates to a different page, then "Custom Page" is selected and the user can choose "Use Current Page" to change the setting.

Comment 10

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.2

I've thought of it a bit during the past few days. 
I don't think we should hide the "Custom Page" row because it has an interesting property (not sure if it is a bug or not) but when you click on it and you have a previously selected start page, the start page preference is reverted to the default (I've integrated a fix into the code to show "Fennec Start" as the selected item in the list in this case)

I think the question to take a decision is: do we want to keep this behavior for "Custom Page" or not?

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #10)
> Created attachment 487230 [details] [diff] [review]
> Patch v0.2

> I think the question to take a decision is: do we want to keep this behavior
> for "Custom Page" or not?

I don't think we want this behavior. The toggle-action is not good since no other choices behave this way.

I think we are bike shedding this issue. What's the simplest approach?

I don't like Matt's approach, leaving "Use Custom Page" visible, since this is the action - set my home page to a page of my choice

I like leaving "Custom Page", which will be checked, since this is the state - you picked a custom page to be your home page

I do not like picking "Custom Page" to toggle the state back to the default - that is a bug.

Comment 12

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.3

Comment 13

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch workflow

This is the workflow implemented by the patch, the first column is when the user is on the about:home page to not penalized feature discoverability, the second column is the default behavior

Comment 14

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 488495 [details] [diff] [review]
Patch v0.3


>diff --git a/chrome/content/preferences.js b/chrome/content/preferences.js
>+  updateHomePageList: function updateHomePageMenuList() {
>+    // Update the "Use Current Page" item in the menulist by disabling it if
>+    // the current page is already the user homepage
>+    let currentUrl = Browser.selectedBrowser.currentURI.spec;
>+    let currentHomepage = Services.prefs.getComplexValue("browser.startup.homepage", Ci.nsIPrefLocalizedString).data;

This will throw if the pref is not set, and it did throw on my build. Use Browser.getHomePage()

r+ with nits fixed

Comment 15

[Mark Finkle (:mfinkle) (use needinfo?)]

fixed nits and pushed:
http://hg.mozilla.org/mobile-browser/rev/a47fe6468101

Comment 16

[Aakash Desai [:aakashd]]

verified FIXED on builds:
Mozilla/5.0 (Maemo; Linux armv71; rv:2.0b8pre) Gecko/20101108 Namoroka/4.0b8pre Fennec/4.0b3pre

and

Mozilla/5.0 (Android; Linux armv71; rv:2.0b8pre) Gecko/20101108 Namoroka/4.0b8pre Fennec/4.0b3pre

Comment 17

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

https://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=13839