Revamp browser install scripts with snap/flatpak support and merge reset-theme into global-theme#1339
Revamp browser install scripts with snap/flatpak support and merge reset-theme into global-theme#1339thepinak503 wants to merge 7 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9794274a31
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if command_exists snap && snap list firefox 2>/dev/null | grep -q firefox; then | ||
| printf "%b\n" "${YELLOW}Removing Snap Firefox...${RC}" | ||
| "$ESCALATION_TOOL" snap remove firefox |
There was a problem hiding this comment.
On Ubuntu systems where the current snap/transitional Firefox is already installed, command_exists firefox is true before this block is reached, so the script prints "already installed" and never removes the snap or configures the Mozilla apt repo. That leaves exactly the snap package this new Ubuntu path is meant to replace; detect/remove the snap before the outer installed check or distinguish snap Firefox from the desired apt install.
Useful? React with 👍 / 👎.
| apt-get|nala) | ||
| "$ESCALATION_TOOL" rm -fv /etc/apt/sources.list.d/thorium.list | ||
| "$ESCALATION_TOOL" curl http://dl.thorium.rocks/debian/dists/stable/thorium.list -o /etc/apt/sources.list.d/thorium.list | ||
| "$ESCALATION_TOOL" wget --no-hsts -P /etc/apt/sources.list.d/ http://dl.thorium.rocks/debian/dists/stable/thorium.list |
There was a problem hiding this comment.
The apt/nala path now depends on wget, but checkEnv only verifies curl and this script does not install wget before invoking it. On minimal Debian/Ubuntu systems where wget is absent, Thorium installation fails here after deleting the old repo file, so either keep using the checked curl dependency or install/require wget first.
Useful? React with 👍 / 👎.
| printf "%b\n" "2. ${YELLOW}Reset Theme to Defaults${RC}" | ||
| printf "%b\n" "3. ${YELLOW}Cancel${RC}" | ||
| printf "%b" "Enter your choice [1-3]: " | ||
| read -r CHOICE |
There was a problem hiding this comment.
This newly added menu makes Global Theme interactive, but its catalog entry still defaults to multi_select = true, so users can queue it with other commands in a batch. The merged runner will reach this read and block or consume terminal input before later queued tasks; this is the interactive/state-dependent case the repo instructions require opting out of, so set the Global Theme entry to multi_select = false with this change.
Useful? React with 👍 / 👎.
| if command_exists sudo; then | ||
| sudo sed -i '/^QT_QPA_PLATFORMTHEME=/d; /^GTK_THEME=/d' /etc/environment 2>/dev/null || true |
There was a problem hiding this comment.
The reset path only edits /etc/environment when sudo exists, ignoring the already selected ESCALATION_TOOL. On supported doas-only systems, or when running as root without sudo installed, a previous dark-theme run can leave GTK_THEME/QT_QPA_PLATFORMTHEME in /etc/environment, so the reset reports success while global dark theming still comes back after login.
Useful? React with 👍 / 👎.
| "$ESCALATION_TOOL" gpg --batch --yes --dearmor -o /etc/apt/keyrings/packages.mozilla.org.gpg /tmp/mozilla-key.gpg 2>/dev/null || { | ||
| "$ESCALATION_TOOL" cp /tmp/mozilla-key.gpg /etc/apt/keyrings/packages.mozilla.org.asc |
There was a problem hiding this comment.
If gpg --dearmor fails or gpg is not installed, the fallback writes the Mozilla key to packages.mozilla.org.asc, but the generated apt source still points Signed-By at packages.mozilla.org.gpg. In that environment apt update cannot use the fallback key file, so either install/require gpg or make the source reference the actual fallback path.
Useful? React with 👍 / 👎.
…TOOL, multi_select
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6101346397
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if [ "$DTYPE" = "ubuntu" ] && command_exists snap && snap list firefox 2>/dev/null | grep -q firefox; then | ||
| printf "%b\n" "${YELLOW}Removing Snap Firefox...${RC}" | ||
| "$ESCALATION_TOOL" snap remove firefox |
There was a problem hiding this comment.
On stock Ubuntu 22.04/24.04 systems the firefox deb package is a transitional wrapper to the snap, so after this removes the snap, command_exists firefox can still succeed via /usr/bin/firefox. In that case the new Mozilla APT setup is skipped and the user is left with a broken/no Firefox instead of the requested install; purge the transitional package or perform the repo install before relying on command_exists.
Useful? React with 👍 / 👎.
|
|
||
| resetWMCommon() { | ||
| for wm_dir in i3 bspwm qtile dwm; do | ||
| rm -f "$HOME/.config/$wm_dir/config" "$HOME/.config/$wm_dir/theme" 2>/dev/null || true |
There was a problem hiding this comment.
When option 2 reaches this helper for i3/bspwm/qtile/dwm, and also from the Sway/Hyprland branches that call it, this removes the user's main WM config files rather than just undoing theme changes. Those files commonly contain keybindings, autostart commands, monitor layout, and other session-critical settings, so a "Reset Theme to Defaults" can irreversibly break the user's desktop without a backup or confirmation; limit the reset to theme-owned settings or preserve these configs.
Useful? React with 👍 / 👎.
|
To use Codex here, create an environment for this repo. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41bc97f789
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| resetSway() { | ||
| rm -f "$HOME/.config/sway/config" 2>/dev/null || true |
There was a problem hiding this comment.
When XDG_CURRENT_DESKTOP is Sway, choosing "Reset Theme to Defaults" removes ~/.config/sway/config, which normally contains keybindings, outputs, autostart commands, and other session behavior rather than just theme settings. Because resetDE calls this before printing that tiling-WM configs are not theme-only, a user trying to reset colors can lose their working WM configuration; this path should skip Sway or edit only known theme-related settings.
Useful? React with 👍 / 👎.
| if command_exists firefox; then | ||
| "$ESCALATION_TOOL" "$PACKAGER" purge -y firefox 2>/dev/null || true | ||
| fi |
There was a problem hiding this comment.
On Ubuntu, after this script has installed the Mozilla .deb once, command_exists firefox is true and snap list firefox is false, so a later run purges the existing firefox package before reaching the idempotent install check. If the Mozilla repo update or reinstall then fails, the user is left without Firefox; the removal should be limited to the Ubuntu transitional/snap package or avoided when the installed package is already the intended native Firefox.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81482595ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| lookandfeeltool -a org.kde.breezedark.desktop | ||
| successOutput | ||
| exit 0 | ||
| return |
There was a problem hiding this comment.
In a KDE session, choosing "Apply Dark Theme" now returns from applyTheming instead of terminating as it did before, so applyDarkTheme continues into the generic setup and writes QT_QPA_PLATFORMTHEME=qt6ct globally. That overrides KDE's native Qt platform theme for future sessions and can break Breeze/KDE integration after the script already applied org.kde.breezedark.desktop; keep the KDE path terminal or skip the generic Qt env setup for KDE.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| resetOpenbox() { | ||
| rm -f "$HOME/.config/openbox/rc.xml" 2>/dev/null || true |
There was a problem hiding this comment.
When XDG_CURRENT_DESKTOP resolves to Openbox and the user chooses "Reset Theme to Defaults", this removes the whole rc.xml and replaces it with the system default. That file contains keybindings, mouse bindings, and application rules in addition to theme settings, so a theme reset silently discards unrelated Openbox customization; update only the theme fields or warn/skip like the other WM-specific paths.
Useful? React with 👍 / 👎.
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…issing trailing newlines
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98ce27cdec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if [ "$DTYPE" = "ubuntu" ] && command_exists snap && snap list firefox 2>/dev/null | grep -q firefox; then | ||
| printf "%b\n" "${YELLOW}Removing Snap Firefox...${RC}" | ||
| "$ESCALATION_TOOL" snap remove firefox |
There was a problem hiding this comment.
On Ubuntu systems that already have Snap Firefox, this removes the working snap before the Mozilla apt repo is added and before the deb install succeeds. Because the script runs with set -e, any later network, repo, or apt failure aborts after this point and leaves the user without Firefox. Install and validate the replacement first, or postpone snap removal until after the apt install completes successfully.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| resetLXQt() { | ||
| rm -f "$HOME/.config/lxqt/lxqt.conf" "$HOME/.config/lxqt/panel.conf" "$HOME/.config/lxqt/session.conf" 2>/dev/null || true |
There was a problem hiding this comment.
For LXQt users who choose Reset Theme to Defaults, this deletes panel.conf and session.conf along with lxqt.conf. Those files hold panel layout and session settings, so a theme reset can wipe unrelated desktop configuration without a backup or explicit warning. Limit this path to LXQt appearance settings, or back up these files before removing them.
Useful? React with 👍 / 👎.
|
First of all, one feature - one PR. You are doing two things, which are affected in 2 different vectors. I make a bet that @ChrisTitusTech will just say "PR is too bad" and will just close that |
|
So this is where all my tokens went... |
Wait, what? D: |
|
Split these up |
3 participants
Summary
This PR overhauls the browser installation scripts across the board and merges the reset theme functionality directly into
global-theme.shthere is no need of more separate script.Browser changes
Every browser script now properly handles Debian-based distros with smart package manager routing:
curl | shapproachwget ca-certificatesdeps, downloads to/tmpinstead of cwd, flatpak as fallbackcurltowget --no-hstsfor repo setupGlobal theme revamp
global-theme.shnow has a clean menu:The reset function detects the desktop environment automatically and restores default themes, icons, cursors, fonts, wallpapers, and panel layouts. It also cleans up GTK/QT config files and theme environment variables that might have been left behind. Supports GNOME, KDE, XFCE, Cinnamon, MATE, Budgie, LXDE, LXQt, Deepin, Pantheon, Sway, Hyprland, i3, bspwm, dwm, qtile, Openbox, Enlightenment, UKUI, Lomiri, and COSMIC.
Other checklists
checkSnap()helper tocommon-script.shalongside the existingcheckFlatpak()bash -n