Remove a few legacy prefs #228
No reviewers
Labels
No labels
bug
confirmed
contribution welcome
duplicate
enhancement
good first issue
help wanted
important
invalid
other
question
upstream
web compat
wontfix
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
celenity/Phoenix!228
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "any1here/Phoenix:dev"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
I am going through old issues and found some prefs that are not used anymore.
toolkit.telemetry.coverage.opt-out -> toolkit.coverage.opt-out
lightweightThemes.getMoreURL
browser.newtabpage.activity-stream.feeds.section.topstories.options -> pocket is dead
I removed network.trr.allow-rfc1918 being set to true a while back
Thanks for the PR!
It looks like
browser.newtabpage.activity-stream.feeds.section.topstories.optionsis still used in ESR, so let's keep it for now, but add a[ESR]tag at the end, so that we can remember to remove it for next ESR cycle.(In general, I don't really recommend or go out of my way to support ESR, but I don't mind keeping prefs around for it, so long as they don't cause issues for release/elsewhere).
We should also probably update the reference URL to
https://searchfox.org/firefox-esr140/rev/8c69555d/browser/extensions/newtab/lib/ActivityStream.sys.mjs#187.Everything else looks good, appreciate the contribution!
@celenity wrote in #228 (comment):
I can add it back, but since Pocket is dead, it should not be able to show any stories anymore.
Edit: Or did you mean the UI toggle?
Hope you don't mind me adding a few more things while I go through Phoenix.
Not sure if I am reading the code wrong/missing something, but shouldn't
intl.allow-insecure-text-inputbe OSX only?I just tested
browser.preferences.experimental.hiddenbeing set totrueand it did not appear at all on first launch.there's a policy too, since v130.0.1. also the dependence on telemetry/studies (comment) no longer applies in version 146+ (ref
-> should it be allowed to be enabled by the user?), but still does apply to ESR.from the ticket:
Since https://mozilla-ohttp.fastly-edge.com does not return anything anymore
@degausser Nice to see you here too :)
Thank you so much, great work here @any1here!
@any1here wrote in #228 (comment):
You're right, good catch. I see you added the
[OSX-ONLY]tag to the end of theintl.allow-insecure-text-inputpref - also make sure to add it to the end of the title (/// Crash on insecure password input) - otherwise it'll just result in an empty/// Crash on insecure password inputsection appearing in the prefs for releases outside of OS X.Not sure about removing
browser.search.widget.inNavBar. I am not able to find anything setting this. If I missed something, I will revert that commit.fyi,
network.proxy.type != 5prevents BITS usage for background updates (= via the scheduled task)user_pref("app.update.background.previous.reasons", '["on Windows but cannot usually use BITS"]')https://searchfox.org/firefox-main/source/toolkit/mozapps/update/BackgroundUpdate.sys.mjs#213-219
https://searchfox.org/firefox-main/source/toolkit/mozapps/update/UpdateService.sys.mjs#864-872
I made accessibility.uia.enable windows only, since it only has any meaningful impact there. No idea why they set it for other platforms.
I added
privacy.restrict3rdpartystorage.heuristic.recently_visited_timewith value0for defense in depth. Normally it does nothing sinceprivacy.restrict3rdpartystorage.heuristic.recently_visitedis set tofalse@degausser
Do you know if setting
security.sandbox.content.shadow-stack.enabledtotruecauses any problems? I added it in the default false state.@any1here #110
also as said in the librewolf pull comments,
network.http.referer.defaultPolicy.trackers*->1breaks youtube embeds with error 153, e.g. itch.io homepagehttps://developers.google.com/youtube/terms/required-minimum-functionality#embedded-player-api-client-identity:~:text=strict%2Dorigin%2Dwhen%2Dcross%2Dorigin
@degausser
I was planning on adding your recommendations after I had finished my initial scroll through of Phoenix. Currently I am just seeing what catches my eye.
3baeec1ab9to5c817ed60fI have finished my scroll through. Everything below this comment (except 1 2) come from suggestions @degausser made here librewolf/settings#107 (comment)
Or should this be a new PR?
@any1here native messaging is covered in the WebCompat wiki entry and was fairly recently closed as wontfix. i stand by what i wrote about it in the comment (disabled signons + keepassxc not working ootb is bad), but i don't think it's a wanted change in phoenix. (likely the same with jit, fpp, rs intervals...). for example disabling push is covered in Network Connections as "not recommended".
if desired, a new pr in general seems cleaner, to facilitate merging this one without hassle.
@celenity for phoenix, at least these could be considered:
permissions.default.localhost->2could at least get a wiki entry about this; one would already need a BlockLAN (on by default in assets.json) uBO list @@ scoped exception to get it working, and then allow the permission prompt.btw,
dom.text_fragments.create_text_fragment.enabledno longer exists in firefox-main, and the entry is in the context menu by default; it exists in ESR as default false.I will leave this PR as-is until @celenity can give input on what can be changed in Phoenix.
Edit: I reverted the last 2 commits regarding native-messaging
Few minor changes, but overall looks good! Thank you very much again for your time and work here @any1here, very much appreciated.
@ -423,10 +423,6 @@ pref("doh-rollout.trrRace.randomSubdomainCount", 0); // [HIDDEN]// https://firefox.settings.services.mozilla.com/v1/buckets/main/collections/moz-essential-domain-fallbacks/changeset?_expected=0pref("network.essential_domains_fallback", false); // [DEFAULT]/// Disable FakespotDespite Fakespot no longer operating, it looks like these prefs are still used for "private attribution", so I think we should keep them.
I can add it back, but like I said before, since https://mozilla-ohttp.fastly-edge.com does not serve anything anymore, it does nothing https://searchfox.org/firefox-main/rev/3eaf7e2acf8186eb7aa579561eaa1312cb89132b/toolkit/components/dap/DAPSender.sys.mjs#292
any1here/Phoenix@6d06e21e4f@ -2945,0 +2930,4 @@// 0: Never. [WINDOWS-ONLY]// 1: Always. [WINDOWS-ONLY]// 2: Enable unless incompatible accessibility clients are detected. (default) [WINDOWS-ONLY]pref("accessibility.uia.enable", 0); // [WINDOWS-ONLY]I agree with your conclusion that this pref only appears to have an impact on Windows, but since it is also defined for other platforms, I think we should still set it there to be safe. I'd replace
[WINDOWS-ONLY]with[Windows]- that'll indicate the fact that it only takes effect on Windows, but will ensure it's still set for other platforms.any1here/Phoenix@46f3067a5eI only added it after the pref. Hope that is fine@ -3129,0 +3117,4 @@// 0=no-referrer, 1=same-origin, 2=strict-origin-when-cross-origin (default),// 3=no-referrer-when-downgrade.// Setting to 1 currently breaks various functionality https://codeberg.org/celenity/Phoenix/pulls/228#issuecomment-10051167pref("network.http.referer.defaultPolicy.trackers", 2); // [DEFAULT]I don't love relaxing these, but the breakage it causes does seem problematic.
We could technically set
urlclassifier.trackingAnnotationSkipURLstohttps://youtube.com/*instead for the instance of YouTube breaking - but I don't think that's a good solution since I believe it would also result in other tracking protections being relaxed for YouTube.So, until we can find a better solution, I agree with your approach - it seems like setting these to
2is the best move.@ -3333,6 +3322,7 @@ pref("security.sandbox.gmp.shadow-stack.enabled", true); // [WINDOWS-ONLY] [DEFApref("security.sandbox.gpu.shadow-stack.enabled", true); // [WINDOWS-ONLY] [DEFAULT]pref("security.sandbox.rdd.shadow-stack.enabled", true); // [WINDOWS-ONLY] [DEFAULT]pref("security.sandbox.socket.shadow-stack.enabled", true); // [WINDOWS-ONLY] [DEFAULT]pref("security.sandbox.content.shadow-stack.enabled", false); // [WINDOWS-ONLY] [DEFAULT]I don't think we should explicitly set this to false, so that once it's ready and Mozilla decides to enable it by default upstream, it'll also be enabled for our users.
I think it's worth keeping this pref for reference though, but let's just comment it out, and maybe add a link to #110.
any1here/Phoenix@af92ef45e9@ -3417,3 +3407,3 @@/// Protect against MIME Exploits// https://www.pcmag.com/encyclopedia/term/mime-exploitpref("dom.workers.importScripts.enforceStrictMimeType", true); // [DEFAULT]pref("dom.workers.importScripts.enforceStrictMimeType", true); // [DEFAULT] [EST]Minor typo: I assume you meant
[ESR].any1here/Phoenix@3eb28cb5a9Nah, this is a timezone specific setting now@ -3819,3 +3800,2 @@pref("network.dnsCacheExpiration", 3600); // (Default = 60)pref("network.dnsCacheExpirationGracePeriod", 120); // (Default = 60)pref("network.dnsCacheEntries", 10000); // (Default = 800)pref("network.dnsCacheExpirationGracePeriod", 600); // (Default = 600)TBH, I think we should just remove this pref (
network.dnsCacheExpirationGracePeriod) - I don't see a point in explicitly setting the default there.any1here/Phoenix@4231c4cb68Thank you for your feedback @degausser! To address your points:
Do you mind sharing more details here - ex. can you provide an example of an impacted website (would help for testing)? I'm not saying that you're wrong, this may very well be an issue (and if so, those prefs can indeed be reconsidered. I'm just surprised to hear this as it's not a problem I've encountered or heard from other users).
I don't like having to relax those prefs, but I agree that it unfortunately seems necessary, so I'm fine with setting those back to the default values (
2) until we can find a better solution (This should be covered by @any1here's PR).I see what you're saying - it appears to do this on at least macOS as well (I just tested it). Will have to think about it - I think it's overall a nice feature from a UX perspective, but I agree it's not ideal that it prevents the hand cursor from drawing unless the whole page has loaded.
Fair enough, good catch. I'll enable
browser.taskbar.lists.enabledandbrowser.taskbar.lists.tasks.enabledfor next release - those seem both harmless and legitimately useful. I think we should probably keepbrowser.taskbar.lists.frequent.enabledandbrowser.taskbar.lists.recent.enabledthough.You're right, will enable for next release. No reason to disable IMO.
Not sure how to feel about this one, curious how others feel.
Great point, thanks for bringing this up. Will do some testing. Honestly, instead of removing
permissions.default.localhost, I think I'd rather remove/disable the Block LAN uBo list - I agree we shouldn't set both, and I think it's better to rely on native browser features for functionality like this over extensions where possible.@celenity Thank you for your consideration.
can be tested e.g. here ->
https://www.techspot.com/. I recall testing it in late December - it manifested as a intermittently stuck redirect, after automatically or manually ticking the captcha box. However I cannot reliably reproduce it now - the prompt informing of the redirect flashing multiple (2-3) times isn't ideal (I guess it does its job), but doesn't prevent the turnstile to eventually suceed. However, I tested with JIT disabled back then, but with stock Firefox settings now, so maybe the js check just sometimes timed out, on slow(er) hardware. Still not great, but reload (CTRL+R) eventually coerced it through (turnstile again->website).As far as I can tell, the ui in
about:preferences#privacydoesn't allow manually entering exceptions, and with2it doesn't show any indicator in urlbar, to add an 'allow' exception for current webpage. The BlockLAN list are mostly exceptions; all of them would break. IMHO0(default prompt) and keeping the list is a better choice, at least for now.btw, click'n'load-relevant entry
LGTM! Thanks again for all your time and work here @any1here, very much appreciated ❤️.
@degausser
Yeah, I did some testing, and I'm inclined to agree with you. Fixed for next release:
62fb7dd81e.