fix(linux): clean GTK4 tray follow-ups - #4
Conversation
* chore: run clippy on all platforms * Only install dependencies for linux * Use cross on Android * mac clippy * mobile clippy * run on `macos-latest` * duplicated cargo * mac clippy suggestion take 2 * ios clippy * Exclude tauri-cli and tauri-bundler on mobile * Exclude `tauri-cli-node` * just test api example on mobile * Fix audit * Android clippy * Run the event handler in test
…pps#15677) * chore(deps): serial_test 3.5.0 to deal with RUSTSEC advisory * add change file
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR bundles CI matrix updates for cross-platform Rust linting, GTK4/WebKitGTK6 supply-chain audit documentation and cargo-vet policy expansion, deprecation of TrayIconConfig.menuOnLeftClick, removal of unnecessary unsafe blocks on iOS/macOS, explicit lifetime parameters on Android JNI closures, desktop-only cfg gating, and tray-icon Linux backend documentation updates. ChangesCI and Supply-Chain
Tray Icon Deprecation and Docs
Safety and Lifetime Cleanups
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@CodeRabbit review please |
|
✅ Action performedReview finished.
|
|
@CodeRabbit what do you think about socket? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -ni 'socket' --type toml -g '**/Cargo.toml' 2>/dev/null | head -50
rg -ni 'socket' -g '*.rs' 2>/dev/null | head -50
fd -i socket 2>/dev/null | head -20Length of output: 412
Once I know the specific context, I can give a more useful opinion. (-.-)? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/tauri/src/app.rs (1)
680-687: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDon’t ignore
run_on_main_threadfailures here.
If dispatch fails, nothing sends ontxandrx.recv()can block forever. Returnfalseimmediately before waiting.Suggested fix
- let _ = self.run_on_main_thread(move || { + if self.run_on_main_thread(move || { let mtm = objc2::MainThreadMarker::new().unwrap(); let ui_application = objc2_ui_kit::UIApplication::sharedApplication(mtm); tx.send(ui_application.supportsMultipleScenes()).unwrap(); - }); + }).is_err() { + return false; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/tauri/src/app.rs` around lines 680 - 687, `supports_multiple_windows` currently ignores the result of `run_on_main_thread`, which can leave `rx.recv()` waiting forever if dispatch fails. Update `App::supports_multiple_windows` to check the `run_on_main_thread` call result and return `false` immediately on failure before attempting to receive from the channel; keep the existing `tx`/`rx` flow only for the successful path.
🧹 Nitpick comments (1)
.github/workflows/lint-rust.yml (1)
56-79: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider disabling credential persistence on checkout.
zizmor flags that
actions/checkout@v7at Line 59 doesn't setpersist-credentials: false, leaving theGITHUB_TOKENwritten to disk for the remainder of the job (all subsequent steps, includingcargo/crossinvocations that pull external code). Cheap hardening for defense-in-depth even though this workflow only runs trusted clippy.🔒 Proposed fix
- uses: actions/checkout@v7 + with: + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/lint-rust.yml around lines 56 - 79, The checkout step in the lint-rust workflow leaves GitHub credentials persisted on disk. Update the `actions/checkout@v7` step to disable credential persistence so the `GITHUB_TOKEN` is not retained for later steps like `cargo` or `cross`; this is the only change needed in the workflow’s checkout configuration.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/tauri/src/webview/webview_window.rs`:
- Around line 1282-1300: The iOS doc example in WebviewWindowBuilder uses an
undefined variable name, so update the snippet to use the existing builder
variable consistently when chaining with_input_accessory_view_builder. Keep the
example anchored to WebviewWindowBuilder::new and builder.build() so it compiles
and clearly shows the accessory view being applied.
In `@ports/tray-icon/README.md`:
- Around line 26-35: The Linux dependencies section has an inconsistent heading
depth, with the Arch Linux / Manjaro and Debian / Ubuntu subsections skipping a
level under Dependencies (Linux Only). Adjust those subsection headings in
README.md to match the surrounding hierarchy so the markdown structure stays
consistent and markdownlint is satisfied.
---
Outside diff comments:
In `@crates/tauri/src/app.rs`:
- Around line 680-687: `supports_multiple_windows` currently ignores the result
of `run_on_main_thread`, which can leave `rx.recv()` waiting forever if dispatch
fails. Update `App::supports_multiple_windows` to check the `run_on_main_thread`
call result and return `false` immediately on failure before attempting to
receive from the channel; keep the existing `tx`/`rx` flow only for the
successful path.
---
Nitpick comments:
In @.github/workflows/lint-rust.yml:
- Around line 56-79: The checkout step in the lint-rust workflow leaves GitHub
credentials persisted on disk. Update the `actions/checkout@v7` step to disable
credential persistence so the `GITHUB_TOKEN` is not retained for later steps
like `cargo` or `cross`; this is the only change needed in the workflow’s
checkout configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4af55f4e-e60e-4e1d-b6a2-c9826d17ad20
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockports/tray-icon/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
.changes/change-pr-15677.md.github/workflows/lint-rust.ymlaudits/GTK4_WebKit6_supply_chain_notes.mdcrates/tauri-bundler/Cargo.tomlcrates/tauri-cli/config.schema.jsoncrates/tauri-cli/src/helpers/pbxproj.rscrates/tauri-runtime-wry/src/lib.rscrates/tauri-runtime-wry/src/window/macos.rscrates/tauri-runtime-wry/src/window/mod.rscrates/tauri-schema-generator/schemas/config.schema.jsoncrates/tauri-utils/Cargo.tomlcrates/tauri-utils/src/config.rscrates/tauri/src/app.rscrates/tauri/src/lib.rscrates/tauri/src/plugin/mobile.rscrates/tauri/src/test/mock_runtime.rscrates/tauri/src/tray/mod.rscrates/tauri/src/webview/webview_window.rsexamples/api/src-tauri/src/lib.rsports/tao/src/platform_impl/linux/icon.rsports/tray-icon/README.mdports/tray-icon/src/lib.rssupply-chain/config.toml
| /// tauri::Builder::default() | ||
| /// .setup(|app| { | ||
| /// let mut builder = tauri::WebviewWindowBuilder::new(app, "label", tauri::WebviewUrl::App("index.html".into())); | ||
| /// #[cfg(target_os = "ios")] | ||
| /// { | ||
| /// window_builder = window_builder.with_input_accessory_view_builder(|_webview| unsafe { | ||
| /// let mtm = objc2::MainThreadMarker::new_unchecked(); | ||
| /// let button = objc2_ui_kit::UIButton::buttonWithType(objc2_ui_kit::UIButtonType(1), mtm); | ||
| /// button.setTitle_forState( | ||
| /// Some(&objc2_foundation::NSString::from_str("Tauri")), | ||
| /// objc2_ui_kit::UIControlState(0), | ||
| /// ); | ||
| /// Some(button.downcast().unwrap()) | ||
| /// }); | ||
| /// } | ||
| /// let webview = builder.build()?; | ||
| /// Ok(()) | ||
| /// }); | ||
| /// ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant section with line numbers
sed -n '1260,1310p' crates/tauri/src/webview/webview_window.rs | cat -n
# Search for the method usage elsewhere in the repository
rg -n "with_input_accessory_view_builder|window_builder|let mut builder = tauri::WebviewWindowBuilder::new" crates/tauri -SRepository: OlympusLedgerOrg/tauri
Length of output: 1967
Fix the iOS doc example's builder variable
window_builder is undefined here; use builder = builder.with_input_accessory_view_builder(...) so the example compiles and the accessory view is applied.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/tauri/src/webview/webview_window.rs` around lines 1282 - 1300, The iOS
doc example in WebviewWindowBuilder uses an undefined variable name, so update
the snippet to use the existing builder variable consistently when chaining
with_input_accessory_view_builder. Keep the example anchored to
WebviewWindowBuilder::new and builder.build() so it compiles and clearly shows
the accessory view being applied.
| #### Arch Linux / Manjaro: | ||
|
|
||
| ```sh | ||
| pacman -S gtk3 xdotool libappindicator-gtk3 #or libayatana-appindicator | ||
| pacman -S gtk4 xdotool dbus | ||
| ``` | ||
|
|
||
| #### Debian / Ubuntu: | ||
|
|
||
| ```sh | ||
| sudo apt install libgtk-3-dev libxdo-dev libappindicator3-dev #or libayatana-appindicator3-dev | ||
| sudo apt install libgtk-4-dev libxdo-dev libdbus-1-dev |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the markdown heading depth consistent.
#### skips a level under ## Dependencies (Linux Only), which trips the markdownlint warning and makes the section hierarchy harder to scan.
Suggested fix
-#### Arch Linux / Manjaro:
+### Arch Linux / Manjaro:
@@
-#### Debian / Ubuntu:
+### Debian / Ubuntu:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #### Arch Linux / Manjaro: | |
| ```sh | |
| pacman -S gtk3 xdotool libappindicator-gtk3 #or libayatana-appindicator | |
| pacman -S gtk4 xdotool dbus | |
| ``` | |
| #### Debian / Ubuntu: | |
| ```sh | |
| sudo apt install libgtk-3-dev libxdo-dev libappindicator3-dev #or libayatana-appindicator3-dev | |
| sudo apt install libgtk-4-dev libxdo-dev libdbus-1-dev | |
| ### Arch Linux / Manjaro: | |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 26-26: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ports/tray-icon/README.md` around lines 26 - 35, The Linux dependencies
section has an inconsistent heading depth, with the Arch Linux / Manjaro and
Debian / Ubuntu subsections skipping a level under Dependencies (Linux Only).
Adjust those subsection headings in README.md to match the surrounding hierarchy
so the markdown structure stays consistent and markdownlint is satisfied.
Source: Linters/SAST tools
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
3 participants
Summary
Validation
This PR is for the fork only. The accidental upstream tauri-apps/tauri PR was closed and marked as opened by mistake.
Summary by CodeRabbit
Bug Fixes
linux-ksnibackend.Documentation
menuOnLeftClickno longer works since v2.2 and pointed to the replacement setting.Chores