Should we disable the default browser check in local builds?

[Brian Grinstead]

There’s a modal prompt that opens on startup after every clobber asking me if I want to set the local build as my default browser (I don’t). Are there any objections to defaulting browser.shell.checkDefaultBrowser to false in local builds? We already do this for tests.

Brian
_______________________________________________
firefox-dev mailing list
firef...@mozilla.org
https://mail.mozilla.org/listinfo/firefox-dev

[Ryan VanderMeulen]

This is using |mach run| I assume? I would think we could certainly make that the default under those circumstances, yes, but I'd hesitate to do so unconditionally for local builds otherwise.

-Ryan

[Matt Howell]

On Windows, you can't actually set a local build as the default browser anyway, unless you've used the locally-built installer, because there are some entries that need to be present in the Windows registry for that to work and only the installer writes them. You can manually give local builds file/protocol associations, but that's all separate from what this prompt is about.

[Brian Grinstead]

Yes, when using |mach run|. However, the only directive I’m aware of that we have for this is !MOZ_OFFICIAL, which would apply regardless of mach. This is something we ran into with Bug 1375280 as well - we don’t currently have a mechanism to set prefs exclusively when running via |mach run|.

Brian

[Kris Maglione]

On Wed, Jul 26, 2017 at 11:03:39AM -0700, Brian Grinstead wrote:
>There’s a modal prompt that opens on startup after every
>clobber asking me if I want to set the local build as my
>default browser (I don’t). Are there any objections to
>defaulting browser.shell.checkDefaultBrowser to false in local
>builds? We already do this for tests.

Please yes. This is one of my biggest development pet peeves...

[Kris Maglione]

On Wed, Jul 26, 2017 at 11:19:06AM -0700, Brian Grinstead wrote:
>Yes, when using |mach run|. However, the only directive I’m
>aware of that we have for this is !MOZ_OFFICIAL, which would
>apply regardless of mach. This is something we ran into with
>Bug 1375280 as well - we don’t currently have a mechanism to
>set prefs exclusively when running via |mach run|.

It would really be nice to have `mach run` accept the --setpref
flag, like other mach commands. But an environment variable
should be sufficient.

>> On Jul 26, 2017, at 11:06 AM, Ryan VanderMeulen <rvande...@mozilla.com> wrote:
>>
>> This is using |mach run| I assume? I would think we could certainly make that the default under those circumstances, yes, but I'd hesitate to do so unconditionally for local builds otherwise.
>>
>> -Ryan
>>
>> On Wed, Jul 26, 2017 at 2:03 PM, Brian Grinstead <bgrin...@mozilla.com> wrote:
>> There’s a modal prompt that opens on startup after every clobber asking me if I want to set the local build as my default browser (I don’t). Are there any objections to defaulting browser.shell.checkDefaultBrowser to false in local builds? We already do this for tests.
>>
>> Brian
>> _______________________________________________
>> firefox-dev mailing list
>> firef...@mozilla.org
>> https://mail.mozilla.org/listinfo/firefox-dev
>>
>
>_______________________________________________
>firefox-dev mailing list
>firef...@mozilla.org
>https://mail.mozilla.org/listinfo/firefox-dev

--
Kris Maglione
Senior Firefox Add-ons Engineer
Mozilla Corporation

Simple things should be simple. Complex things should be possible.
--Alan Kay

[Brian Grinstead]

Thanks Matt,
Seems like another reason to disable it since it won't do the right thing in this case (yet it does pop up in a local Windows build)

Brian

[Brian Grinstead]

> On Jul 26, 2017, at 11:35 AM, Kris Maglione <kmag...@mozilla.com> wrote:
>
> On Wed, Jul 26, 2017 at 11:19:06AM -0700, Brian Grinstead wrote:
>> Yes, when using |mach run|. However, the only directive I’m aware of that we have for this is !MOZ_OFFICIAL, which would apply regardless of mach. This is something we ran into with Bug 1375280 as well - we don’t currently have a mechanism to set prefs exclusively when running via |mach run|.
>
> It would really be nice to have `mach run` accept the --setpref flag, like other mach commands. But an environment variable should be sufficient.

I was hoping to not rely on an environment variable for cases like these since I assumed it would imply doing either:
1) Change all places that access the pref to also look for an environment variable and bypass the pref value.
2) Run some code during startup based on the environment variable that changes the default values of relevant prefs (which adds yet another place where default prefs are defined).

I agree --setpref would be nice for |mach run| - I guess we'd pass it along as a new command line argument to Firefox (as opposed to in tests where a new profile is created). But an extra note is that in this case we will want to use a sticky_pref to prevent a scenario where you open a profile in Nightly and set browser.shell.checkDefaultBrowser to false, then open the same profile in a local build (which resets it into a default value), then open the same profile again in Nightly (where it is now true again). So I’m not sure if we’d want one command line option for normal prefs and a different one for sticky prefs, or what.

Brian

[Ehsan Akhgari]

On Wed, Jul 26, 2017 at 2:34 PM, Kris Maglione <kmag...@mozilla.com> wrote:

On Wed, Jul 26, 2017 at 11:03:39AM -0700, Brian Grinstead wrote:

There’s a modal prompt that opens on startup after every clobber asking me if I want to set the local build as my default browser (I don’t). Are there any objections to defaulting browser.shell.checkDefaultBrowser to false in local builds? We already do this for tests.

Please yes. This is one of my biggest development pet peeves...

Unpopular opinion: I don't think this is a good idea.  The code involved in checking whether we are the default browser has had performance issues (see https://bugzilla.mozilla.org/show_bug.cgi?id=1357146) and is in my humble opinion not so nice behavior which we've wanted to fix (https://bugzilla.mozilla.org/show_bug.cgi?id=1143116) for all users.  Hiding this from our developers seems like a disservice to our users, especially to the new users who we are hoping to attract to Firefox as we are working on improving the browser.

I understand the desire to do the easy thing and put a band-aid on where we are bleeding for now, but perhaps we should at least consider this as a reminder to check back with the right folks working on the original effort to fix this properly for all users?

--

Ehsan

[Richard Newman]

Brian: would it meet your needs to simply set this pref to false in user.js in your testing profile?

(And one way to do that is to have mach run write that to user.js for you… after all, it knows which profile you're using.)

[Kris Maglione]

On Wed, Jul 26, 2017 at 03:35:18PM -0400, Ehsan Akhgari wrote:
>Unpopular opinion: I don't think this is a good idea. The code involved in
>checking whether we are the default browser has had performance issues (see
>https://bugzilla.mozilla.org/show_bug.cgi?id=1357146) and is in my humble
>opinion not so nice behavior which we've wanted to fix (
>https://bugzilla.mozilla.org/show_bug.cgi?id=1143116) for all users.
>Hiding this from our developers seems like a disservice to our users,
>especially to the new users who we are hoping to attract to Firefox as we
>are working on improving the browser.

Fair enough, but my issue is that I see and have to deal with
this dialog during development much more often than ordinary
users do in their daily usage. Sometimes many times in a single
day, often in debug builds, where dismissing it takes much
longer than it normally would.

Fixing it for users would be nice, but developer workflows are
and often should be different, and I think we should consider
changing default behaviors for development runs when they make
development significantly easier. And in the particular case of
the default browser check... even if we improve the user
experience, it doesn't really ever make sense from `mach run` on
a development build, unless we explicitly need it in order to
test something.

[Kris Maglione]

The issue is that in development builds, we often wind up
repeatedly creating new profiles, particularly after clobber.
Having `mach run` create a user.js file might be a good option,
but there are still plenty of times when I need to change some
particular preference for one or several runs (like I do for
talos or mochitest runs on a regular basis) and being able to
pass an ephemeral value via --setpref would be much more useful.

[Robert Strong]

For these types of one-offs I have patches in the front of my queue. For this I'd just add a pref change so the default is false.

[Dave Townsend]

On Wed, Jul 26, 2017 at 12:48 PM Richard Newman <rne...@mozilla.com> wrote:

Brian: would it meet your needs to simply set this pref to false in user.js in your testing profile?

(And one way to do that is to have mach run write that to user.js for you… after all, it knows which profile you're using.)

https://bugzilla.mozilla.org/show_bug.cgi?id=1172574 is something I've wanted for some time that would be a solution here. Perhaps we can get someone working on it?

[Andrew Halberstadt]

I put up a proof of concept on bug 1172574 that allows you to:

1) Clone profiles

2) Set prefs via command line

3) Set prefs via config file

It still needs testing and a bit of work, but it seems to mostly get the job done. Not sure how much time I have to push on it right now, but if anyone wants to run with it, please feel free.

[Brian Grinstead]

Thank you for doing that, having setpref would really help! I’ll test it out some more today and leave comments in the bug.

Brian

[Brian Grinstead]

> On Jul 26, 2017, at 12:35 PM, Ehsan Akhgari <ehsan....@gmail.com> wrote:
> Unpopular opinion: I don't think this is a good idea. The code involved in checking whether we are the default browser has had performance issues (see https://bugzilla.mozilla.org/show_bug.cgi?id=1357146) and is in my humble opinion not so nice behavior which we've wanted to fix (https://bugzilla.mozilla.org/show_bug.cgi?id=1143116) for all users. Hiding this from our developers seems like a disservice to our users, especially to the new users who we are hoping to attract to Firefox as we are working on improving the browser.

I agree about this for most features. But with the default browser prompt, I don't think improvements for our users align with improvements for our developers. The change that would make the prompt better for me is if it wasn't modal, because then I could ignore it. I don't have any reason to believe that would be an improvement for users, though.

> I understand the desire to do the easy thing and put a band-aid on where we are bleeding for now, but perhaps we should at least consider this as a reminder to check back with the right folks working on the original effort to fix this properly for all users?

Maybe I’m misreading the bug, but we already don’t show the prompt on the first run (via the browser.shell.skipDefaultBrowserCheckOnFirstRun pref). I see the prompt on subsequent runs: so `./mach run --profile /tmp/foo` (no prompt), close browser, `./mach run --profile /tmp/foo` (prompt).

Brian

[Gregory Szorc]

We have machrc config files. IMO mach should do the most reasonable thing for the majority of developers by default. Minority and edge use cases can be dealt with through config files escape hatches. If we need to establish "profiles" to encapsulate sets of common configs to appease large factions, so be it.

I suspect this startup prompt falls into "annoying for majority" and/or "relevant to minority" so I think disabling by default makes sense.

Also, IIRC there is no way to set single prefs from a Firefox CLI argument. 'mach run' doesn't yet involve itself heavily in profile management: it just creates an empty directory for a temp profile. It feels unfortunate we have to muck around with profiles to accomplish this simple task. But if it is just writing a user.js file in an empty to-be-profile directory, that does seem too bad. Still, it would be nice to have something more formal on the CLI, as our prevailing convention of environment variables lends itself to poor documentation, poor discovery, makes us susceptible to conflicts with other programs' use of environment variables, and may even open us up to security issues like the HTTPoxy class of vulnerabilities.

[Ehsan Akhgari]

On Thu, Jul 27, 2017 at 10:36 AM, Brian Grinstead <bgrin...@mozilla.com> wrote:

> On Jul 26, 2017, at 12:35 PM, Ehsan Akhgari <ehsan....@gmail.com> wrote:
> Unpopular opinion: I don't think this is a good idea.  The code involved in checking whether we are the default browser has had performance issues (see https://bugzilla.mozilla.org/show_bug.cgi?id=1357146) and is in my humble opinion not so nice behavior which we've wanted to fix (https://bugzilla.mozilla.org/show_bug.cgi?id=1143116) for all users.  Hiding this from our developers seems like a disservice to our users, especially to the new users who we are hoping to attract to Firefox as we are working on improving the browser.

I agree about this for most features. But with the default browser prompt, I don't think improvements for our users align with improvements for our developers. The change that would make the prompt better for me is if it wasn't modal, because then I could ignore it. I don't have any reason to believe that would be an improvement for users, though.

I think the exact same reason would make making the dialog non-modal an improvement for us holds for our users as well.  Just like us, they also like to use their browser to browse the web, and a modal dialog prevents them from doing that, and a modeless prompt can be ignored by them and is therefore an improvement.

 

> I understand the desire to do the easy thing and put a band-aid on where we are bleeding for now, but perhaps we should at least consider this as a reminder to check back with the right folks working on the original effort to fix this properly for all users?

Maybe I’m misreading the bug, but we already don’t show the prompt on the first run (via the browser.shell.skipDefaultBrowserCheckOnFirstRun pref). I see the prompt on subsequent runs: so `./mach run --profile /tmp/foo` (no prompt), close browser, `./mach run --profile /tmp/foo` (prompt).

No, I think your understanding of the bug is correct.  We have since changed things to show the prompt on the second run I think, but the modality of it is still as hostile to the user as before.

Also, I prefixed my previous comment with "unpopular opinion" for a reason!  I wasn't really expecting anyone to resist itching this itch, because with my developer hat on I don't like to deal with this any more than anyone else here.  But I thought it was worth mentioning nonetheless, even if an objection doomed to be ignored.  :-)

Cheers,

--

Ehsan

[Brian Grinstead]

OK, it seems like there are a few paths forward for changing default pref values:

1) A combination of !MOZ_OFFICIAL and a sticky_pref with a different default. This means local builds will get the default without mach, and that you can share a dev profile bewteen Nightly and a local build while getting different default behavior in each. This is what we are doing for the Browser Toolbox prefs.
2) Do something like Bug 1172574, where we could specify a default set of prefs for scratch_user profiles created by mach. It would only affect `mach run`. It doesn't work when sharing a profile between Nightly and a local build.
3) Add a new CLI argument for Firefox that lets you set prefs. This does allow for sharing a dev profile between Nightly and a local build, but with what may be undesirable behavior (the pref would remain set to the 'local build' value even when opening in Nightly).

For the small set of prefs like this (and the about:config warning) that we'd consider flipping for all local builds, I still think (1) is the best option since it still allows for the 'sharing a profile between local and Nightly' workflow. But if we wanted to focus just on the 'mach run with the scratch_user’ experience (2) could work just as well. Also, (2) seems really useful generally since it'd let people change whatever prefs suit them (either via mach run CLI argument or a mach config).

Brian

[Brian Grinstead]

FYI we landed some changes to `./mach run` as a result of this discussion (in bug 1172574). These changes only affect the scratch_user, the profile created if -P or -profile aren’t passed.

1) The default browser check is disabled by default
2) The about:config warning is disabled by default
3) You can set arbitrary prefs through the CLI using --setpref or through your mach configuration file [0] at ~/.mozbuild/machrc. You can see more details by doing `./mach help run`

Here's an example command using setpref:

./mach run --setpref browser.startup.homepage="about:config" --setpref devtools.theme=dark

And here’s an example ~/.mozbuild/machrc file that would revert (1) and (2) for all calls to `./mach run`

[runprefs]
browser.shell.checkDefaultBrowser=true
general.warnOnAboutConfig=true

It'd be possible to expand these changes to work with -P and -profile, but we’d have to decide on the expected side effects from doing so (i.e. should the preference changes stick or be reset after the browser closes?).

Brian

[Andrew Halberstadt]

Thanks for pushing this through Brian!

In terms of the config file, you can also point $MACHRC at it, or pass it in via the global --settings mach argument.

[Enrico Weigelt, metux IT consult]

On 31.07.2017 19:31, Andrew Halberstadt wrote:

<snip>

I, personally, would like to drop that completely.

This is one of the thing that always annoyed me: $app asking to be
default for $xyz, and then having some platform and DE specific code
to somehow make this work ...

And such behaviour really reminds me to second-hand car dealers.

Anyways, this all depends on that there really is some concept of
"default application" on the actual platform (my systems usually
dont have that at all), and the individual applications have to
implement lots of special platform specific stuff (eg. haven't seen
anything in moz that touches mc.ext file, ...).

Oh, wait, there's even more: default for *what* exactly ?

Any URL w/ http://... ? or ftp ? HTML files ? Perhaps other files
that moz can handle (eg. images) ? ... the definition of for what
the default should apply isn't even clear and differs between
platforms. Can of worms.

IMHO, this strictly belongs into the control of the DE (if there
even is any - or the user/operator). The only actual audience I see
here are people using installers (which are a debatable concept
anyways), so that stuff should be moved there.


--mtx