Closed Bug 1227348 Opened 4 years ago Closed 4 years ago

Add "Use default homepage" to homepage preference

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 45
Tracking Status
firefox45 --- verified

People

(Reporter: mcomella, Assigned: mcomella)

References

Details

Attachments

(5 files, 2 obsolete files)

Follow-up to bug 1224214. See antlam's mock:
  https://bug1224214.bmoattachments.org/attachment.cgi?id=8689708

We should uplift this if we're allowed to uplift w/ the string, otherwise tracking 45.
Margaret, you raised concerns that we shouldn't show "about:home" but rather something more user-friendly like, "Firefox Homepage" – are we going with about:home or the latter?
Flags: needinfo?(margaret.leibovic)
Spoke on IRC – we're going to use something more user-friendly like, "Firefox Home" or "Firefox Home Page".
Flags: needinfo?(margaret.leibovic)
Bug 1227348 - Remove imports, provide better access levels. r=margaret
Attachment #8691608 - Flags: review?(margaret.leibovic)
Bug 1227348 - Add default homepage checkbox to homepage preference. r=margaret

The dialog will display "Firefox Home" when you click "Use default homepage"
but the Settings menu item will list the preference value directly (i.e. it
will be about:home).
Attachment #8691609 - Flags: review?(margaret.leibovic)
Bug 1227348 - Update homepage preference dialog title to match mock. r=margaret
Attachment #8691610 - Flags: review?(margaret.leibovic)
Comment on attachment 8691608 [details]
MozReview Request: Bug 1227348 - Remove imports, provide better access levels. r=margaret

https://reviewboard.mozilla.org/r/26153/#review23583
Attachment #8691608 - Flags: review?(margaret.leibovic) → review+
Attachment #8691609 - Flags: review?(margaret.leibovic)
Comment on attachment 8691609 [details]
MozReview Request: Bug 1227348 - Add default homepage radio buttons to homepage preference. r=margaret

https://reviewboard.mozilla.org/r/26155/#review23585

::: mobile/android/base/preferences/SetHomepagePreference.java:114
(Diff revision 1)
> +                homepageEditText.setText(defaultHomepagePlaceholderName);

I think we can use the "hint" attribute for the placehodler text. We could set that in XML, and then we don't need to worry about restting the string manually in code like this.
Comment on attachment 8691610 [details]
MozReview Request: Bug 1227348 - Update homepage preference dialog title to match mock. r=margaret

https://reviewboard.mozilla.org/r/26157/#review23587

::: mobile/android/base/locales/en-US/android_strings.dtd:197
(Diff revision 1)
> +<!ENTITY home_homepage_dialog_title "Set a URL">

I'm not convinced this is the best string, given that the user can put whatever text they want in here, not necessarily a URL.

I agree it's weird to see the word "homepage" all over the place, but I think we should look at the overall design to address this, rather than just changing this one string.
Attachment #8691610 - Flags: review?(margaret.leibovic)
https://reviewboard.mozilla.org/r/26155/#review23585

> I think we can use the "hint" attribute for the placehodler text. We could set that in XML, and then we don't need to worry about restting the string manually in code like this.

hint won't work because it's not a placeholder for the empty state, it's a placeholder for when the "use default homepage" box is checked. I'll rename the variable to make it more clear.
Anthony, as per comment 8, do you have a better suggestion for the title of the dialog? Alternatively, should I see if I can remove the title?

> I agree it's weird to see the word "homepage" all over the place, but I think we should look at the
> overall design to address this, rather than just changing this one string.

fwiw, this was my interpretation and suggestion, not something I got from Anthony.
Flags: needinfo?(alam)
Anthony, here's what I have so far:
  https://people.mozilla.com/~mcomella/apks/mcomella-1227348_01.apk

What do you think? Note that it will restore what you were typing if you toggle the "Use default homepage" twice.
Assignee: nobody → michael.l.comella
Attached image prev_set_dialog5.png
Thanks Mike! Sorry I haven't been able to finalize this design earlier. But I think I've got it!

I tried out the build and it looks like we have to do something more "custom" than I'd like. I originally tried to use platform conventions too much here and as a result I think the UX isn't great.

So, could we do this? I think the radio buttons more accurately show the OR relationship here and the input box next to it just behaves as it does currently.

Check https://invis.io/4G539UBW7 for some interactivity :)
Flags: needinfo?(alam) → needinfo?(michael.l.comella)
We talked on IRC – this design is non-trivial because making the text to the side of a RadioButton editable is not built into the framework. Instead, we make the EditText appear when the "Custom" RadioButton is pressed.

Anthony, here's a build: https://people.mozilla.com/~mcomella/apks/mcomella-1227348_02.apk

What say you? Note that the title of the dialog is incorrect and should be "Set a Homepage".
Flags: needinfo?(michael.l.comella) → needinfo?(alam)
Attached image screenshot.png (obsolete) —
Thanks for the build! just a few nits.

 - font color looks a bit light, can we use our text grey for the options? not including the placeholder of "Search or enter address" (that one can probably be the font color of the current options ("Custom" and "Use Firefox Home")
 - Remove "Use" from "Use Firefox Home"
 - span input field of "Custom" to be full length all the time (as it is if the field was full - as in the attachment)

that should do it!
Flags: needinfo?(alam) → needinfo?(michael.l.comella)
Anthony, what version of Android are you running? I hit bug 1220315 on my device, but you don't seem to. Maybe I'm missing a bugfix release?

(In reply to Anthony Lam (:antlam) from comment #14)
>  - font color looks a bit light, can we use our text grey for the options?

Used text_and_tabs_tray_grey.

> not including the placeholder of "Search or enter address" (that one can
> probably be the font color of the current options ("Custom" and "Use Firefox
> Home")

I used the default hint color (worth noting that the "custom" & "use Firefox Home" were the default color before too).

>  - Remove "Use" from "Use Firefox Home"

Done.

>  - span input field of "Custom" to be full length all the time (as it is if
> the field was full - as in the attachment)

Done.
Flags: needinfo?(michael.l.comella) → needinfo?(alam)
Attached image Screenshot post patch
With exception to bug 1220315, how does this look, Anthony?
Attachment #8694255 - Attachment is obsolete: true
Attachment #8694356 - Flags: feedback?(alam)
(In reply to Michael Comella (:mcomella) from comment #15)
> Anthony, what version of Android are you running? I hit bug 1220315 on my
> device, but you don't seem to. Maybe I'm missing a bugfix release?

I am running 6.0

> (In reply to Anthony Lam (:antlam) from comment #14)
> >  - font color looks a bit light, can we use our text grey for the options?
> 
> Used text_and_tabs_tray_grey.
> 
> > not including the placeholder of "Search or enter address" (that one can
> > probably be the font color of the current options ("Custom" and "Use Firefox
> > Home")
> 
> I used the default hint color (worth noting that the "custom" & "use Firefox
> Home" were the default color before too).

So to be clear, the current placeholder text of "Search or enter address" should be lighter, whereas the labels of "Firefox Home" and "Custom" should be darker.

I think that's it then! :) thanks Mike!
Flags: needinfo?(alam)
Comment on attachment 8694356 [details]
Screenshot post patch

+! 

Also spoke on IRC about setting the placeholder text to match our palette
Attachment #8694356 - Flags: feedback?(alam) → feedback+
(In reply to Anthony Lam (:antlam) from comment #17)

> So to be clear, the current placeholder text of "Search or enter address"

Can we actually search from that edit box?
Comment on attachment 8691608 [details]
MozReview Request: Bug 1227348 - Remove imports, provide better access levels. r=margaret

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26153/diff/1-2/
Comment on attachment 8691609 [details]
MozReview Request: Bug 1227348 - Add default homepage radio buttons to homepage preference. r=margaret

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26155/diff/1-2/
Attachment #8691609 - Attachment description: MozReview Request: Bug 1227348 - Add default homepage checkbox to homepage preference. r=margaret → MozReview Request: Bug 1227348 - Add default homepage radio buttons to homepage preference. r=margaret
Attachment #8691609 - Flags: review?(margaret.leibovic)
Attachment #8691610 - Attachment is obsolete: true
(In reply to Mark Finkle (:mfinkle) from comment #19)
> Can we actually search from that edit box?

You can't search in the sense that it will autocomplete addresses. If you type "apple computers", your homepage will be a search for "apple computers" via your default search engine. If you type a url, the url will open.

Anthony, this sounds confusing – any thoughts on new copy?
Flags: needinfo?(alam)
(In reply to Michael Comella (:mcomella) from comment #22)
> (In reply to Mark Finkle (:mfinkle) from comment #19)
> > Can we actually search from that edit box?
> 
> You can't search in the sense that it will autocomplete addresses. If you
> type "apple computers", your homepage will be a search for "apple computers"
> via your default search engine. If you type a url, the url will open.
> 
> Anthony, this sounds confusing – any thoughts on new copy?

How about "Enter address or search term"?
Flags: needinfo?(alam)
Comment on attachment 8691609 [details]
MozReview Request: Bug 1227348 - Add default homepage radio buttons to homepage preference. r=margaret

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26155/diff/2-3/
Comment on attachment 8691609 [details]
MozReview Request: Bug 1227348 - Add default homepage radio buttons to homepage preference. r=margaret

https://reviewboard.mozilla.org/r/26155/#review24217

Nice, I like the explict UX state here. r+ with comments addressed.

::: mobile/android/base/locales/en-US/android_strings.dtd:197
(Diff revision 3)
> +<!ENTITY home_homepage_radio_default "Firefox Home">

I think this should be "&brandShortName; Home"

::: mobile/android/base/preferences/SetHomepagePreference.java:65
(Diff revision 3)
> -            public void run() {
> +            public void onCheckedChanged(RadioGroup radioGroup, int i) {

Nit: I would name this variable `checkedId`. When seein `i`, I assume it's an index, which is confusing.

::: mobile/android/base/preferences/SetHomepagePreference.java:87
(Diff revision 3)
> -        if (!AboutPages.HOME.equals(url)) {
> +        return TextUtils.isEmpty(url) || DEFAULT_HOMEPAGE.equals(url);

What happens if the user enters "about:home" as their custom URL? I know this is an edge case, but we should probably avoid unexpected things happening.

I would say that maybe we should just let the user still have a "custom" state in this case, but I would want to make sure that doesn't affect startup performance (i.e. don't wait for Gecko to be up and running to realize that we just want to show the home pager).
Attachment #8691609 - Flags: review?(margaret.leibovic) → review+
(In reply to Michael Comella (:mcomella) from comment #22)
> (In reply to Mark Finkle (:mfinkle) from comment #19)
> > Can we actually search from that edit box?
> 
> You can't search in the sense that it will autocomplete addresses. If you
> type "apple computers", your homepage will be a search for "apple computers"
> via your default search engine. If you type a url, the url will open.
> 
> Anthony, this sounds confusing – any thoughts on new copy?

It would be neat though, could we ever get this to work? i.e. also pulling from https://bugzilla.mozilla.org/show_bug.cgi?id=858829
(In reply to Barbara Bermes [:barbara] from comment #26)
> > > Can we actually search from that edit box?
> > 
> > You can't search in the sense that it will autocomplete addresses. If you
> > type "apple computers", your homepage will be a search for "apple computers"
> > via your default search engine. If you type a url, the url will open.
> 
> It would be neat though, could we ever get this to work? i.e. also pulling
> from https://bugzilla.mozilla.org/show_bug.cgi?id=858829

afaik, it should be possible.
https://reviewboard.mozilla.org/r/26155/#review24217

> What happens if the user enters "about:home" as their custom URL? I know this is an edge case, but we should probably avoid unexpected things happening.
> 
> I would say that maybe we should just let the user still have a "custom" state in this case, but I would want to make sure that doesn't affect startup performance (i.e. don't wait for Gecko to be up and running to realize that we just want to show the home pager).

Nice find.

It appears we handle setting the preference as the empty String or `about:home` gracefully and there is no degredation of startup performance (at least on my GS4).

That being said, to be on the safe side and handle this more consistently, I added a line so that we'll handle the empty string or `about:home` as if the user pressed the default url radio button (i.e. we remove the preference value).
https://hg.mozilla.org/mozilla-central/rev/1b749564824e
https://hg.mozilla.org/mozilla-central/rev/92c54aa86e1c
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
(In reply to Michael Comella (:mcomella) from comment #27)
> (In reply to Barbara Bermes [:barbara] from comment #26)
> > > > Can we actually search from that edit box?
> > > 
> > > You can't search in the sense that it will autocomplete addresses. If you
> > > type "apple computers", your homepage will be a search for "apple computers"
> > > via your default search engine. If you type a url, the url will open.
> > 
> > It would be neat though, could we ever get this to work? i.e. also pulling
> > from https://bugzilla.mozilla.org/show_bug.cgi?id=858829
> 
> afaik, it should be possible.

Thanks Mike.

Mark, I would consider this a "delightful" addition.
Flags: needinfo?(mark.finkle)
(In reply to Barbara Bermes [:barbara] from comment #31)
> (In reply to Michael Comella (:mcomella) from comment #27)
> > (In reply to Barbara Bermes [:barbara] from comment #26)
> > > > > Can we actually search from that edit box?
> > > > 
> > > > You can't search in the sense that it will autocomplete addresses. If you
> > > > type "apple computers", your homepage will be a search for "apple computers"
> > > > via your default search engine. If you type a url, the url will open.
> > > 
> > > It would be neat though, could we ever get this to work? i.e. also pulling
> > > from https://bugzilla.mozilla.org/show_bug.cgi?id=858829
> > 
> > afaik, it should be possible.
> 
> Thanks Mike.
> 
> Mark, I would consider this a "delightful" addition.

I filed bug 1230218 to track this.
Moved NI to bug 1230218 comment 1.
Flags: needinfo?(mark.finkle)
Verified as fixed using:
Device: nexus 6 (Android 6.0)
Build: Firefox for Android 45.0a1 (2015-12-06)
You need to log in before you can comment on or make changes to this bug.