From 8c600087dc406149109f19b90250cc3e3b3ac819 Mon Sep 17 00:00:00 2001 From: "sorokin.oleg.edu" Date: Sun, 6 Sep 2026 06:51:59 +0300 Subject: [PATCH] Bind behavior scroll state to its document and owner --- crates/oa-desktop/src/behavior.rs | 179 ++++++++++++++++++++- harness/release-process-scroll-identity.md | 32 ++++ 2 files changed, 207 insertions(+), 4 deletions(-) create mode 100644 harness/release-process-scroll-identity.md diff --git a/crates/oa-desktop/src/behavior.rs b/crates/oa-desktop/src/behavior.rs index d0078041..b122640b 100644 --- a/crates/oa-desktop/src/behavior.rs +++ b/crates/oa-desktop/src/behavior.rs @@ -65,6 +65,10 @@ mod scenario_scroll { process: bool, } type Report = Option<(bool, Id, Id, Vector)>; + struct ObservedScroll { + identity: (Id, Id), + report: Report, + } #[derive(Default)] struct Translation(Option); impl widget::Operation for Translation { @@ -102,7 +106,7 @@ mod scenario_scroll { self.context.scenario, offset, )); - let previous = tree.state.downcast_mut::(); + let previous = &mut tree.state.downcast_mut::().report; if *previous != next { *previous = next; shell.publish(if self.process { @@ -129,16 +133,28 @@ mod scenario_scroll { } impl Widget for Observer<'_, R> { fn tag(&self) -> widget::tree::Tag { - widget::tree::Tag::of::() + widget::tree::Tag::of::() } fn state(&self) -> widget::tree::State { - widget::tree::State::new(Report::None) + widget::tree::State::new(ObservedScroll { + identity: (self.context.document, self.context.scenario), + report: None, + }) } fn children(&self) -> Vec { vec![widget::Tree::new(self.content.as_widget())] } fn diff(&self, tree: &mut widget::Tree) { - tree.diff_children(std::slice::from_ref(&self.content)); + if tree.state.downcast_ref::().identity + != (self.context.document, self.context.scenario) + { + // A new owner/document needs fresh real scroll state, not merely + // cleared mirrored offsets. Edits and tool/form changes retain it. + tree.state = self.state(); + tree.children = self.children(); + } else { + tree.diff_children(std::slice::from_ref(&self.content)); + } } fn size(&self) -> Size { self.content.as_widget().size() @@ -259,6 +275,7 @@ mod scenario_scroll { area: scenario::ScrollArea::Canvas, process: false, content: scrollable(iced::widget::Space::new().width(200.).height(height)) + .id("observer-test") .width(200.) .height(200.) .into(), @@ -303,6 +320,160 @@ mod scenario_scroll { }) .collect() } + #[test] + fn independent_process_owner_change_resets_actual_scroll_offsets() { + let context = scenario::direct_tests::context(Id::new_v4()); + let redraw = || { + iced::Event::Window(iced::window::Event::RedrawRequested( + std::time::Instant::now(), + )) + }; + let process_offsets = |messages: Vec| { + messages + .into_iter() + .filter_map(|message| { + if let Message::TopologyCanvas(topology::CanvasEvent::Scrolled( + _, + _, + offset, + )) = message + { + Some(offset) + } else { + None + } + }) + .collect::>() + }; + let mut view = observer(600., context); + view.process = true; + let mut tree = widget::Tree::new(&view as &dyn Widget); + assert_eq!( + process_offsets(update(&mut view, &mut tree, redraw()).0), + vec![Vector::ZERO] + ); + let (messages, captured) = update( + &mut view, + &mut tree, + iced::Event::Mouse(mouse::Event::WheelScrolled { + delta: mouse::ScrollDelta::Pixels { x: 0., y: -180. }, + }), + ); + assert!(captured); + assert_eq!(process_offsets(messages), vec![Vector::new(0., 180.)]); + let mut other = context; + other.scenario = Id::new_v4(); + let mut state = topology::State::default(); + state.enter(context.document, context.scenario); + state.scrolled(scenario::ScrollArea::Canvas, Vector::new(0., 180.)); + state.enter(other.document, other.scenario); + assert_eq!(state.scroll_offset(), Vector::ZERO); + view = observer(600., other); + view.process = true; + view.diff(&mut tree); + let reported = process_offsets(update(&mut view, &mut tree, redraw()).0); + assert_eq!( + reported, + vec![Vector::ZERO], + "The new process immediately reacquires its predecessor's scroll offset" + ); + } + + #[test] + fn direct_scroll_mounted_identity_resets_navigation_but_preserves_edits_before_first_report() + { + use widget::operation::scrollable::{AbsoluteOffset, scroll_to}; + for process in [false, true] { + for area in [scenario::ScrollArea::Canvas, scenario::ScrollArea::Page] { + for change in 0..4 { + let context = scenario::direct_tests::context(Id::new_v4()); + let mut view = observer(600., context); + view.process = process; + view.area = area; + let mut tree = widget::Tree::new(&view as &dyn Widget); + let node = view.layout( + &mut tree, + &(), + &layout::Limits::new(Size::ZERO, Size::new(800., 700.)), + ); + // Model a programmatic reveal before the observer has reported. + // Mounted identity must not depend on its last scroll notification. + let mut operation = scroll_to::<()>( + widget::Id::new("observer-test"), + AbsoluteOffset { + x: None, + y: Some(180.), + }, + ); + view.operate(&mut tree, Layout::new(&node), &(), &mut operation); + let mut next = context; + match change { + 0 => next.scenario = Id::new_v4(), + 1 => next.document = Id::new_v4(), + 2 => { + next.revision += 1; + next.epoch += 5; + } + _ => {} + } + view = observer(if change == 3 { 800. } else { 600. }, next); + view.process = process; + view.area = area; + view.diff(&mut tree); + let redraw = || { + iced::Event::Window(iced::window::Event::RedrawRequested( + std::time::Instant::now(), + )) + }; + let (messages, _) = update(&mut view, &mut tree, redraw()); + let actual = messages + .into_iter() + .filter_map(|message| match message { + Message::TopologyCanvas(topology::CanvasEvent::Scrolled( + c, + reported_area, + offset, + )) => { + assert_eq!( + (c.document, c.owner, c.revision, c.epoch), + (next.document, next.scenario, next.revision, next.epoch) + ); + assert_eq!( + std::mem::discriminant(&reported_area), + std::mem::discriminant(&area) + ); + Some(offset) + } + Message::ScenarioCanvas(scenario::CanvasEvent::Scrolled( + c, + reported_area, + offset, + )) => { + assert_eq!(c, next); + assert_eq!( + std::mem::discriminant(&reported_area), + std::mem::discriminant(&area) + ); + Some(offset) + } + _ => None, + }) + .collect::>(); + assert_eq!( + actual, + vec![if change < 2 { + Vector::ZERO + } else { + Vector::new(0., 180.) + }], + "process={process} area={area:?} change={change}" + ); + assert!(update(&mut view, &mut tree, redraw()).0.is_empty()); + } + } + } + } + #[test] fn direct_scroll_observer_reports_actual_initial_clamped_and_remounted_offsets_once() { let context = scenario::direct_tests::context(Id::new_v4()); diff --git a/harness/release-process-scroll-identity.md b/harness/release-process-scroll-identity.md new file mode 100644 index 00000000..90181946 --- /dev/null +++ b/harness/release-process-scroll-identity.md @@ -0,0 +1,32 @@ +# Behavior scroll identity correction + +Status: implementation and focused checks passed; full platform checks and native +acceptance are pending. Base `1b0b072` includes the independently reviewed process +camera/form corrections in `cf33785`. + +The independent actual-Iced probe reproduced a preexisting navigation defect: +`topology::State::enter` cleared its mirrored offset, but the retained Scrollable +still held y180 and immediately reported it under the next process owner. The +reviewer's test is now a repository regression, using Iced's null renderer, real +layout, wheel input and observer messages. It failed against the original code +with y180 instead of zero and passes with this correction. + +The existing observer now stores its mounted `(document, owner)` identity at +construction. Its diff recreates local child widget state only when that pair +changes. Keeping the same owner preserves the actual scrollable and its report; +revision, epoch, tool and form transitions are deliberately not identity keys. +No general state framework, model command, schema or persistence change is added. +The shared observer applies the same lifetime to inner-canvas and outer-page +scrolling in process and scenario views. + +A second actual-Iced regression seeds a programmatic scroll operation before any +observer report, then varies document, owner, revision/epoch and form-like content +height. This guards against deriving identity from the last notification. Its +matrix covers both message families and both scroll areas: only document/owner +navigation resets to zero, and unchanged-owner edits retain y180. It also verifies +current event context and one-time reporting. Both defect probes failed before the +fix; all three observer tests and 22 process tests now pass. Existing passive clamp, +remount, palette camera, form widget-state and stale-gesture tests remain passing. + +The process/native ledger, STATE and other lead-owned documents are untouched. +The lead owns independent re-review and actual native navigation acceptance.