Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

consolidate firefox incognitobrowser logic (and deprecate opera/launcher) #5805

Merged
merged 11 commits into from
Jan 11, 2025

Conversation

teknsl
Copy link
Contributor

@teknsl teknsl commented Jan 8, 2025

Mozilla’s firefox-nightly apt package now points to firefox-bin in its desktop file, catch all firefox variants by mozilla with one if statement

not sure what the changelog should be

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

src/util/IncognitoBrowser.cpp Outdated Show resolved Hide resolved
src/util/IncognitoBrowser.cpp Outdated Show resolved Hide resolved
src/util/IncognitoBrowser.cpp Outdated Show resolved Hide resolved
Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some nits, will test better later.

Changelog could be:

  • Minor: Treat all browsers starting with the filename firefox as Firefox browsers

src/util/IncognitoBrowser.cpp Outdated Show resolved Hide resolved
src/util/IncognitoBrowser.cpp Outdated Show resolved Hide resolved
src/util/IncognitoBrowser.cpp Outdated Show resolved Hide resolved
@teknsl
Copy link
Contributor Author

teknsl commented Jan 9, 2025

it would be possible to just do .startsWith in the loop instead of == now with basename being used, then the ifdef can be removed and a single firefox entry will do in the table and no if statement needed for it

on second thought too easy for conflicts

@Mm2PL
Copy link
Collaborator

Mm2PL commented Jan 9, 2025 via email

@teknsl
Copy link
Contributor Author

teknsl commented Jan 9, 2025

the desktop file provided by the package (/usr/share/applications/firefox-nightly.desktop) effectively points to /usr/bin/firefox-nightly which is a symlink to /usr/lib/firefox-nightly/firefox-bin, the original behaviour has no problem with this but it seems when you click make default browser in firefox it creates a new desktop file under ~/.local/share/applications/userapp-Nightly-XXXXXX.desktop (or XDG_DATA_HOME) which will point to /usr/lib/firefox-nightly/firefox-bin (and is made default)

not sure if this is new or old behavior ill try to test

this "new" behaviour is present in the firefox (133.0.3) firefox-esr (128.6.0) packages from mozilla but is not in the packages firefox-esr (128.6.0) firefox (134.0-1) from debian unstable

@teknsl teknsl requested a review from pajlada January 10, 2025 18:38
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

src/util/IncognitoBrowser.cpp Show resolved Hide resolved
@pajlada pajlada changed the title catch 'firefox-bin' and all mozilla browsers in one consolidate firefox incognitobrowser logic (and deprecate opera/launcher) Jan 11, 2025
@pajlada pajlada merged commit 687fb35 into Chatterino:master Jan 11, 2025
18 checks passed
@teknsl teknsl deleted the firefox-incognito branch January 11, 2025 21:54
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants