firefox: read the appName for search engine disclaimer from package#5685
Merged
rycee merged 1 commit intonix-community:masterfrom Nov 17, 2024
Merged
Conversation
6 tasks
6 tasks
donovanglover
approved these changes
Aug 13, 2024
Contributor
donovanglover
left a comment
donovanglover
left a comment
There was a problem hiding this comment.
This fixed changing the default search engine for me with LibreWolf and #5684. Looking forward to the merge!
MithicSpirit
approved these changes
Sep 2, 2024
Contributor
MithicSpirit
left a comment
MithicSpirit
left a comment
There was a problem hiding this comment.
Been using this and #5684 for a couple of days now, and it's been working very well.
Contributor
|
Merge conflict is due to #5697 |
kkoniuszy
force-pushed
the
firefox-read-appname-from-package
branch
from
49cc1ed to
e55fc68
Compare
github-actions
bot
added
the
mail
Contributor
Author
|
Conflict resolved |
Collaborator
|
we got kira-bruneau's approval so it's good to go, just need a rebase to fix ci |
kkoniuszy
force-pushed
the
firefox-read-appname-from-package
branch
2 times, most recently
from
f4b38b0 to
4ad0a2b
Compare
Using a fixed application name in the salt for the search engine name hash can break with minor branding changes. For example, LibreWolf 127 used the application name "LibreWolf", but in version 128 it is "Firefox". The proper name can be found in about:support -> Application Basics. Because it doesn't have to be related to the product name visible in most of the browser (for example in the window title and help menus), we shouldn't rely on cfg.name for that. The application name can be read from lib/*/application.ini and we can use that if the browser was installed via Home Manager. If not, we can fall back to cfg.name.
rycee
force-pushed
the
firefox-read-appname-from-package
branch
from
4ad0a2b to
aecd341
Compare
Member
|
Thanks! Merged to master now 🙂 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Using a fixed application name in the salt for the search engine name hash can break with minor branding changes. For example, LibreWolf 127 used the application name "LibreWolf", but in version 128 it's "Firefox".
The proper name can be found in about:support -> Application Basics.
Because it doesn't have to be related to the product name visible in most of the browser (for example in the window title and help menus), we shouldn't rely on cfg.name for that.
The application name can be read from lib/*/application.ini and we can use that if the browser was installed via home-manager. If not, we can fall back to cfg.name.
Checklist
Change is backwards compatible.
Code formatted with
./format.Code tested through
nix-shell --pure tests -A run.allornix develop --ignore-environment .#allusing Flakes.Test cases updated/added. See example.
Commit messages are formatted like
See CONTRIBUTING for more information and recent commit messages for examples.
If this PR adds a new module
Maintainer CC
@rycee @brckd @kira-bruneau