Add "Default Browser" check setting to preferences

VERIFIED FIXED in Firefox 34

Status

()

Firefox
Preferences
VERIFIED FIXED
4 years ago
3 years ago

People

(Reporter: sevaan, Assigned: mano)

Tracking

unspecified
Firefox 34
Points:
3
Bug Flags:
firefox-backlog +
qe-verify +

Firefox Tracking Flags

(firefox34 verified)

Details

Attachments

(6 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
Created attachment 8458808 [details]
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.
(Reporter)

Updated

4 years ago
Flags: firefox-backlog+
Hi Mano, can you provide a point value and mark the bug as either [qa+] or [qa-] for verification.
Assignee: nobody → mano
Status: NEW → ASSIGNED
Iteration: --- → 34.1
QA Whiteboard: [qa?]
Flags: needinfo?(mano)

Updated

4 years ago
Component: General → Preferences

Updated

4 years ago
Points: --- → 3
QA Whiteboard: [qa?] → [qa+]
QA Contact: camelia.badau

Updated

4 years ago
Flags: needinfo?(mano)
Created attachment 8466942 [details] [diff] [review]
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.
Attachment #8466942 - Flags: review?(jaws)
4) I should fix the indentation in main.xul.
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?
Attachment #8466942 - Flags: review?(jaws) → review-

Updated

4 years ago
Iteration: 34.1 → 34.2
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.
(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"?
Flags: needinfo?(sfranks)
(Reporter)

Comment 7

4 years ago
(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.
Flags: needinfo?(sfranks)
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).
(Reporter)

Comment 9

4 years ago
Mike, what's your thought on this?
Flags: needinfo?(mmaslaney)
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?
Flags: needinfo?(sfranks)
Flags: needinfo?(mmaslaney)
Flags: needinfo?(jaws)
Yeah, we can use bold or italics for this.
Flags: needinfo?(jaws)
(Reporter)

Comment 12

4 years ago
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?
Flags: needinfo?(sfranks) → needinfo?(Mnovak)
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.
Flags: needinfo?(Mnovak)
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
Attachment #8471438 - Flags: ui-review?(sfranks)
Attachment #8471438 - Flags: review?(jaws)
Attachment #8471438 - Flags: review+
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.
(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).
(Reporter)

Comment 18

4 years ago
: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!
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.
https://hg.mozilla.org/mozilla-central/rev/f7d6385d2d69
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
(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
(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.
Mano, can you land a patch that updates the string ID for alwaysCheckDefault?
Flags: needinfo?(mano)
(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.

Updated

4 years ago
Iteration: 34.2 → 34.3
QA Whiteboard: [qa+]
Flags: qe-verify+
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.
QA Whiteboard: [bugday-20140820]
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.
Status: RESOLVED → VERIFIED
status-firefox34: --- → verified
Will do tomorrow.
Flags: needinfo?(mano)

Updated

3 years ago
Depends on: 1144099
No longer depends on: 1144099
You need to log in before you can comment on or make changes to this bug.