Enable MOZILLA_OFFICIAL #97

ahmedalsudani commented 2025年06月07日 00:16:08 +02:00

The restart shortcut (Ctrl+Alt+R) conflicts with the shortcut for reader
mode. This change disables it so reader mode can be toggled with the
shorctut.

Testing:

• verified Ctrl+Alt+R does not restart the browser
• verified Ctrl+Alt+R toggles reader mode

Tested on Fedora 41

Fixes #1192 and #1610

The restart shortcut (Ctrl+Alt+R) conflicts with the shortcut for reader mode. This change disables it so reader mode can be toggled with the shorctut. Testing: - verified Ctrl+Alt+R does not restart the browser - verified Ctrl+Alt+R toggles reader mode Tested on Fedora 41 Fixes #1192 and #1610

sertonix commented 2025年06月07日 00:35:24 +02:00

This seems to be caused by the fact that we don't have MOZILLA_OFFICIAL set. We should consider setting that option to avoid diverging from upstrean defaults. Then we don't need to patch the behaviour.

The way I read this doc it shouldn't be a problem to set that variable: https://searchfox.org/mozilla-central/source/toolkit/components/glean/docs/dev/preferences.md#91

This seems to be caused by the fact that we don't have MOZILLA_OFFICIAL set. We should consider setting that option to avoid diverging from upstrean defaults. Then we don't need to patch the behaviour. The way I read this doc it shouldn't be a problem to set that variable: https://searchfox.org/mozilla-central/source/toolkit/components/glean/docs/dev/preferences.md#91

ahmedalsudani commented 2025年06月07日 01:51:23 +02:00

Yeah it's easier to set that variable. I assumed there was a reason it hadn't been enabled--just pushed a change replacing the patch with setting the variable.

I tested that the browser starts and that Ctrl+Alt+R behaves as desired. Did no test anything beyond that.

Yeah it's easier to set that variable. I assumed there was a reason it hadn't been enabled--just pushed a change replacing the patch with setting the variable. I tested that the browser starts and that Ctrl+Alt+R behaves as desired. Did no test anything beyond that.

maltejur commented 2025年06月07日 23:07:35 +02:00

There do seem to be many places MOZ_OFFICIAL is used, so we should be careful before we just enable that. For example, just some of the places we should double-check before enabling this:

• https://searchfox.org/mozilla-central/rev/7f7e8f6e4b8e09b145d29a57a1b341c6b11f4225/toolkit/library/rust/gkrust-features.mozbuild#63-64
• https://searchfox.org/mozilla-central/rev/7f7e8f6e4b8e09b145d29a57a1b341c6b11f4225/modules/libpref/init/all.js#3285-3291
• https://searchfox.org/mozilla-central/rev/7f7e8f6e4b8e09b145d29a57a1b341c6b11f4225/modules/libpref/init/all.js#3347-3353
• https://searchfox.org/mozilla-central/rev/7f7e8f6e4b8e09b145d29a57a1b341c6b11f4225/browser/extensions/formautofill/test/browser/creditCard/browser_manageCreditCardsDialog.js#203-207

We should consider setting that option to avoid diverging from upstrean defaults.

That is also a good point, but have we so far seen any other problems because MOZ_OFFICIAL is not set? If not, we may just be better off patching out the restart shortcut. (削除) This would be the place for that. And with a patch, we could even add a pref for users to choose which of the two shortcuts they want. (削除ここまで) Edit: Oh, I see you already created that patch originally :D

It may be interesting to see what other "third-party" Firefox builds/forks do here.

There do seem to be many [places `MOZ_OFFICIAL` is used](https://searchfox.org/mozilla-central/search?q=MOZILLA_OFFICIAL), so we should be careful before we just enable that. For example, just some of the places we should double-check before enabling this: - https://searchfox.org/mozilla-central/rev/7f7e8f6e4b8e09b145d29a57a1b341c6b11f4225/toolkit/library/rust/gkrust-features.mozbuild#63-64 - https://searchfox.org/mozilla-central/rev/7f7e8f6e4b8e09b145d29a57a1b341c6b11f4225/modules/libpref/init/all.js#3285-3291 - https://searchfox.org/mozilla-central/rev/7f7e8f6e4b8e09b145d29a57a1b341c6b11f4225/modules/libpref/init/all.js#3347-3353 - https://searchfox.org/mozilla-central/rev/7f7e8f6e4b8e09b145d29a57a1b341c6b11f4225/browser/extensions/formautofill/test/browser/creditCard/browser_manageCreditCardsDialog.js#203-207 > We should consider setting that option to avoid diverging from upstrean defaults. That is also a good point, but have we so far seen any other problems because `MOZ_OFFICIAL` is not set? If not, we may just be better off patching out the restart shortcut. ~[This](https://searchfox.org/mozilla-central/rev/7f7e8f6e4b8e09b145d29a57a1b341c6b11f4225/browser/base/content/browser-init.js#481-483) would be the place for that. And with a patch, we could even add a pref for users to choose which of the two shortcuts they want.~ Edit: Oh, I see you already created that patch originally :D It may be interesting to see what other "third-party" Firefox builds/forks do here.

sertonix commented 2025年06月08日 03:18:35 +02:00

In various cases !MOZILLA_OFFICIAL seems to be considered a development/local build which is definetly not what we want. eg. an approximation of "isLocalBuild", check for an unofficial build. https://searchfox.org/mozilla-central/source/devtools/client/framework/browser-toolbox/Launcher.sys.mjs#341

So I would argue that we need to be more carefull if we don't set MOZILLA_OFFICIAL.

In various cases `!MOZILLA_OFFICIAL` seems to be considered a development/local build which is definetly not what we want. eg. `an approximation of "isLocalBuild", check for an unofficial build.` https://searchfox.org/mozilla-central/source/devtools/client/framework/browser-toolbox/Launcher.sys.mjs#341 So I would argue that we need to be more carefull if we don't set `MOZILLA_OFFICIAL`.

sertonix commented 2025年06月08日 03:35:44 +02:00

have we so far seen any other problems because MOZ_OFFICIAL is not set?

I found at least 3 prefs in librewolf.cfg which were only needed cause we don't set MOZILLA_OFFICIAL.

Ref librewolf/settings@ebec9c7db2
And also devtools.debugger.remote-enabled

> have we so far seen any other problems because MOZ_OFFICIAL is not set? I found at least 3 prefs in librewolf.cfg which were only needed cause we don't set MOZILLA_OFFICIAL. Ref https://codeberg.org/librewolf/settings/commit/ebec9c7db23ec1d1407da547b05207f49ff9c575 And also `devtools.debugger.remote-enabled`

maltejur commented 2025年06月08日 19:33:42 +02:00

In various cases !MOZILLA_OFFICIAL seems to be considered a development/local build which is definetly not what we want

I found at least 3 prefs in librewolf.cfg which were only needed cause we don't set MOZILLA_OFFICIAL.

Ref ebec9c7db2 And also devtools.debugger.remote-enabled

Hm, that's true. I suppose it is probably a good idea then to set MOZILLA_OFFICIAL.

If there aren't any other opinions on this, I would say we should merge this in the next days. As a follow-up before the next release, I'll then go through the list of all uses of MOZILLA_OFFICIAL, and try to figure out if contains any stuff we don't want enabled.

> In various cases !MOZILLA_OFFICIAL seems to be considered a development/local build which is definetly not what we want > I found at least 3 prefs in librewolf.cfg which were only needed cause we don't set MOZILLA_OFFICIAL. > > Ref [`ebec9c7db2`](https://codeberg.org/librewolf/settings/commit/ebec9c7db23ec1d1407da547b05207f49ff9c575) And also `devtools.debugger.remote-enabled` Hm, that's true. I suppose it is probably a good idea then to set MOZILLA_OFFICIAL. If there aren't any other opinions on this, I would say we should merge this in the next days. As a follow-up before the next release, I'll then go through the list of all uses of MOZILLA_OFFICIAL, and try to figure out if contains any stuff we don't want enabled.

maltejur commented 2025年06月24日 13:30:23 +02:00

FYI: I wrote down the changes this would include in librewolf/issues#2532 (comment)

FYI: I wrote down the changes this would include in https://codeberg.org/librewolf/issues/issues/2532#issuecomment-5547800

maltejur commented 2025年06月26日 23:30:18 +02:00

@maltejur wrote in #97 (comment):

As a follow-up before the next release, I'll then go through the list of all uses of MOZILLA_OFFICIAL, and try to figure out if contains any stuff we don't want enabled.

As we are ready to release 140, and there are still things to do in librewolf/issues#2532 (comment), I won't merge this yet, but after the 140 release.

@maltejur wrote in https://codeberg.org/librewolf/source/pulls/97#issuecomment-5049042: > As a follow-up before the next release, I'll then go through the list of all uses of MOZILLA_OFFICIAL, and try to figure out if contains any stuff we don't want enabled. As we are ready to release 140, and there are still things to do in https://codeberg.org/librewolf/issues/issues/2532#issuecomment-5547800, I won't merge this yet, but after the 140 release.

celenity commented 2025年06月30日 09:15:29 +02:00

Late to the party, but yeah, I agree we should use MOZILLA_OFFICIAL.

We've been using it on IronFox for some time now, and for the most part, we haven't had issues or needed to make changes.

As you pointed out though @maltejur, there are some minor downsides - but we updated our gecko-disable-telemetry patch` to address those concerns. Notably:

• We disabled the Glean crate upload by changing if not CONFIG["MOZILLA_OFFICIAL"]: to if not CONFIG["0"]: on that file for the gkrust_features += ["glean_disable_upload"] line.
• We did also disable the telemetry being marked as official by setting GetIsOfficialTelemetry to always return false for redundancy.

You're of course welcome to use/adapt our fixes here or anything else from our patch - a few of the other unrelated changes there are specific to Android, but a lot would also apply to Desktop.

But yeah, we haven't had to deal with any other issues. LibreWolf should definitely enable MOZILLA_OFFICIAL here to ensure we don't enable any undesired functionality meant for debugging/development/etc.

Late to the party, but yeah, I agree we should use `MOZILLA_OFFICIAL`. We've been using it on IronFox for some time now, and for the most part, we haven't had issues or needed to make changes. As you pointed out though @maltejur, there are some minor downsides - but we updated our [`gecko-disable-telemetry` patch`](https://codeberg.org/ironfox-oss/IronFox/src/branch/dev/patches/gecko-disable-telemetry.patch) to address those concerns. Notably: - We [disabled the Glean crate upload](https://codeberg.org/ironfox-oss/IronFox/src/commit/70038ef6d4de4ebcf86c5c972465c272426a5b8f/patches/gecko-disable-telemetry.patch#L220) by changing `if not CONFIG["MOZILLA_OFFICIAL"]:` to `if not CONFIG["0"]:` on that file for the `gkrust_features += ["glean_disable_upload"]` line. - We did also [disable the telemetry being marked as `official`](https://codeberg.org/ironfox-oss/IronFox/src/commit/70038ef6d4de4ebcf86c5c972465c272426a5b8f/patches/gecko-disable-telemetry.patch#L160) by setting `GetIsOfficialTelemetry` to always return `false` for redundancy. You're of course welcome to use/adapt our fixes here or anything else from our patch - a few of the other unrelated changes there are specific to Android, but a lot would also apply to Desktop. But yeah, we haven't had to deal with any other issues. LibreWolf should definitely enable `MOZILLA_OFFICIAL` here to ensure we don't enable any undesired functionality meant for debugging/development/etc.

keithbowes commented 2025年07月09日 18:12:20 +02:00

I thought about suggesting this but figured it would be rejected due to breakages, so I just added this to my userChrome.css:

 #menu_FileRestartItem {
 display: none;
}

It doesn't do anything about the hotkey conflict but it does hide that annoying "Restart (developer)" menu item.

I thought about suggesting this but figured it would be rejected due to breakages, so I just added this to my userChrome.css: ```css #menu_FileRestartItem { display: none; } ``` It doesn't do anything about the hotkey conflict but it does hide that annoying "Restart (developer)" menu item.

maltejur commented 2025年07月26日 13:26:38 +02:00

@maltejur wrote in #97 (comment):

As we are ready to release 140, and there are still things to do in librewolf/issues#2532 (comment), I won't merge this yet, but after the 140 release.

This is still the case for 141. Sorry that I haven't gotten to those things yet, but it definetely isn't off the radar.

@maltejur wrote in https://codeberg.org/librewolf/source/pulls/97#issuecomment-5618783: > As we are ready to release 140, and there are still things to do in librewolf/issues#2532 (comment), I won't merge this yet, but after the 140 release. This is still the case for 141. Sorry that I haven't gotten to those things yet, but it definetely isn't off the radar.

any1here commented 2025年11月30日 19:01:01 +01:00

This should definitely be considered. I just started looking into this, and not having MOZILLA_OFFICIAL=1 set enables various developer and debug options, which should not be enabled for end users.

This should definitely be considered. I just started looking into this, and not having MOZILLA_OFFICIAL=1 set enables various developer and debug options, which should not be enabled for end users.

This pull request can be merged automatically.

You are not authorized to merge this pull request.