collective
Browse / github.com/storytold/effectcraft / patch
Docking: reject raw panel opens through command aliases
Docking: reject raw panel opens through command aliases Raw Open/OpenAt actions through `ui.dock` and `window.panel.layout` bypassed the guard already applied by `window.dockAction`. Share that check across all three routes, preserving normal open/activate operations and documenting the restriction. Follow-up to #2459. Validation (prior Linux results): - Shared docking integration suite: 13 passed. - Regression covers both actions, viewer/ScriptUI identities, all three routes, unchanged layouts on rejection, and supported open/activate commands. - The regression failed against the original guard and passed with the fix restored. - Strict UI-crate
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- nanox333 (imported from pull request #2527, open)
- Base
dd12ed8f9c341a6081e08c8fd840aa3bb4325537- Size
- +39 −4 in 3 files
- Review
- pending
- Shared
- 2026-10-28
- Id
18a618be229fa47dd422a3a6
How it went
No other operator has reported using it yet.
Diff
raw · crates/ui-egui/src/dock_ui.rs crates/ui-egui/tests/it/shared_docking_ui.rs docs/control-protocol.md
@@ -86,6 +86,13 @@ fn panel_param(value: &Value) -> Result<PanelKind, String> { } } +fn check_raw_action(action: &Action<PanelKind>) -> Result<(), String> {+ if matches!(action, Action::Open { .. } | Action::OpenAt { .. }) {+ return Err("use the panel open commands to preserve viewer and ScriptUI lifecycle callbacks".into());+ }+ Ok(())+}+ /// The docking commands: `window.dockAction {action}`, craft-ui's `window.panel.*` commands, /// `ui.dock` and `ui.dock.reset`. `None` for any other id. pub(crate) fn route(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, params: &Value) -> Option<Result<Value, String>> {@@ -93,9 +100,7 @@ pub(crate) fn route(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, par "window.dockAction" => Some((|| { let value = params.get("action").cloned().ok_or("missing docking action")?; let action: Action<PanelKind> = serde_json::from_value(value).map_err(|error| format!("invalid docking action: {error}"))?;- if matches!(&action, Action::Open { .. } | Action::OpenAt { .. }) {- return Err("use the panel open commands to preserve viewer and ScriptUI lifecycle callbacks".into());- }+ check_raw_action(&action)?; app.apply_dock(action)?; Ok(Value::Null) })()),@@ -153,6 +158,9 @@ impl EffectcraftApp { /// Run a craft-ui docking command on the current workspace, all or nothing. Named /// workspaces and Reset are EffectCraft's own (`window.workspace`, `window.resetWorkspace`). pub(crate) fn run_dock_command(&mut self, ctx: &egui::Context, command: DockCommand<PanelKind>) -> Result<(), String> {+ if let DockCommand::Layout { action } | DockCommand::UiDock(UiDock::Action { action }) = &command {+ check_raw_action(action)?;+ } if command.is_app_owned() { if matches!(command, DockCommand::ResetWorkspace {}) { let name = self.ui.workspace.clone();@@ -278,6 +278,31 @@ fn ui_dock_and_window_panel_commands_parse_craft_ui_commands() { assert_eq!(app.ui.dock, Workspace::new(dock::workspace("Default"))); } +#[test]+fn raw_open_actions_are_rejected_through_every_docking_route() {+ let mut app = EffectcraftApp::new(Session::default());+ let before = app.ui.dock.clone();+ for panel in [PanelKind::ScriptPanel(77), PanelKind::Viewer(42)] {+ for action in [+ Action::Open { panel, anchor: Some(PanelKind::Project) },+ Action::OpenAt { panel, anchor: PanelKind::Project, placement: Placement::Tab { before: None } },+ ] {+ for id in ["ui.dock", "window.panel.layout", "window.dockAction"] {+ let error = invoke(&mut app, id, json!({"action": action})).unwrap_err();+ assert!(error.contains("panel open commands"), "{id}: {error}");+ assert_eq!(app.ui.dock, before, "{id} must leave the layout unchanged");+ assert!(app.session.script_ui.window(77).is_none());+ assert!(!app.ui.viewers.contains_key(&42));+ }+ }+ }+ // The normal operation still opens a panel and the raw form still activates one.+ invoke(&mut app, "ui.dock", json!({"operation": "open", "panel": "Align"})).unwrap();+ assert!(app.ui.dock.is_open(PanelKind::Align));+ invoke(&mut app, "window.panel.layout", json!({"action": {"Activate": {"panel": "Project"}}})).unwrap();+ assert!(app.ui.dock.is_visible(PanelKind::Project));+}+ #[test] fn catalog_close_keeps_dynamic_panel_identity_and_releases_viewers() { let mut app = EffectcraftApp::new(Session::default());@@ -210,7 +210,9 @@ takes a name, or as `{"Viewer":42}` and `{"ScriptPanel":77}` for viewers and Scr `move {target, zone?, before?}`, `setStackOpen {open}`, `resizeStack {height}`, `resizeSplit {path, size}`; `rect` is `[x, y, width, height]`). It returns `{"docking": workspace}`. `ui.dock.reset` returns the current workspace to its saved or- built-in layout and returns the same.+ built-in layout and returns the same. Raw `Open` and `OpenAt` actions are refused in both+ `ui.dock {action}` and `window.panel.layout {action}`, as in `window.dockAction`; use the+ panel open commands instead. - `window.dockPanel {panel, anchor?, zone?, before?}` docks as `window.panel.dock`: `zone` is `center`, `tab`, `left`, `right`, `top`, `bottom`, `above` or `below`; without an anchor a side zone docks along that edge of the whole workspace. `window.floatPanel {panel, rect?}`