"Open default browser settings" in onboarding does nothing

VERIFIED FIXED in Firefox 56

Status

()

Firefox
New Tab Page
P1
normal
VERIFIED FIXED
2 months ago
13 days ago

People

(Reporter: jonathanGB, Assigned: gasolin)

Tracking

(Depends on: 1 bug)

unspecified
Firefox 56
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: [photon-onboarding])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

Running latest nightly on macOS. 

Other buttons like "Show private browsing in menu" work, only this one does not.

http://imgur.com/a/kCG9a
NB: I have nightly as default already, but I feel like either we should:
1- not show the panel on "default" if it already is, OR
2- show some feedback saying "firefox is already your default browser" (e.g. some sort of feedback)

Updated

2 months ago
Whiteboard: [photon-onboarding] → [photon-onboarding][triage]
(Assignee)

Comment 2

2 months ago
Hi Michael, what we need to do if user have set Firefox as default browser before visiting the tour?


Maybe we can mark the tour as completed when we detected Firefox is already the default browser?
Flags: needinfo?(mverdi)

Comment 3

2 months ago
(In reply to Fred Lin [:gasolin] from comment #2)
> Hi Michael, what we need to do if user have set Firefox as default browser
> before visiting the tour?
> 
> 
> Maybe we can mark the tour as completed when we detected Firefox is already
> the default browser?

I thought marking it as complete if it was already the default browser was cut for 57. Doing that would be nice if still possible. Whether or not we can do that, the button should still work. If Firefox is your default and you click it, it will just confirm that this is in fact the case. That's better than a button that doesn't work for an unknown reason.
Flags: needinfo?(mverdi)
(Assignee)

Comment 4

2 months ago
(In reply to Verdi [:verdi] from comment #3)
> (In reply to Fred Lin [:gasolin] from comment #2)
> > Hi Michael, what we need to do if user have set Firefox as default browser
> > before visiting the tour?
> > (del)
> Whether or not we can do that, the button should still work. If Firefox is your default and
> you click it, it will just confirm that this is in fact the case. That's
> better than a button that doesn't work for an unknown reason.

Ok, then what's the expect behavior when user click the button when he/she already set Firefox as the default browser?
Flags: needinfo?(mverdi)

Comment 5

2 months ago
(In reply to Fred Lin [:gasolin] from comment #4)
> Ok, then what's the expect behavior when user click the button when he/she
> already set Firefox as the default browser?

The system dialog should open. In windows 10, for example, it will show that Firefox is set as the default browser already.
Flags: needinfo?(mverdi)
(In reply to Verdi [:verdi] from comment #5)
> (In reply to Fred Lin [:gasolin] from comment #4)
> > Ok, then what's the expect behavior when user click the button when he/she
> > already set Firefox as the default browser?
> 
> The system dialog should open. In windows 10, for example, it will show that
> Firefox is set as the default browser already.

That's not how any other OS works. Only Windows 10 asks the users to set default browser explictly in system settings. We still need to come up with something on other OSes.

Fred, could you make sure you get a implementable solution this week? Please also help Verdi understand the capablity of UITour; e.g. if we can dim the button or show a different message if we already know we are the default browser.
Flags: needinfo?(mverdi)
Flags: needinfo?(gasolin)
(Assignee)

Comment 7

2 months ago
Checked with verdi this afternoon and he suggest to 
1. disable the button and 
2. show a different message when we detected the user already set firefox as default. 

(According to the fact that we can detect the browser is set to default with UITour, but not easy to change the behavior with the system dialog, which the behavior detail is wrapped in platform API and may vary between OSes)


@verdi, could you help make the spec so we can start work on this?
Flags: needinfo?(gasolin)

Updated

2 months ago
Blocks: 1356488

Updated

2 months ago
Assignee: nobody → gasolin
Status: NEW → ASSIGNED
Flags: qe-verify+
Priority: -- → P1
QA Contact: jwilliams
Whiteboard: [photon-onboarding][triage] → [photon-onboarding]
Target Milestone: --- → Firefox 56
(Assignee)

Comment 8

2 months ago
There are 3 cases need to be addressed in UI spec:

[not set to default]
When user open the default browser tour, the user will see the clickable button


[already set to default] 
When user open the default browser tour, the user will see the alternative description 
(ex: Nightly is currently your default browser)


[not set to default but already clicked the tour button (so the tour is marked as complete)] 
When user open the default browser tour, the user will see ?
(Assignee)

Comment 9

2 months ago
Since we like to decouple with the tour complete state (marking it as complete if it was already the default browser),
I suggest a new UX flow to solve this issue:

1. always show the button when user open the default browser tour
2. when user click the button, open the default browser settings (when firefox is not the default browser), or hide the button and show the alternative description at the same place(when firefox is the default browser)

With that interaction, the user can click only once and will solve the related issue (what should we do when user click the button several times).


Verdi, how do you think?


(un-assign myself since the UX of this issue is not final yet)
Assignee: gasolin → nobody
Status: ASSIGNED → NEW
Priority: P1 → P2
(Assignee)

Comment 10

2 months ago
Created attachment 8884736 [details]
prototype

Hi Verdi, here's the prototype of above-mentioned proposal
Here's the video http://recordit.co/gRPNYUwRmg

1. When user click the button, it will disabled to prevent click again.
2. When user click the button and the browser already set to default, the button text will change to alternative string.

With this way we don't have to concern the tour complete state. Please let me know how you think.
Attachment #8884736 - Flags: ui-review?(mverdi)

Updated

2 months ago
status-firefox56: --- → affected

Comment 11

a month ago
(In reply to Fred Lin [:gasolin] from comment #10)
> Created attachment 8884736 [details]
> prototype
> 
> Hi Verdi, here's the prototype of above-mentioned proposal
> Here's the video http://recordit.co/gRPNYUwRmg
> 
> 1. When user click the button, it will disabled to prevent click again.
> 2. When user click the button and the browser already set to default, the
> button text will change to alternative string.
> 
> With this way we don't have to concern the tour complete state. Please let
> me know how you think.

I talked with Michelle and the copy we'd like to use is, "You’ve got this! Firefox is already your default browser." Localization can ignore the "You've got this!" part if there isn't an equivalent. 

I don't think the alternate text should look like a button. I'll have to mock something up.
Flags: needinfo?(mverdi)
Fred, is above actionable for you?
Flags: needinfo?(gasolin)
(Assignee)

Comment 13

a month ago
Thanks Verdi,

If its not a button, what kind of element should I use? I could make a patch first then fix the style once you provided.
Flags: needinfo?(gasolin) → needinfo?(mverdi)
(Assignee)

Comment 14

a month ago
And I'd like to make sure the prototype UI flow works for you before make a patch.
Firefox 56 scope, need to get it done soon.
Assignee: nobody → gasolin
Status: NEW → ASSIGNED
Priority: P2 → P1

Comment 16

a month ago
Created attachment 8885557 [details]
5a-default-browser-overlay-set.png

Can we replace the button with two lines of body style, centered text.
Flags: needinfo?(mverdi)
(Assignee)

Comment 17

a month ago
Yes, will do. Can you help do the the new string copy check to locale & legal team, so we can replace it at the time I land the patch.
Flags: needinfo?(mverdi)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 20

a month ago
In this patch:

* the call-to-action button will disabled when user click the button, to prevent open settings panel several times
* the call-to-action button will be replaced to alternative text when Firefox is already the default browser
Comment hidden (mozreview-request)

Comment 22

a month ago
mozreview-review
Comment on attachment 8886060 [details]
Bug 1374717 - show alternative message when firefox already the default browser;

https://reviewboard.mozilla.org/r/156856/#review161966

Overall it looks good to me but I'm wondering the l10n part, would you add l10n member to look at it?

::: browser/extensions/onboarding/content/onboarding-tour-agent.js:24
(Diff revision 2)
> +      Mozilla.UITour.getConfiguration("appinfo", (config) => {
> +        let isDefaultBrowser = config.defaultBrowser;
> +        let btn = document.getElementById("onboarding-tour-default-browser-button");
> +        let msg = document.getElementById("onboarding-tour-is-default-browser-msg");
> +        if (isDefaultBrowser) {
> +          if (!btn.classList.contains("onboarding-hidden")) {

Maybe we can just call add/remove without if().

::: browser/extensions/onboarding/content/onboarding-tour-agent.js:27
(Diff revision 2)
> +        let msg = document.getElementById("onboarding-tour-is-default-browser-msg");
> +        if (isDefaultBrowser) {
> +          if (!btn.classList.contains("onboarding-hidden")) {
> +            btn.classList.add("onboarding-hidden");
> +          }
> +          if (msg.classList.contains("onboarding-hidden")) {

same above

::: browser/extensions/onboarding/content/onboarding-tour-agent.js:31
(Diff revision 2)
> +          }
> +          if (msg.classList.contains("onboarding-hidden")) {
> +            msg.classList.remove("onboarding-hidden");
> +          }
> +        } else {
> +          btn.disabled = true;

User may cancel the system dialog in case of mac, so maybe we don't have to disable it?

::: browser/extensions/onboarding/content/onboarding.js:180
(Diff revision 2)
>          <section class="onboarding-tour-content">
>            <img src="resource://onboarding/img/figure_default.svg" />
>          </section>
>          <aside class="onboarding-tour-button-container">
>            <button id="onboarding-tour-default-browser-button" class="onboarding-tour-action-button" data-l10n-id="${defaultBrowserButtonId}"></button>
> +          <div id="onboarding-tour-is-default-browser-msg" class="onboarding-hidden">${isDefaultMessage}<br/>${isDefault2ndMessage}</div>

Not sure if breaking this into two lines is okay for l10n, would you add l10n team member for review?
Attachment #8886060 - Flags: review?(rexboy)
(Assignee)

Comment 23

a month ago
Comment on attachment 8886060 [details]
Bug 1374717 - show alternative message when firefox already the default browser;

flod, could you take a look on l10n part?
Attachment #8886060 - Flags: feedback?(francesco.lodolo)
(Assignee)

Comment 24

a month ago
> ::: browser/extensions/onboarding/content/onboarding-tour-agent.js:31
> (Diff revision 2)
> > +          }
> > +          if (msg.classList.contains("onboarding-hidden")) {
> > +            msg.classList.remove("onboarding-hidden");
> > +          }
> > +        } else {
> > +          btn.disabled = true;
> 
> User may cancel the system dialog in case of mac, so maybe we don't have to
> disable it?
> 

I think to disable the button after user click still make sense.
If user want to trigger system panel again, user can open a new tab and the button will be recovered to the default state.

Comment 25

a month ago
mozreview-review
Comment on attachment 8886060 [details]
Bug 1374717 - show alternative message when firefox already the default browser;

https://reviewboard.mozilla.org/r/156856/#review162060

::: browser/extensions/onboarding/locales/en-US/onboarding.properties:59
(Diff revision 3)
>  onboarding.tour-default-browser.description2=Love %1$S? Set it as your default browser. Open a link from another application, and %1$S will be there for you.
>  # LOCALIZATION NOTE(onboarding.tour-default-browser.button): Label for a button to open the OS default browser settings where it's not possible to set the default browser directly. (OSX, Linux, Windows 8 and higher)
>  onboarding.tour-default-browser.button=Open Default Browser Settings
>  # LOCALIZATION NOTE(onboarding.tour-default-browser.win7.button): Label for a button to directly set the default browser (Windows 7). %S is brandShortName
>  onboarding.tour-default-browser.win7.button=Make %S Your Default Browser
> +# LOCALIZATION NOTE(onboarding.tour-default-browser.is-default.2nd-message): Label to admire user and show with onboarding.tour-default-browser.is-default.2nd-message.

Can you explain what you mean here? "admire" is not the verb you want.

I guess something along the line of "Label displayed when Firefox is already set as default browser, followed on a new line by "onboarding.tour-default-browser.is-default.2nd-message".

::: browser/extensions/onboarding/locales/en-US/onboarding.properties:61
(Diff revision 3)
>  onboarding.tour-default-browser.button=Open Default Browser Settings
>  # LOCALIZATION NOTE(onboarding.tour-default-browser.win7.button): Label for a button to directly set the default browser (Windows 7). %S is brandShortName
>  onboarding.tour-default-browser.win7.button=Make %S Your Default Browser
> +# LOCALIZATION NOTE(onboarding.tour-default-browser.is-default.2nd-message): Label to admire user and show with onboarding.tour-default-browser.is-default.2nd-message.
> +onboarding.tour-default-browser.is-default.message=You’ve got this!
> +# LOCALIZATION NOTE(onboarding.tour-default-browser.is-default.2nd-message): Label to show the browser is already set as the default browser. %S is brandShortName

Label displayed when Firefox is already set as default browser. %S is brandShortName

Comment 26

a month ago
(In reply to Fred Lin [:gasolin] from comment #17)
> Yes, will do. Can you help do the the new string copy check to locale &
> legal team, so we can replace it at the time I land the patch.

This copy is approved.
Flags: needinfo?(mverdi)
Comment hidden (mozreview-request)

Comment 28

a month ago
mozreview-review
Comment on attachment 8886060 [details]
Bug 1374717 - show alternative message when firefox already the default browser;

https://reviewboard.mozilla.org/r/156856/#review162410
Attachment #8886060 - Flags: review+

Updated

a month ago
Attachment #8886060 - Flags: review?(rexboy) → review+

Comment 29

a month ago
mozreview-review
Comment on attachment 8886060 [details]
Bug 1374717 - show alternative message when firefox already the default browser;

https://reviewboard.mozilla.org/r/156856/#review162562
Attachment #8886060 - Flags: review?(dtownsend) → review+
(Assignee)

Comment 30

a month ago
thanks!
Keywords: checkin-needed

Comment 31

a month ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f4aacb756bac
show alternative message when firefox already the default browser;r=flod,mossop
Keywords: checkin-needed

Comment 32

a month ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f4aacb756bac
Status: ASSIGNED → RESOLVED
Last Resolved: a month ago
status-firefox56: affected → fixed
Resolution: --- → FIXED
I have verified this is fixed on todays nightly across all platforms.
Status: RESOLVED → VERIFIED

Comment 34

a month ago
Comment on attachment 8884736 [details]
prototype

Clearing review request
Attachment #8884736 - Flags: ui-review?(mverdi) → ui-review+
On Ubuntu no settings panel is shown. The default browser is automatically set to Nightly. Is this expected? 

If so, maybe change "Open default browser settings" to "Set Nightly as your default browser" on Ubuntu.

I can write a bug for this if you all agree.
Flags: needinfo?(mverdi)
Flags: needinfo?(gasolin)

Comment 36

29 days ago
Ah yes, if this is the case with Ubuntu, it should use the same copy as Windows 7, "Make Firefox Your Default Browser".
Flags: needinfo?(mverdi)
(Assignee)

Comment 37

28 days ago
Sorry Justin I've reply this but not send:/

In short we should check other linux distribution as well (fedora, ubuntu with KDE) to make sure its ubuntu only or it applied to all linux distributions. Currently we don't have a way to identify ubuntu from other linux distribution.
Flags: needinfo?(gasolin)
In case people didn't know, this is what we have the `canSetDefaultBrowserInBackground` property for on the getConfiguration("appInfo") response. Please update and use that rather than doing custom OS checks.
(Assignee)

Updated

27 days ago
Blocks: 1385170
Depends on: 1389558
You need to log in before you can comment on or make changes to this bug.