1040868 - Add "Default Browser" check setting to preferences

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect)

Description

[Sevaan Franks [:sevaan]]

Attachment: Mockup of Preferences

The preference to set Firefox as your default browser should be front and center when loading up the preferences. Currently it is buried under advanced options.

Add to the "Startup" section of preferences a checkbox with the following strings:

- Always check to see if Firefox is my default browser on startup.
- Firefox is currently your default browser.
- Firefox is not your default browser. Make it my default browser.

Comment 1

[Marco Mucci [:MarcoM]]

Hi Mano, can you provide a point value and mark the bug as either [qa+] or [qa-] for verification.

Comment 2

[Asaf Romano (gone)]

Attachment: patch

1) The dtd changes are going to break legacy preferences ui. Is that a problem?
2) I rev'ed the accesskeys to ensure localizers check for collisions.
3) "color: red" is probably native. I assume UX have some "custom" color in mind.

Comment 3

[Asaf Romano (gone)]

4) I should fix the indentation in main.xul.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8466942 [details] [diff] [review]
patch

Review of attachment 8466942 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/locales/en-US/chrome/browser/preferences/advanced.dtd
@@ -30,5 @@
> -<!ENTITY alwaysCheckDefault.label        "Always check to see if &brandShortName; is the default browser on startup"><!--XXX-->
> -<!ENTITY alwaysCheckDefault.accesskey    "w">
> -<!ENTITY setDefault.label                "Make &brandShortName; the default browser">
> -<!ENTITY setDefault.accesskey            "d">
> -<!ENTITY isDefault.label                 "&brandShortName; is currently your default browser">

We should duplicate these labels for main.dtd since we will still want to support disabling in-content prefs during at least 1 release cycle in case some unexpected major issue is found.

::: browser/themes/shared/incontentprefs/preferences.css
@@ +600,5 @@
>  
>  /* General Pane */
>  
> +#isNotDefaultLabel {
> +  color: red;

This will break for high-contrast white-on-black themes. Can we use font-weight:bold for this instead?

Comment 5

[Asaf Romano (gone)]

Couple of things:
1) I think that instead of copying the entities over, I'll fix the "legacy" UI too. Given that the state on in-content-preferences is not clear, that's probably a good idea.
2) fwiw, "color: red" was "the spec". I don't like it either.

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Mano (needinfo? for any questions; not reading general bugmail) from comment #5)
> Couple of things:
> 1) I think that instead of copying the entities over, I'll fix the "legacy"
> UI too. Given that the state on in-content-preferences is not clear, that's
> probably a good idea.

Yeah, that sounds good.

> 2) fwiw, "color: red" was "the spec". I don't like it either.

We shouldn't be blind to the spec. Perhaps the author of the spec didn't take High Contrast into consideration. Sevaan, are you OK with us using bold-weighted font instead of a red color to show "Firefox is not your default browser"?

Comment 7

[Sevaan Franks [:sevaan]]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #6)
> (In reply to Mano (needinfo? for any questions; not reading general bugmail)
> from comment #5)
> > Couple of things:
> > 1) I think that instead of copying the entities over, I'll fix the "legacy"
> > UI too. Given that the state on in-content-preferences is not clear, that's
> > probably a good idea.
> 
> Yeah, that sounds good.
> 
> > 2) fwiw, "color: red" was "the spec". I don't like it either.
> 
> We shouldn't be blind to the spec. Perhaps the author of the spec didn't
> take High Contrast into consideration. Sevaan, are you OK with us using
> bold-weighted font instead of a red color to show "Firefox is not your
> default browser"?

Sure, I'm good with that, and no I did not take that into account. Thanks for bringing it to my attention; I'll be more conscious going again.

Is it possible to do both red and bold? The idea is that we really want to draw the users eye to it, hopefully causing them to reconsider their default browser choice.

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

We don't have a system color value that is meant for drawing attention (in this case, red-by-default and will change with the system theme). The closest we have is -moz-activehyperlink (which is red) but that can be changed by the user and is not associated with drawing attention so it might not work in a few cases and is a misuse of it's intended purpose (hence the reason for it not working in a few cases).

Comment 9

[Sevaan Franks [:sevaan]]

Mike, what's your thought on this?

Comment 10

[Michael Maslaney [:mmaslaney] ([e-mail])]

Sevaan, instead of using color to grab attention, how do you feel about using a different font weight like bold or italic? Maybe the copy could be a bit more emotional as well, like "we are sad to report that Firefox is not your default browser" or "OMG, did you know that Firefox is not your default browser!", which could also grab attention.

Jared, can we use Bold or Italic weights for this case?

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

Yeah, we can use bold or italics for this.

Comment 12

[Sevaan Franks [:sevaan]]

I'm good with bold.

Looping Matej in:

Hey Matej, please see a mockup of the preferences here: https://bugzilla.mozilla.org/attachment.cgi?id=8458808

The text in red is the string I came up with to alert users as to whether Firefox is set as their default browser. Instead of going with Red, we are just going to use a bold font. Mike Maslaney suggested a couple other options in Comment 10 that might be more attention getting. What are your thoughts?

Comment 13

[Matej Novak [:matej]]

As much as I'm all for emotion and personality in copy, I feel like this is a place where we should be clear and straightforward, so I think what's in the mockup works well (it matches the rest of the copy in Preferences, too).

What's a bit confusing to me is that we're using both "my" and "your." Does that bother anyone else?

One last thing: "My" on the button should be capitalized as well.

Thanks.

Comment 14

[Asaf Romano (gone)]

Attachment: patch

Comment 15

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8471438 [details] [diff] [review]
patch

Review of attachment 8471438 [details] [diff] [review]:
-----------------------------------------------------------------

Interestingly enough, this bug is basically a revert of bug 406387.

As an alternative, would it make more sense to place it in the Applications pane since that pane deals with default actions of applications? (flagging sevaan for ui-review about this question) This would match the placement of Internet Explorer, http://screencast.com/t/BnrAWH70

::: browser/locales/en-US/chrome/browser/preferences/main.dtd
@@ +30,5 @@
>  <!ENTITY chooseFolderMac.accesskey    "e">
>  <!ENTITY alwaysAsk.label "Always ask me where to save files">
>  <!ENTITY alwaysAsk.accesskey "A">
> +
> +<!ENTITY alwaysCheckDefault.label         "Always check to see if &brandShortName; is the default browser on startup"><!--XXX-->

nit, this XXX can get removed

Comment 16

[Asaf Romano (gone)]

I think the essence of this bug was to make this piece of UI as visible as possible. That is, it would go in Applications if Application was the default tab.

Comment 17

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Mano (needinfo? for any questions; not reading general bugmail) from comment #16)
> I think the essence of this bug was to make this piece of UI as visible as
> possible. That is, it would go in Applications if Application was the
> default tab.

Yeah, I get that :) and my question probably doesn't help because it goes against that initial goal. But another question is about placing it at the bottom of the default page, which would closely match what Chrome is doing. I'm not sure if it should be the 1st and highest priority preference when opening Preferences (it shouldn't get changed often, and doesn't need to get referenced often).

Comment 18

[Sevaan Franks [:sevaan]]

:jaws, how do you feel about it not being the very first option in the Startup section, but rather the third, underneath the homepage stuff, so kind of in the center of the page?

We definitely want it on the first page in the preferences, as :mano mentioned, and I am cool with moving it around on that page, including putting it at the bottom underneath a "Default Browser" section. However, might be nice to do it a little different than Chome, and since the preference is about checking for default status on Startup, that first section makes sense.

Also, do you mind uploading a screenshot of that patch you wanted a review on? I'm sorry, but I'm not set up to build Firefox yet. I need to get on that!

Comment 19

[Asaf Romano (gone)]

I'm landing it as is, with the review comment addressed. We can tweak it however we want later on. I'll attach screenshot in a bit.

Comment 20

[Asaf Romano (gone)]

https://hg.mozilla.org/integration/fx-team/rev/f7d6385d2d69

Comment 21

[Asaf Romano (gone)]

Attachment: In-content, is-default

Comment 22

[Asaf Romano (gone)]

Attachment: in-content, not-default

Comment 23

[Asaf Romano (gone)]

Attachment: in-window, is-default

Comment 24

[Asaf Romano (gone)]

Attachment: in-window, not-default

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f7d6385d2d69

Comment 26

[Francesco Lodolo [:flod]]

(In reply to Mano (needinfo? for any questions; not reading general bugmail) from comment #2)
> 2) I rev'ed the accesskeys to ensure localizers check for collisions.

Next time better to change the string ID too. Right now we have

alwaysCheckDefault.label -> alwaysCheckDefault2.accesskey 
setAsMyDefaultBrowser.label -> setAsMyDefaultBrowser.accesskey

The first couple is now disconnected, so tools trying to put them together will fail (also not sure why only one was changed).

See also https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_best_practices#Updating_Entity_Names

Comment 27

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Francesco Lodolo [:flod] from comment #26)
> alwaysCheckDefault.label -> alwaysCheckDefault2.accesskey 
> setAsMyDefaultBrowser.label -> setAsMyDefaultBrowser.accesskey
> 
> (also not sure why only one was changed).

alwaysCheckDefault.label was changed because the context for the accesskey has changed. Thank you for the feedback about keeping the string IDs in sync.

setAsMyDefaultBrowser string ID was previously setDefault, so this did change. I'm not sure if this was what you were asking about when you said "also not sure why only one was changed", but I hope this answers your question.

Comment 28

[Jared Wein [:jaws] (please needinfo? me)]

Mano, can you land a patch that updates the string ID for alwaysCheckDefault?

Comment 29

[Francesco Lodolo [:flod]]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #27)
> setAsMyDefaultBrowser string ID was previously setDefault, so this did
> change. I'm not sure if this was what you were asking about when you said
> "also not sure why only one was changed", but I hope this answers your
> question.

You're right, I realized that the key ID changed only after commenting and forgot to update my question.

Comment 30

[Ricardo Maçãs [:Ricmacas]]

Tested old version Mozilla/5.0 (X11; Linux x86_64; rv:33.0) Gecko/20100101 Firefox/33.0, default browser does not exist.
Tested nightly Mozilla/5.0 (X11; Linux x86_64; rv:34.0) Gecko/20100101 Firefox/34.0, default browser is in (and indicates if the current browser is the default browser successfully afaict).
Awaiting confirmation in other platforms.

Comment 31

[Camelia Badau [:cbadau], Desktop Test Engineering]

Verified on Windows 7 64bit, Ubuntu 13.10 32bit and Mac OSX 10.8.5 using latest Nightly 34.0a1 (buildID:	20140820030202) and the "Default Browser" setting correctly appears in preferences, in "Startup" section.

Comment 32

[Asaf Romano (gone)]

Will do tomorrow.