diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a6b423f..1399617 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -38,6 +38,10 @@ jobs: GSK_RENDERER=cairo \ xvfb-run -a cargo test -p onenote-render-gtk \ scroll_adjustments_invalidate_the_viewport_snapshot --locked --offline + GTK_A11Y=none \ + GSK_RENDERER=cairo \ + xvfb-run -a cargo test -p onenote-render-gtk \ + attachment_ --locked --offline GTK_A11Y=none \ GSK_RENDERER=cairo \ xvfb-run -a cargo test -p onenote-viewer --locked --offline diff --git a/Cargo.lock b/Cargo.lock index fff30d4..0117007 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1780,12 +1780,14 @@ name = "onenote-viewer" version = "0.1.4" dependencies = [ "anyhow", + "blake3", "glib-build-tools", "gtk4", "onenote-core", "onenote-index", "onenote-render", "onenote-render-gtk", + "sanitize-filename", "serde", "serde_json", "tempfile", diff --git a/Cargo.toml b/Cargo.toml index 04d40db..5617964 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,6 +31,7 @@ onenote-render = { path = "crates/onenote-render" } onenote-render-gtk = { path = "crates/onenote-render-gtk" } onenote_parser = { git = "https://github.com/msiemens/onenote.rs", tag = "v2.0.0" } rusqlite = { version = "0.32.1", features = ["bundled"] } +sanitize-filename = "0.6.0" serde = { version = "1.0.219", features = ["derive", "rc"] } serde_json = "1.0.140" tempfile = "3.19.1" diff --git a/crates/onenote-core/src/error.rs b/crates/onenote-core/src/error.rs index 04c3f9f..a1d753b 100644 --- a/crates/onenote-core/src/error.rs +++ b/crates/onenote-core/src/error.rs @@ -45,7 +45,7 @@ pub enum Error { status: crate::ResourceStatus, }, - /// A resource exceeded the caller's explicit memory limit. + /// A resource exceeded the caller's explicit size limit. #[error("resource {id} is {declared_bytes} bytes, above the {limit_bytes}-byte limit")] ResourceTooLarge { /// Stable resource identifier. @@ -66,6 +66,36 @@ pub enum Error { source: std::io::Error, }, + /// Writing a lazily loaded resource failed. + #[error("could not write resource {id}: {source}")] + ResourceWrite { + /// Stable resource identifier. + id: crate::ResourceId, + /// Underlying output failure. + #[source] + source: std::io::Error, + }, + + /// Copying a lazy resource was cancelled by its caller. + #[error("copying resource {id} was cancelled")] + ResourceCopyCancelled { + /// Stable resource identifier. + id: crate::ResourceId, + }, + + /// The resource payload length disagreed with its declared length. + #[error( + "resource {id} contains {actual_bytes} bytes, but its source declares {declared_bytes} bytes" + )] + ResourceSizeMismatch { + /// Stable resource identifier. + id: crate::ResourceId, + /// Size recorded in the `OneNote` source. + declared_bytes: u64, + /// Number of bytes returned by the lazy reader. + actual_bytes: u64, + }, + /// No supported external CAB extractor is available. #[error("could not find 7zz or 7z in PATH; install 7-Zip to open .onepkg files")] ExtractorNotFound, diff --git a/crates/onenote-core/src/lib.rs b/crates/onenote-core/src/lib.rs index 7709617..3b2f3a5 100644 --- a/crates/onenote-core/src/lib.rs +++ b/crates/onenote-core/src/lib.rs @@ -21,10 +21,13 @@ pub use model::{ }; pub use package::{ExtractionPhase, ExtractionReport, OnePkgExtractor}; pub use parser::{LoadOptions, LoadedNotebook, OneNoteLoader, ParseLimits}; -pub use resource::ResourceStore; +pub use resource::{ + ResourceCopyControl, ResourceCopyOptions, ResourceCopyProgress, ResourceCopyReport, + ResourceStore, +}; /// The crate API version during the pre-1.0 implementation phase. -pub const API_VERSION: u32 = 5; +pub const API_VERSION: u32 = 6; /// Logical display pixels per `OneNote` half-inch layout unit at 96 DPI. pub const PIXELS_PER_HALF_INCH: f32 = 48.0; diff --git a/crates/onenote-core/src/resource.rs b/crates/onenote-core/src/resource.rs index 3a1e2e2..1f8014f 100644 --- a/crates/onenote-core/src/resource.rs +++ b/crates/onenote-core/src/resource.rs @@ -1,7 +1,66 @@ use crate::{Error, ResourceId, ResourceStatus, Result}; use onenote_parser::contents::{EmbeddedFile, FileDataStatus, Image}; use std::collections::HashMap; -use std::io::Read; +use std::io::{Read, Write}; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::Arc; + +const COPY_BUFFER_BYTES: usize = 64 * 1024; + +/// Limits applied while streaming a lazy binary resource. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub struct ResourceCopyOptions { + /// Maximum number of payload bytes accepted from the source. + pub limit_bytes: u64, +} + +impl ResourceCopyOptions { + /// Construct options with an explicit maximum payload size. + pub const fn new(limit_bytes: u64) -> Self { + Self { limit_bytes } + } +} + +/// Progress reported after a resource chunk has been written. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub struct ResourceCopyProgress { + /// Payload bytes written so far. + pub copied_bytes: u64, + /// Payload size declared by the source, when reliable. + pub declared_bytes: Option, +} + +/// Result of a successful streamed resource copy. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub struct ResourceCopyReport { + /// Total payload bytes written. + pub bytes_written: u64, +} + +/// Cloneable cooperative cancellation handle for resource copies. +#[derive(Clone, Debug, Default)] +pub struct ResourceCopyControl { + cancelled: Arc, +} + +impl ResourceCopyControl { + /// Create a cancellation handle in the active state. + pub fn new() -> Self { + Self::default() + } + + /// Request cancellation. A running copy observes this between bounded I/O + /// operations; callers must also cancel a blocked destination backend when + /// that backend supports cancellation. + pub fn cancel(&self) { + self.cancelled.store(true, Ordering::Release); + } + + /// Whether cancellation has been requested. + pub fn is_cancelled(&self) -> bool { + self.cancelled.load(Ordering::Acquire) + } +} #[derive(Clone, Debug)] pub(crate) enum ResourceLoader { @@ -31,6 +90,13 @@ impl ResourceLoader { Self::Attachment(file) => Some(file.read()), } } + + fn verified_size(&self) -> Option { + match self { + Self::Image(image) => image.size(), + Self::Attachment(file) => Some(file.size()), + } + } } /// Lazy binary resources retained by a parsed notebook. @@ -97,6 +163,43 @@ impl ResourceStore { /// Returns a not-found error, an unavailable-payload error, a size-limit /// error, or an underlying lazy resource read error. pub fn read_limited(&self, id: &ResourceId, limit_bytes: u64) -> Result> { + let declared = self.declared_size(id)?; + let capacity = usize::try_from(declared.min(limit_bytes)).unwrap_or(usize::MAX); + let mut bytes = Vec::with_capacity(capacity); + self.copy_to( + id, + &mut bytes, + ResourceCopyOptions::new(limit_bytes), + &ResourceCopyControl::new(), + |_| {}, + )?; + Ok(bytes) + } + + /// Stream a lazy resource into `writer` without materializing the payload. + /// + /// The copy performs blocking I/O and should run away from interactive UI + /// threads. Progress callbacks run on the calling thread after each + /// successfully written chunk. The caller owns destination publication and + /// must discard or roll back partial output after any error. + /// + /// # Errors + /// + /// Returns a typed error for missing or unavailable resources, size-limit + /// violations, cancellation, source or destination I/O failures, or a + /// mismatch between a reliable declared size and the streamed payload. + pub fn copy_to( + &self, + id: &ResourceId, + writer: &mut W, + options: ResourceCopyOptions, + control: &ResourceCopyControl, + progress: F, + ) -> Result + where + W: Write, + F: FnMut(ResourceCopyProgress), + { let loader = self .loaders .get(id) @@ -108,39 +211,105 @@ impl ResourceStore { status, }); } - let declared = loader.size(); - if declared > limit_bytes { + let declared = loader.verified_size(); + if declared.is_some_and(|size| size > options.limit_bytes) { return Err(Error::ResourceTooLarge { id: id.clone(), - declared_bytes: declared, - limit_bytes, + declared_bytes: declared.unwrap_or_default(), + limit_bytes: options.limit_bytes, }); } - - let capacity = usize::try_from(declared.min(limit_bytes)).unwrap_or(usize::MAX); - let mut bytes = Vec::with_capacity(capacity); + if control.is_cancelled() { + return Err(Error::ResourceCopyCancelled { id: id.clone() }); + } let Some(reader) = loader.reader() else { return Err(Error::ResourceUnavailable { id: id.clone(), status: ResourceStatus::Missing, }); }; - reader - .take(limit_bytes.saturating_add(1)) - .read_to_end(&mut bytes) + copy_reader(id, reader, writer, options, control, declared, progress) + } +} + +fn copy_reader( + id: &ResourceId, + mut reader: R, + writer: &mut W, + options: ResourceCopyOptions, + control: &ResourceCopyControl, + declared_bytes: Option, + mut progress: F, +) -> Result +where + R: Read, + W: Write, + F: FnMut(ResourceCopyProgress), +{ + if declared_bytes.is_some_and(|size| size > options.limit_bytes) { + return Err(Error::ResourceTooLarge { + id: id.clone(), + declared_bytes: declared_bytes.unwrap_or_default(), + limit_bytes: options.limit_bytes, + }); + } + if control.is_cancelled() { + return Err(Error::ResourceCopyCancelled { id: id.clone() }); + } + let mut buffer = vec![0_u8; COPY_BUFFER_BYTES].into_boxed_slice(); + let mut copied_bytes = 0_u64; + progress(ResourceCopyProgress { + copied_bytes, + declared_bytes, + }); + loop { + if control.is_cancelled() { + return Err(Error::ResourceCopyCancelled { id: id.clone() }); + } + let count = reader + .read(&mut buffer) .map_err(|source| Error::ResourceRead { id: id.clone(), source, })?; - if u64::try_from(bytes.len()).unwrap_or(u64::MAX) > limit_bytes { + if count == 0 { + break; + } + let next = copied_bytes.saturating_add(u64::try_from(count).unwrap_or(u64::MAX)); + if next > options.limit_bytes { return Err(Error::ResourceTooLarge { id: id.clone(), - declared_bytes: u64::try_from(bytes.len()).unwrap_or(u64::MAX), - limit_bytes, + declared_bytes: next, + limit_bytes: options.limit_bytes, + }); + } + if let Err(source) = writer.write_all(&buffer[..count]) { + if control.is_cancelled() { + return Err(Error::ResourceCopyCancelled { id: id.clone() }); + } + return Err(Error::ResourceWrite { + id: id.clone(), + source, + }); + } + copied_bytes = next; + progress(ResourceCopyProgress { + copied_bytes, + declared_bytes, + }); + } + if let Some(declared_bytes) = declared_bytes { + if copied_bytes != declared_bytes { + return Err(Error::ResourceSizeMismatch { + id: id.clone(), + declared_bytes, + actual_bytes: copied_bytes, }); } - Ok(bytes) } + Ok(ResourceCopyReport { + bytes_written: copied_bytes, + }) } // The upstream non-exhaustive enum requires a conservative fallback. @@ -153,3 +322,238 @@ pub(crate) fn resource_status(status: FileDataStatus) -> ResourceStatus { _ => ResourceStatus::Invalid, } } + +#[cfg(test)] +mod tests { + use super::*; + use std::io::{self, Cursor}; + + fn id() -> ResourceId { + ResourceId::new("resource") + } + + #[test] + fn copy_reader_streams_exact_bytes_and_monotonic_progress() { + let payload = vec![0x5a; COPY_BUFFER_BYTES * 2 + 17]; + let mut output = Vec::new(); + let mut updates = Vec::new(); + let report = copy_reader( + &id(), + Cursor::new(&payload), + &mut output, + ResourceCopyOptions::new(payload.len() as u64), + &ResourceCopyControl::default(), + Some(payload.len() as u64), + |progress| updates.push(progress), + ) + .expect("copy"); + + assert_eq!(output, payload); + assert_eq!(report.bytes_written, payload.len() as u64); + assert_eq!(updates.first().expect("first").copied_bytes, 0); + assert_eq!( + updates.last().expect("last").copied_bytes, + report.bytes_written + ); + assert!(updates + .windows(2) + .all(|pair| pair[0].copied_bytes <= pair[1].copied_bytes)); + } + + #[test] + fn copy_reader_rejects_actual_limit_overrun() { + let mut output = Vec::new(); + let error = copy_reader( + &id(), + Cursor::new(vec![1_u8; 9]), + &mut output, + ResourceCopyOptions::new(8), + &ResourceCopyControl::default(), + None, + |_| {}, + ) + .expect_err("limit"); + assert!(matches!(error, Error::ResourceTooLarge { .. })); + } + + #[test] + fn copy_reader_detects_short_declared_payload() { + let mut output = Vec::new(); + let error = copy_reader( + &id(), + Cursor::new(vec![1_u8; 4]), + &mut output, + ResourceCopyOptions::new(8), + &ResourceCopyControl::default(), + Some(5), + |_| {}, + ) + .expect_err("mismatch"); + assert!(matches!( + error, + Error::ResourceSizeMismatch { + declared_bytes: 5, + actual_bytes: 4, + .. + } + )); + } + + #[test] + fn copy_reader_rejects_declared_limit_before_reading() { + struct PanicReader; + impl Read for PanicReader { + fn read(&mut self, _buffer: &mut [u8]) -> io::Result { + panic!("reader must not be opened") + } + } + let error = copy_reader( + &id(), + PanicReader, + &mut Vec::new(), + ResourceCopyOptions::new(4), + &ResourceCopyControl::default(), + Some(5), + |_| {}, + ) + .expect_err("declared limit"); + assert!(matches!(error, Error::ResourceTooLarge { .. })); + } + + #[test] + fn copy_reader_honors_pre_copy_cancellation() { + let control = ResourceCopyControl::default(); + control.cancel(); + let error = copy_reader( + &id(), + Cursor::new([1_u8]), + &mut Vec::new(), + ResourceCopyOptions::new(1), + &control, + Some(1), + |_| {}, + ) + .expect_err("cancelled"); + assert!(matches!(error, Error::ResourceCopyCancelled { .. })); + } + + #[test] + fn copy_reader_uses_a_fixed_bounded_buffer() { + struct RecordingReader { + remaining: usize, + largest_request: usize, + } + impl Read for RecordingReader { + fn read(&mut self, buffer: &mut [u8]) -> io::Result { + self.largest_request = self.largest_request.max(buffer.len()); + let count = self.remaining.min(buffer.len()); + buffer[..count].fill(1); + self.remaining -= count; + Ok(count) + } + } + let mut reader = RecordingReader { + remaining: COPY_BUFFER_BYTES * 3 + 1, + largest_request: 0, + }; + let expected = reader.remaining; + let report = copy_reader( + &id(), + &mut reader, + &mut io::sink(), + ResourceCopyOptions::new(expected as u64), + &ResourceCopyControl::default(), + Some(expected as u64), + |_| {}, + ) + .expect("copy"); + assert_eq!(report.bytes_written, expected as u64); + assert_eq!(reader.largest_request, COPY_BUFFER_BYTES); + } + + #[test] + fn copy_reader_preserves_reader_failures() { + struct FailingReader; + impl Read for FailingReader { + fn read(&mut self, _buffer: &mut [u8]) -> io::Result { + Err(io::Error::other("read failed")) + } + } + let error = copy_reader( + &id(), + FailingReader, + &mut Vec::new(), + ResourceCopyOptions::new(1), + &ResourceCopyControl::default(), + None, + |_| {}, + ) + .expect_err("reader failure"); + assert!(matches!(error, Error::ResourceRead { .. })); + } + + struct CancellingWriter<'a> { + control: &'a ResourceCopyControl, + bytes: usize, + } + + impl Write for CancellingWriter<'_> { + fn write(&mut self, buffer: &[u8]) -> io::Result { + self.bytes += buffer.len(); + self.control.cancel(); + Ok(buffer.len()) + } + + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } + } + + #[test] + fn copy_reader_observes_mid_copy_cancellation() { + let control = ResourceCopyControl::default(); + let mut writer = CancellingWriter { + control: &control, + bytes: 0, + }; + let error = copy_reader( + &id(), + Cursor::new(vec![1_u8; COPY_BUFFER_BYTES + 1]), + &mut writer, + ResourceCopyOptions::new(u64::MAX), + &control, + None, + |_| {}, + ) + .expect_err("cancelled"); + assert!(matches!(error, Error::ResourceCopyCancelled { .. })); + assert_eq!(writer.bytes, COPY_BUFFER_BYTES); + } + + struct FailingWriter; + + impl Write for FailingWriter { + fn write(&mut self, _buffer: &[u8]) -> io::Result { + Err(io::Error::other("write failed")) + } + + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } + } + + #[test] + fn copy_reader_preserves_writer_failures() { + let error = copy_reader( + &id(), + Cursor::new([1_u8]), + &mut FailingWriter, + ResourceCopyOptions::new(1), + &ResourceCopyControl::default(), + Some(1), + |_| {}, + ) + .expect_err("writer failure"); + assert!(matches!(error, Error::ResourceWrite { .. })); + } +} diff --git a/crates/onenote-render-gtk/src/canvas.rs b/crates/onenote-render-gtk/src/canvas.rs index 54d7a90..ab2c366 100644 --- a/crates/onenote-render-gtk/src/canvas.rs +++ b/crates/onenote-render-gtk/src/canvas.rs @@ -8,7 +8,7 @@ use gtk::graphene; use gtk::prelude::*; use gtk::subclass::prelude::*; use num_traits::ToPrimitive; -use onenote_core::{Color, MathSpan, Rect, ResourceId, ResourceStore, TextStyle}; +use onenote_core::{Color, MathSpan, Rect, ResourceId, ResourceStatus, ResourceStore, TextStyle}; use onenote_render::{ HitAction, MathLayoutBackend, PageScene, SceneNode, SceneNodeId, ScenePrimitive, }; @@ -35,6 +35,7 @@ mod imp { TypstMathBackend, CANVAS_MARGIN, DEFAULT_ZOOM, MAX_ZOOM, MIN_ZOOM, }; use gtk::prelude::*; + use gtk::subclass::prelude::ObjectSubclassIsExt; use gtk::subclass::prelude::*; pub struct PageCanvas { @@ -43,6 +44,8 @@ mod imp { pub(super) zoom: Cell, pub(super) default_text_color: RefCell, pub(super) action_handler: RefCell>, + pub(super) hovered_attachment: RefCell>, + pub(super) focused_attachment: Cell>, pub(super) text_layouts: RefCell>>, pub(super) resolved_layout: RefCell>, pub(super) layout_generation: Cell, @@ -68,6 +71,8 @@ mod imp { zoom: Cell::new(DEFAULT_ZOOM), default_text_color: RefCell::new(gdk::RGBA::BLACK), action_handler: RefCell::default(), + hovered_attachment: RefCell::default(), + focused_attachment: Cell::default(), text_layouts: RefCell::default(), resolved_layout: RefCell::default(), layout_generation: Cell::default(), @@ -141,13 +146,42 @@ mod imp { if let Some(object) = weak.upgrade() { object.set_cursor_from_name(None); object.set_tooltip_text(None); + object.set_hovered_attachment(None); } }); object.add_controller(motion); + let keys = gtk::EventControllerKey::new(); + let weak = object.downgrade(); + keys.connect_key_pressed(move |_, key, _, _| { + let Some(object) = weak.upgrade() else { + return glib::Propagation::Proceed; + }; + if matches!(key, gdk::Key::Return | gdk::Key::KP_Enter | gdk::Key::space) { + if object.activate_focused_attachment() { + return glib::Propagation::Stop; + } + } else if key == gdk::Key::Escape { + object.imp().focused_attachment.set(None); + object.queue_draw(); + } + glib::Propagation::Proceed + }); + object.add_controller(keys); } } impl WidgetImpl for PageCanvas { + fn focus(&self, direction: gtk::DirectionType) -> bool { + if matches!( + direction, + gtk::DirectionType::TabForward | gtk::DirectionType::TabBackward + ) && self.obj().move_attachment_focus(direction) + { + return true; + } + self.parent_focus(direction) + } + fn measure(&self, orientation: gtk::Orientation, _for_size: i32) -> (i32, i32, i32, i32) { let Some(scene) = self.scene.borrow().clone() else { return (1, 1, -1, -1); @@ -203,6 +237,8 @@ impl PageCanvas { pub fn set_scene(&self, scene: Option>) { let imp = self.imp(); *imp.scene.borrow_mut() = scene; + imp.hovered_attachment.borrow_mut().take(); + imp.focused_attachment.set(None); self.invalidate_text_geometry(); imp.textures.borrow_mut().clear(); imp.pending.borrow_mut().clear(); @@ -319,6 +355,10 @@ impl PageCanvas { fn activate_at(&self, x: f64, y: f64) { let action = self.action_at(x, y); + if let Some(HitAction::OpenAttachment(resource_id)) = &action { + self.focus_attachment(resource_id); + self.grab_focus(); + } if let (Some(action), Some(handler)) = (action, self.imp().action_handler.borrow().as_ref()) { handler(action); @@ -326,15 +366,144 @@ impl PageCanvas { } fn update_pointer_at(&self, x: f64, y: f64) { - if let Some(HitAction::OpenLink(target)) = self.action_at(x, y) { - self.set_cursor_from_name(Some("pointer")); - self.set_tooltip_text(Some(&target)); - } else { - self.set_cursor_from_name(None); - self.set_tooltip_text(None); + match self.action_at(x, y) { + Some(HitAction::OpenLink(target)) => { + self.set_cursor_from_name(Some("pointer")); + self.set_tooltip_text(Some(&target)); + self.set_hovered_attachment(None); + } + Some(HitAction::OpenAttachment(resource_id)) => { + self.set_cursor_from_name(Some("pointer")); + let name = self.attachment_name(&resource_id); + let tooltip = name.as_deref().map(text::glib_text); + self.set_tooltip_text(tooltip.as_deref()); + self.set_hovered_attachment(Some(resource_id)); + } + Some(HitAction::SelectObject(_)) | None => { + self.set_cursor_from_name(None); + self.set_tooltip_text(None); + self.set_hovered_attachment(None); + } + } + } + + fn set_hovered_attachment(&self, resource_id: Option) { + if *self.imp().hovered_attachment.borrow() == resource_id { + return; + } + *self.imp().hovered_attachment.borrow_mut() = resource_id; + self.queue_draw(); + } + + fn attachment_name(&self, resource_id: &ResourceId) -> Option { + self.scene()? + .nodes + .iter() + .find_map(|node| match &node.primitive { + ScenePrimitive::Attachment(attachment) + if attachment.resource.id == *resource_id => + { + Some(attachment.resource.name.clone()) + } + _ => None, + }) + } + + fn attachment_actions(&self) -> Vec<(ResourceId, HitAction, Rect)> { + let Some(scene) = self.scene() else { + return Vec::new(); + }; + let resolved = self.resolved_layout(&scene); + scene + .hit_regions + .iter() + .filter_map(|region| { + let HitAction::OpenAttachment(resource_id) = ®ion.action else { + return None; + }; + let node = scene.nodes.iter().find(|node| node.id == region.node_id)?; + Some(( + resource_id.clone(), + region.action.clone(), + resolved.hit_region_bounds(node, region), + )) + }) + .collect() + } + + fn focus_attachment(&self, resource_id: &ResourceId) { + if let Some(index) = self + .attachment_actions() + .iter() + .position(|(candidate, _, _)| candidate == resource_id) + { + self.imp().focused_attachment.set(Some(index)); + self.reveal_attachment(index); + self.queue_draw(); } } + fn move_attachment_focus(&self, direction: gtk::DirectionType) -> bool { + let targets = self.attachment_actions(); + if targets.is_empty() { + return false; + } + let backwards = direction == gtk::DirectionType::TabBackward; + let next = next_attachment_index( + self.imp().focused_attachment.get(), + targets.len(), + backwards, + ); + let Some(next) = next else { + self.imp().focused_attachment.set(None); + self.queue_draw(); + return false; + }; + if !self.has_focus() && !self.grab_focus() { + return false; + } + self.imp().focused_attachment.set(Some(next)); + self.reveal_attachment(next); + self.queue_draw(); + true + } + + fn activate_focused_attachment(&self) -> bool { + let Some(index) = self.imp().focused_attachment.get() else { + return false; + }; + let Some((_, action, _)) = self.attachment_actions().get(index).cloned() else { + self.imp().focused_attachment.set(None); + return false; + }; + let Some(handler) = self.imp().action_handler.borrow().as_ref().cloned() else { + return false; + }; + handler(action); + true + } + + fn reveal_attachment(&self, index: usize) { + let Some((_, _, bounds)) = self.attachment_actions().get(index).cloned() else { + return; + }; + let Some(scene) = self.scene() else { + return; + }; + let Some(root) = self + .ancestor(gtk::ScrolledWindow::static_type()) + .and_then(|widget| widget.downcast::().ok()) + else { + return; + }; + let resolved = self.resolved_layout(&scene); + let zoom = f64::from(self.zoom()); + let x = f64::from(bounds.x - resolved.bounds.x + CANVAS_MARGIN) * zoom; + let y = f64::from(bounds.y - resolved.bounds.y + CANVAS_MARGIN) * zoom; + reveal_adjustment(&root.hadjustment(), x, f64::from(bounds.width) * zoom); + reveal_adjustment(&root.vadjustment(), y, f64::from(bounds.height) * zoom); + } + fn action_at(&self, x: f64, y: f64) -> Option { let scene = self.scene()?; let resolved = self.resolved_layout(&scene); @@ -536,16 +705,16 @@ impl PageCanvas { } } ScenePrimitive::Attachment(attachment) => { - snapshot.append_color( - &gdk::RGBA::new(0.95, 0.96, 0.98, 1.0), - &graphene_rect(node_bounds), - ); - self.snapshot_label( - snapshot, - node_bounds, - &attachment.resource.name, - gdk::RGBA::new(0.12, 0.25, 0.45, 1.0), - ); + let hovered = self.imp().hovered_attachment.borrow().as_ref() + == Some(&attachment.resource.id); + let focused = self.has_focus() + && self + .imp() + .focused_attachment + .get() + .and_then(|index| self.attachment_actions().get(index).cloned()) + .is_some_and(|(resource_id, _, _)| resource_id == attachment.resource.id); + self.snapshot_attachment(snapshot, node_bounds, attachment, hovered, focused); } ScenePrimitive::Ink { strokes } => snapshot_ink(snapshot, node_bounds, strokes), ScenePrimitive::Line { @@ -568,6 +737,72 @@ impl PageCanvas { } } + fn snapshot_attachment( + &self, + snapshot: >k::Snapshot, + bounds: Rect, + attachment: &onenote_core::Attachment, + hovered: bool, + focused: bool, + ) { + let text_color = self.default_text_color(); + let dark_surface = perceived_lightness(text_color) > 0.55; + let background = if dark_surface { + if hovered { + gdk::RGBA::new(0.20, 0.21, 0.23, 1.0) + } else { + gdk::RGBA::new(0.14, 0.15, 0.17, 1.0) + } + } else if hovered { + gdk::RGBA::new(0.91, 0.93, 0.96, 1.0) + } else { + gdk::RGBA::new(0.96, 0.97, 0.98, 1.0) + }; + let border = if dark_surface { + gdk::RGBA::new(0.42, 0.44, 0.48, 1.0) + } else { + gdk::RGBA::new(0.65, 0.68, 0.73, 1.0) + }; + let accent = if attachment.resource.status == ResourceStatus::Available { + if dark_surface { + gdk::RGBA::new(0.55, 0.75, 0.98, 1.0) + } else { + gdk::RGBA::new(0.12, 0.36, 0.64, 1.0) + } + } else { + gdk::RGBA::new(0.78, 0.22, 0.24, 1.0) + }; + snapshot.append_color(&background, &graphene_rect(bounds)); + snapshot_attachment_border(snapshot, bounds, border, 1.0); + snapshot_file_icon(snapshot, bounds, accent); + + let label_bounds = Rect { + x: bounds.x + 32.0, + y: bounds.y, + width: (bounds.width - 32.0).max(1.0), + height: bounds.height, + }; + self.snapshot_label( + snapshot, + label_bounds, + &attachment.resource.name, + text_color, + ); + if focused { + snapshot_attachment_border( + snapshot, + Rect { + x: bounds.x + 2.0, + y: bounds.y + 2.0, + width: (bounds.width - 4.0).max(1.0), + height: (bounds.height - 4.0).max(1.0), + }, + accent, + 2.0, + ); + } + } + fn snapshot_label( &self, snapshot: >k::Snapshot, @@ -842,6 +1077,66 @@ fn snapshot_line( let _ignored = cairo.stroke(); } +fn snapshot_attachment_border( + snapshot: >k::Snapshot, + bounds: Rect, + color: gdk::RGBA, + width: f64, +) { + let cairo = snapshot.append_cairo(&graphene_rect(bounds)); + cairo.set_source_rgba( + f64::from(color.red()), + f64::from(color.green()), + f64::from(color.blue()), + f64::from(color.alpha()), + ); + cairo.set_line_width(width); + let inset = width / 2.0; + cairo.rectangle( + inset, + inset, + (f64::from(bounds.width) - width).max(0.0), + (f64::from(bounds.height) - width).max(0.0), + ); + let _ignored = cairo.stroke(); +} + +fn snapshot_file_icon(snapshot: >k::Snapshot, bounds: Rect, color: gdk::RGBA) { + let icon_size = bounds.height.min(24.0).min(bounds.width).max(1.0); + let scale = f64::from(icon_size / 24.0); + let cairo = snapshot.append_cairo(&graphene_rect(bounds)); + cairo.translate(4.0, f64::from((bounds.height - icon_size) / 2.0)); + cairo.scale(scale, scale); + cairo.set_source_rgba( + f64::from(color.red()), + f64::from(color.green()), + f64::from(color.blue()), + f64::from(color.alpha()), + ); + cairo.set_line_width(2.0); + cairo.set_line_cap(gtk::cairo::LineCap::Round); + cairo.set_line_join(gtk::cairo::LineJoin::Round); + // Lucide "file" icon geometry, rendered directly by the snapshot canvas. + cairo.move_to(14.0, 2.0); + cairo.line_to(6.0, 2.0); + cairo.curve_to(4.9, 2.0, 4.0, 2.9, 4.0, 4.0); + cairo.line_to(4.0, 20.0); + cairo.curve_to(4.0, 21.1, 4.9, 22.0, 6.0, 22.0); + cairo.line_to(18.0, 22.0); + cairo.curve_to(19.1, 22.0, 20.0, 21.1, 20.0, 20.0); + cairo.line_to(20.0, 8.0); + cairo.close_path(); + let _ignored = cairo.stroke(); + cairo.move_to(14.0, 2.0); + cairo.line_to(14.0, 8.0); + cairo.line_to(20.0, 8.0); + let _ignored = cairo.stroke(); +} + +fn perceived_lightness(color: gdk::RGBA) -> f32 { + color.red() * 0.2126 + color.green() * 0.7152 + color.blue() * 0.0722 +} + fn ink_bounds(strokes: &[onenote_core::InkStroke]) -> Option { strokes .iter() @@ -882,6 +1177,30 @@ fn contains(rect: Rect, x: f32, y: f32) -> bool { x >= rect.x && x <= rect.x + rect.width && y >= rect.y && y <= rect.y + rect.height } +fn next_attachment_index(current: Option, len: usize, backwards: bool) -> Option { + if len == 0 { + return None; + } + match current { + None => Some(if backwards { len - 1 } else { 0 }), + Some(0) if backwards => None, + Some(index) if !backwards && index + 1 >= len => None, + Some(index) if backwards => Some(index - 1), + Some(index) => Some(index + 1), + } +} + +fn reveal_adjustment(adjustment: >k::Adjustment, start: f64, extent: f64) { + let end = start + extent; + let visible_start = adjustment.value(); + let visible_end = visible_start + adjustment.page_size(); + if start < visible_start { + adjustment.set_value(start); + } else if end > visible_end { + adjustment.set_value(end - adjustment.page_size()); + } +} + fn to_pango_units(value: f32) -> i32 { finite_f32_to_i32(value * 1_024.0) } @@ -908,7 +1227,21 @@ fn f64_to_f32(value: f64) -> f32 { #[cfg(test)] mod tests { - use super::{normalize_zoom, DEFAULT_ZOOM, MAX_ZOOM, MIN_ZOOM}; + use super::{ + next_attachment_index, normalize_zoom, PageCanvas, DEFAULT_ZOOM, MAX_ZOOM, MIN_ZOOM, + }; + use gtk::prelude::*; + use gtk::subclass::prelude::ObjectSubclassIsExt; + use onenote_core::{ + Attachment, ObjectId, PageId, Rect, ResourceId, ResourceRef, ResourceStatus, + }; + use onenote_render::{ + AccessibilityRole, AccessibilitySemantics, HitAction, HitRegion, PageScene, SceneNode, + SceneNodeId, ScenePrimitive, + }; + use std::cell::RefCell; + use std::rc::Rc; + use std::sync::Arc; #[test] fn zoom_normalization_is_finite_and_bounded() { @@ -919,6 +1252,95 @@ mod tests { assert_zoom_eq(normalize_zoom(1.21), 1.21); } + #[test] + fn attachment_focus_enters_moves_and_leaves_at_boundaries() { + assert_eq!(next_attachment_index(None, 3, false), Some(0)); + assert_eq!(next_attachment_index(Some(0), 3, false), Some(1)); + assert_eq!(next_attachment_index(Some(2), 3, false), None); + assert_eq!(next_attachment_index(None, 3, true), Some(2)); + assert_eq!(next_attachment_index(Some(2), 3, true), Some(1)); + assert_eq!(next_attachment_index(Some(0), 3, true), None); + assert_eq!(next_attachment_index(None, 0, false), None); + } + + #[test] + fn attachment_pointer_and_click_dispatch_the_resource_action() { + if std::env::var_os("DISPLAY").is_none() && std::env::var_os("WAYLAND_DISPLAY").is_none() { + return; + } + if gtk::init().is_err() { + return; + } + let resource_id = ResourceId::new("attachment"); + let node_id = SceneNodeId("node".to_owned()); + let object_id = ObjectId::new("object"); + let bounds = Rect { + x: 0.0, + y: 0.0, + width: 180.0, + height: 44.0, + }; + let scene = PageScene { + page_id: PageId::new("page"), + bounds, + nodes: vec![SceneNode { + id: node_id.clone(), + source_object_id: object_id.clone(), + bounds, + flow_path: Vec::new(), + z_index: 0, + primitive: ScenePrimitive::Attachment(Attachment { + resource: ResourceRef { + id: resource_id.clone(), + name: "manual\0.pdf".to_owned(), + media_type: "application/pdf".to_owned(), + size: 12, + status: ResourceStatus::Available, + }, + width: None, + height: None, + }), + accessibility: AccessibilitySemantics { + role: AccessibilityRole::Attachment, + label: "manual.pdf".to_owned(), + description: None, + }, + }], + hit_regions: vec![HitRegion { + node_id, + source_object_id: object_id, + bounds, + action: HitAction::OpenAttachment(resource_id.clone()), + }], + diagnostics: Vec::new(), + }; + let canvas = PageCanvas::new(); + canvas.set_scene(Some(Arc::new(scene))); + for color in [gtk::gdk::RGBA::BLACK, gtk::gdk::RGBA::WHITE] { + canvas.set_default_text_color(&color); + let snapshot = gtk::Snapshot::new(); + canvas.snapshot_scene(&snapshot); + assert!(snapshot.to_node().is_some()); + } + let dispatched = Rc::new(RefCell::new(Vec::new())); + let dispatched_from_handler = Rc::clone(&dispatched); + canvas.set_action_handler(Some(move |action| { + dispatched_from_handler.borrow_mut().push(action); + })); + + canvas.update_pointer_at(40.0, 40.0); + assert_eq!(canvas.tooltip_text().as_deref(), Some("manual�.pdf")); + canvas.activate_at(40.0, 40.0); + + assert_eq!( + dispatched.borrow().as_slice(), + &[HitAction::OpenAttachment(resource_id)] + ); + assert_eq!(canvas.imp().focused_attachment.get(), Some(0)); + assert!(canvas.activate_focused_attachment()); + assert_eq!(dispatched.borrow().len(), 2); + } + fn assert_zoom_eq(actual: f32, expected: f32) { assert!((actual - expected).abs() <= f32::EPSILON); } diff --git a/crates/onenote-render/src/builder.rs b/crates/onenote-render/src/builder.rs index a9d1cdd..027f48a 100644 --- a/crates/onenote-render/src/builder.rs +++ b/crates/onenote-render/src/builder.rs @@ -570,27 +570,9 @@ impl BuildState<'_> { z_index: i32, flow_path: &[SceneFlowPosition], ) -> Result<()> { - if attachment.resource.status != ResourceStatus::Available { - let name = bounded_label(&attachment.resource.name, 128); - let label = if attachment.resource.status == ResourceStatus::Missing { - format!("Attachment data missing: {name}") - } else { - format!("Broken attachment: {name}") - }; - return self.unavailable_resource( - object, - bounds, - z_index, - UnavailableResource { - label, - role: AccessibilityRole::Attachment, - status: attachment.resource.status, - }, - flow_path, - ); - } let resource_id = attachment.resource.id.clone(); let label = attachment.resource.name.clone(); + let status = attachment.resource.status; let node_id = self.push_node_in_flow( object, bounds, @@ -599,7 +581,11 @@ impl BuildState<'_> { AccessibilitySemantics { role: AccessibilityRole::Attachment, label, - description: Some("Embedded file".to_owned()), + description: Some(if status == ResourceStatus::Available { + "Embedded file".to_owned() + } else { + format!("Embedded file data is {status:?}") + }), }, flow_path, )?; @@ -609,6 +595,13 @@ impl BuildState<'_> { bounds, action: HitAction::OpenAttachment(resource_id), }); + if status != ResourceStatus::Available { + self.diagnostics.push(SceneDiagnostic { + code: "resource_unavailable".to_owned(), + message: format!("A referenced attachment is unavailable ({status:?})"), + object_id: Some(object.id.clone()), + }); + } Ok(()) } @@ -1081,7 +1074,7 @@ fn scene_bounds(page: &Page, nodes: &[SceneNode], options: SceneOptions) -> Rect #[cfg(test)] mod tests { use super::{estimate_text_height, format_list_number, ListState, SceneBuilder, SceneOptions}; - use crate::{AccessibilityRole, Error, ScenePrimitive}; + use crate::{AccessibilityRole, Error, HitAction, ScenePrimitive}; use onenote_core::{ Attachment, ElementContent, Image, ListMarker, ListMarkerPart, ListNumberFormat, ObjectId, ObjectKind, Outline, OutlineElement, Page, PageId, PageObject, PageObjectRole, Rect, @@ -1348,7 +1341,7 @@ mod tests { } #[test] - fn unavailable_resources_render_as_inert_labeled_placeholders() { + fn unavailable_attachments_remain_actionable_with_diagnostics() { let unavailable = |name: &str, status| ResourceRef { id: ResourceId::new(format!("resource-{name}")), name: name.to_owned(), @@ -1417,11 +1410,12 @@ mod tests { }) .collect(); - assert_eq!( - labels, - vec!["Broken image", "Attachment data missing: report.pdf"] - ); - assert!(scene.hit_regions.is_empty()); + assert_eq!(labels, vec!["Broken image"]); + assert_eq!(scene.hit_regions.len(), 1); + assert!(matches!( + scene.hit_regions[0].action, + HitAction::OpenAttachment(_) + )); assert_eq!(scene.diagnostics.len(), 2); assert!(scene .diagnostics diff --git a/crates/onenote-viewer/Cargo.toml b/crates/onenote-viewer/Cargo.toml index 733d1b8..f76aba1 100644 --- a/crates/onenote-viewer/Cargo.toml +++ b/crates/onenote-viewer/Cargo.toml @@ -10,11 +10,13 @@ publish = false [dependencies] anyhow.workspace = true +blake3.workspace = true gtk.workspace = true onenote-core.workspace = true onenote-index.workspace = true onenote-render.workspace = true onenote-render-gtk.workspace = true +sanitize-filename.workspace = true serde.workspace = true serde_json.workspace = true diff --git a/crates/onenote-viewer/src/app.rs b/crates/onenote-viewer/src/app.rs index ce629c0..8eca78b 100644 --- a/crates/onenote-viewer/src/app.rs +++ b/crates/onenote-viewer/src/app.rs @@ -8,10 +8,11 @@ use gtk::gio; use gtk::glib; use gtk::prelude::*; use onenote_core::{ - ExtractionPhase, LoadOptions, LoadedNotebook, ObjectId, Page, PageId, Rect, SectionId, SourceId, + ExtractionPhase, LoadOptions, LoadedNotebook, ObjectId, Page, PageId, Rect, ResourceId, + ResourceRef, SectionId, SourceId, }; use onenote_index::SearchHit; -use onenote_render::HitAction; +use onenote_render::{HitAction, ScenePrimitive}; use onenote_render_gtk::{PageView, DEFAULT_ZOOM}; use std::borrow::Cow; use std::cell::{Cell, RefCell}; @@ -52,6 +53,7 @@ const SYMBOLIC_ICON_NAMES: [&str; 19] = [ pub(crate) fn run(requested_sources: Vec) -> Result<()> { register_resources()?; + std::thread::spawn(crate::attachment::prune_cache); let (workspace_path, index_path) = workspace::paths()?; workspace::ensure_index_parent(&index_path)?; let persisted = workspace::load(&workspace_path).unwrap_or_default(); @@ -101,6 +103,7 @@ pub(crate) fn run(requested_sources: Vec) -> Result<()> { let status = application.run_with_args::<&str>(&[]); if let Some(viewer) = viewer.borrow().as_ref() { + viewer.cancel_foreground_operation_for_shutdown(Duration::from_secs(5)); let settings_result = viewer.flush_settings(); let _ignored = viewer.commands.send(Command::Shutdown); settings_result?; @@ -256,6 +259,42 @@ struct RevealTarget { bounds: Rect, } +#[derive(Clone)] +struct AttachmentContext { + resource: ResourceRef, + loaded: Arc, + source_id: SourceId, + fingerprint: onenote_core::SourceFingerprint, +} + +enum ForegroundOperationKind { + PackageImport(Arc), + Attachment(crate::attachment::CopyCancellation), +} + +struct ForegroundOperation { + id: u64, + kind: ForegroundOperationKind, +} + +impl ForegroundOperation { + fn cancel(&self) { + match &self.kind { + ForegroundOperationKind::PackageImport(cancel) => { + cancel.store(true, Ordering::Release); + } + ForegroundOperationKind::Attachment(cancel) => cancel.cancel(), + } + } + + fn is_cancelled(&self) -> bool { + match &self.kind { + ForegroundOperationKind::PackageImport(cancel) => cancel.load(Ordering::Acquire), + ForegroundOperationKind::Attachment(cancel) => cancel.is_cancelled(), + } + } +} + #[derive(Default)] struct State { sources: Vec, @@ -286,13 +325,15 @@ struct Viewer { status: gtk::Label, spinner: gtk::Spinner, zoom_label: gtk::Label, - import_activity: gtk::Revealer, - import_activity_title: gtk::Label, - import_activity_phase: gtk::Label, - import_progress: gtk::ProgressBar, - import_cancel_button: gtk::Button, + operation_activity: gtk::Revealer, + operation_activity_title: gtk::Label, + operation_activity_phase: gtk::Label, + operation_progress: gtk::ProgressBar, + operation_cancel_button: gtk::Button, import_package_action: gio::SimpleAction, - import_cancel: RefCell>>, + foreground_operation: RefCell>, + next_operation_id: Cell, + operation_pulsing: Cell, scene_cancel: RefCell>>, selection_syncing: Cell, state: RefCell, @@ -477,37 +518,37 @@ impl Viewer { footer.append(&zoom_in); footer.append(&zoom_reset); - let import_activity_title = gtk::Label::builder() + let operation_activity_title = gtk::Label::builder() .xalign(0.0) .hexpand(true) .ellipsize(gtk::pango::EllipsizeMode::End) .build(); - import_activity_title.add_css_class("activity-title"); - let import_activity_phase = gtk::Label::builder() + operation_activity_title.add_css_class("activity-title"); + let operation_activity_phase = gtk::Label::builder() .xalign(0.0) .ellipsize(gtk::pango::EllipsizeMode::End) .build(); - import_activity_phase.add_css_class("activity-phase"); + operation_activity_phase.add_css_class("activity-phase"); let activity_labels = gtk::Box::new(gtk::Orientation::Vertical, 2); activity_labels.set_hexpand(true); - activity_labels.append(&import_activity_title); - activity_labels.append(&import_activity_phase); - let import_progress = gtk::ProgressBar::builder() + activity_labels.append(&operation_activity_title); + activity_labels.append(&operation_activity_phase); + let operation_progress = gtk::ProgressBar::builder() .width_request(240) .valign(gtk::Align::Center) .build(); - import_progress.set_pulse_step(0.025); - let import_cancel_button = gtk::Button::with_label("Cancel"); + operation_progress.set_pulse_step(0.025); + let operation_cancel_button = gtk::Button::with_label("Cancel"); let activity_content = gtk::Box::new(gtk::Orientation::Horizontal, 12); activity_content.set_margin_start(16); activity_content.set_margin_end(16); activity_content.set_margin_top(10); activity_content.set_margin_bottom(10); activity_content.append(&activity_labels); - activity_content.append(&import_progress); - activity_content.append(&import_cancel_button); - activity_content.add_css_class("import-activity"); - let import_activity = gtk::Revealer::builder() + activity_content.append(&operation_progress); + activity_content.append(&operation_cancel_button); + activity_content.add_css_class("operation-activity"); + let operation_activity = gtk::Revealer::builder() .transition_type(gtk::RevealerTransitionType::SlideDown) .child(&activity_content) .build(); @@ -541,7 +582,7 @@ impl Viewer { ); let root = gtk::Box::new(gtk::Orientation::Vertical, 0); - root.append(&import_activity); + root.append(&operation_activity); root.append(&content_paned); root.append(&separator_horizontal()); root.append(&footer); @@ -576,13 +617,15 @@ impl Viewer { status, spinner, zoom_label, - import_activity, - import_activity_title, - import_activity_phase, - import_progress, - import_cancel_button, + operation_activity, + operation_activity_title, + operation_activity_phase, + operation_progress, + operation_cancel_button, import_package_action: import_package.clone(), - import_cancel: RefCell::default(), + foreground_operation: RefCell::default(), + next_operation_id: Cell::new(1), + operation_pulsing: Cell::new(false), scene_cancel: RefCell::default(), selection_syncing: Cell::new(false), state: RefCell::default(), @@ -619,7 +662,7 @@ impl Viewer { ); viewer.connect_about(&show_about); viewer.connect_system_theme(); - viewer.connect_import_activity(); + viewer.connect_operation_activity(); viewer.connect_zoom(&zoom_out, &zoom_in, &zoom_reset); viewer.connect_page_actions(); viewer.poll_events(); @@ -749,20 +792,21 @@ impl Viewer { }); } - fn connect_import_activity(self: &Rc) { + fn connect_operation_activity(self: &Rc) { let weak = Rc::downgrade(self); - self.import_cancel_button.connect_clicked(move |button| { + self.operation_cancel_button.connect_clicked(move |button| { let Some(viewer) = weak.upgrade() else { return; }; - let Some(cancel) = viewer.import_cancel.borrow().as_ref().cloned() else { + let operation = viewer.foreground_operation.borrow(); + let Some(operation) = operation.as_ref() else { return; }; - cancel.store(true, Ordering::Relaxed); + operation.cancel(); button.set_sensitive(false); viewer - .import_activity_phase - .set_label("Cancelling and cleaning temporary files..."); + .operation_activity_phase + .set_label("Cancelling and cleaning temporary output..."); }); } @@ -810,14 +854,174 @@ impl Viewer { fn handle_page_action(self: &Rc, action: HitAction) { match action { HitAction::OpenLink(target) => self.open_link(&target), - HitAction::OpenAttachment(_) => { - self.status - .set_label("Opening attachments is not implemented yet"); - } + HitAction::OpenAttachment(resource_id) => self.show_attachment(&resource_id), HitAction::SelectObject(_) => {} } } + fn show_attachment(self: &Rc, resource_id: &ResourceId) { + let Some(context) = self.attachment_context(resource_id) else { + self.show_error( + "Could not access attachment", + "The selected attachment no longer belongs to the displayed page.", + ); + return; + }; + let open_context = context.clone(); + let save_context = context.clone(); + let weak_open = Rc::downgrade(self); + let weak_save = weak_open.clone(); + crate::dialogs::present_attachment( + &self.window, + &context.resource, + move || { + if let Some(viewer) = weak_open.upgrade() { + viewer.open_attachment(open_context.clone()); + } + }, + move || { + if let Some(viewer) = weak_save.upgrade() { + viewer.choose_attachment_destination(save_context.clone()); + } + }, + ); + } + + fn attachment_context(&self, resource_id: &ResourceId) -> Option { + let scene = self.page_view.canvas().scene()?; + let resource = scene.nodes.iter().find_map(|node| match &node.primitive { + ScenePrimitive::Attachment(attachment) if attachment.resource.id == *resource_id => { + Some(attachment.resource.clone()) + } + _ => None, + })?; + let active_source = self.state.borrow().active.as_ref()?.source.clone(); + let state = self.state.borrow(); + let source = state + .sources + .iter() + .find(|source| source.loaded.notebook.source_id == active_source)?; + Some(AttachmentContext { + resource, + loaded: source.loaded.clone(), + source_id: source.loaded.notebook.source_id.clone(), + fingerprint: source.loaded.notebook.fingerprint.clone(), + }) + } + + fn choose_attachment_destination(self: &Rc, context: AttachmentContext) { + let initial_name = crate::attachment::sanitized_filename(&context.resource.name); + let dialog = gtk::FileDialog::builder() + .title("Save Attachment") + .accept_label("Save") + .initial_name(initial_name) + .modal(true) + .build(); + let weak = Rc::downgrade(self); + dialog.save( + Some(&self.window), + None::<&gio::Cancellable>, + move |result| match result { + Ok(destination) => { + if let Some(viewer) = weak.upgrade() { + viewer.start_attachment_copy( + context, + destination, + worker::AttachmentPurpose::Save, + ); + } + } + Err(error) if error.matches(gtk::DialogError::Dismissed) => {} + Err(error) => { + if let Some(viewer) = weak.upgrade() { + viewer.show_error( + "Could not choose attachment destination", + &error.to_string(), + ); + } + } + }, + ); + } + + fn open_attachment(self: &Rc, context: AttachmentContext) { + let destination = match crate::attachment::cache_file( + &context.source_id, + &context.fingerprint, + &context.resource.id, + &context.resource.name, + ) { + Ok(destination) => destination, + Err(error) => { + self.show_error("Could not prepare attachment cache", &error.to_string()); + return; + } + }; + self.start_attachment_copy(context, destination, worker::AttachmentPurpose::Open); + } + + fn start_attachment_copy( + self: &Rc, + context: AttachmentContext, + destination: gio::File, + purpose: worker::AttachmentPurpose, + ) { + if self.foreground_operation.borrow().is_some() { + self.status + .set_label("Another file operation is already running"); + return; + } + let operation_id = self.allocate_operation_id(); + let cancellation = crate::attachment::CopyCancellation::new(); + *self.foreground_operation.borrow_mut() = Some(ForegroundOperation { + id: operation_id, + kind: ForegroundOperationKind::Attachment(cancellation.clone()), + }); + self.operation_pulsing.set(context.resource.size == 0); + self.import_package_action.set_enabled(false); + self.operation_cancel_button.set_sensitive(true); + self.operation_progress.set_fraction(0.0); + self.operation_activity_title.set_label(&format!( + "{} {}", + if purpose == worker::AttachmentPurpose::Open { + "Preparing" + } else { + "Saving" + }, + gtk_text(&context.resource.name) + )); + self.operation_activity_phase + .set_label("Copying attachment safely..."); + self.operation_activity.set_reveal_child(true); + self.set_busy("Copying attachment"); + worker::copy_attachment( + operation_id, + purpose, + crate::attachment::CopyRequest { + loaded: context.loaded, + resource_id: context.resource.id, + destination, + cancellation, + }, + self.events.clone(), + ); + } + + fn launch_attachment(self: &Rc, file: &gio::File) { + let launcher = gtk::FileLauncher::new(Some(file)); + launcher.set_writable(false); + let weak = Rc::downgrade(self); + launcher.launch( + Some(&self.window), + None::<&gio::Cancellable>, + move |result| { + if let (Err(error), Some(viewer)) = (result, weak.upgrade()) { + viewer.show_error("Could not open attachment", &error.to_string()); + } + }, + ); + } + fn open_link(self: &Rc, target: &str) { let target = target.trim(); if target.is_empty() || target.contains('\0') { @@ -995,8 +1199,11 @@ impl Viewer { for event in events { viewer.handle_event(event); } - if viewer.import_activity.reveals_child() && viewer.import_cancel.borrow().is_some() { - viewer.import_progress.pulse(); + if viewer.operation_activity.reveals_child() + && viewer.foreground_operation.borrow().is_some() + && viewer.operation_pulsing.get() + { + viewer.operation_progress.pulse(); } glib::ControlFlow::Continue }); @@ -1075,33 +1282,122 @@ impl Viewer { Err(error) => self.show_error("Could not render page", &error), } } - Event::Extracted { result } => match result { - Ok(destination) => { - self.status.set_label("Package imported; opening notebooks"); - self.import_cancel.borrow_mut().take(); - self.import_cancel_button.set_sensitive(false); - self.import_progress.set_fraction(1.0); - self.import_activity_phase - .set_label("Imported successfully; opening notebooks..."); - self.hide_import_activity_after(Duration::from_millis(1_800)); - self.discover(destination); - } - Err(error) => { - self.finish_import_activity(); - if error == "OneNote package extraction was cancelled" { - self.status.set_label("Package import cancelled"); - } else { - self.show_error("Package import failed", &error); - } + Event::Extracted { + operation_id, + result, + } => self.handle_extracted(operation_id, result), + Event::ExtractionProgress { + operation_id, + phase, + } => self.handle_extraction_progress(operation_id, phase), + Event::AttachmentProgress { + operation_id, + copied_bytes, + declared_bytes, + } => self.handle_attachment_progress(operation_id, copied_bytes, declared_bytes), + Event::AttachmentCopied { + operation_id, + purpose, + destination, + result, + } => self.handle_attachment_copied(operation_id, purpose, &destination, result), + } + } + + fn handle_extracted( + self: &Rc, + operation_id: u64, + result: std::result::Result, + ) { + if !self.operation_is_active(operation_id) { + return; + } + match result { + Ok(destination) => { + self.status.set_label("Package imported; opening notebooks"); + self.operation_cancel_button.set_sensitive(false); + self.operation_progress.set_fraction(1.0); + self.operation_activity_phase + .set_label("Imported successfully; opening notebooks..."); + self.hide_operation_activity_after(operation_id, Duration::from_millis(1_800)); + self.discover(destination); + } + Err(error) => { + let cancelled = self.operation_was_cancelled(operation_id); + self.finish_operation(operation_id); + if cancelled || error == "OneNote package extraction was cancelled" { + self.status.set_label("Package import cancelled"); + } else { + self.show_error("Package import failed", &error); } - }, - Event::ExtractionProgress { phase } => { - self.import_activity_phase - .set_label(extraction_phase_label(phase)); } } } + fn handle_extraction_progress(&self, operation_id: u64, phase: ExtractionPhase) { + if self.operation_is_active(operation_id) { + self.operation_activity_phase + .set_label(extraction_phase_label(phase)); + } + } + + fn handle_attachment_progress( + &self, + operation_id: u64, + copied_bytes: u64, + declared_bytes: Option, + ) { + if !self.operation_is_active(operation_id) { + return; + } + if let Some(total) = declared_bytes.filter(|total| *total > 0) { + self.operation_pulsing.set(false); + self.operation_progress + .set_fraction(attachment_progress_fraction(copied_bytes, total)); + self.operation_activity_phase.set_label(&format!( + "Copied {} of {}", + crate::attachment::format_size(copied_bytes), + crate::attachment::format_size(total) + )); + } else { + self.operation_pulsing.set(true); + self.operation_activity_phase.set_label(&format!( + "Copied {}", + crate::attachment::format_size(copied_bytes) + )); + } + } + + fn handle_attachment_copied( + self: &Rc, + operation_id: u64, + purpose: worker::AttachmentPurpose, + destination: &gio::File, + result: std::result::Result, + ) { + if !self.operation_is_active(operation_id) { + return; + } + let cancelled = self.operation_was_cancelled(operation_id); + self.finish_operation(operation_id); + match result { + Ok(bytes) if purpose == worker::AttachmentPurpose::Open => { + self.status.set_label(&format!( + "Attachment ready ({})", + crate::attachment::format_size(bytes) + )); + self.launch_attachment(destination); + } + Ok(bytes) => self.status.set_label(&format!( + "Attachment saved to {} ({})", + destination.parse_name(), + crate::attachment::format_size(bytes) + )), + Err(_) if cancelled => self.status.set_label("Attachment copy cancelled"), + Err(error) => self.show_error("Could not copy attachment", &error), + } + } + fn reveal_pending_target(&self) { let Some(target) = self.state.borrow_mut().pending_reveal.take() else { return; @@ -1652,9 +1948,9 @@ impl Viewer { } fn choose_package(self: &Rc) { - if self.import_cancel.borrow().is_some() { + if self.foreground_operation.borrow().is_some() { self.status - .set_label("A OneNote package import is already running"); + .set_label("Another file operation is already running"); return; } let filter = gtk::FileFilter::new(); @@ -1796,42 +2092,112 @@ impl Viewer { } fn start_package_import(&self, package: PathBuf, destination: PathBuf) { - if self.import_cancel.borrow().is_some() { + if self.foreground_operation.borrow().is_some() { self.status - .set_label("A OneNote package import is already running"); + .set_label("Another file operation is already running"); return; } + let operation_id = self.allocate_operation_id(); let cancel = Arc::new(AtomicBool::new(false)); - *self.import_cancel.borrow_mut() = Some(Arc::clone(&cancel)); + *self.foreground_operation.borrow_mut() = Some(ForegroundOperation { + id: operation_id, + kind: ForegroundOperationKind::PackageImport(Arc::clone(&cancel)), + }); + self.operation_pulsing.set(true); self.import_package_action.set_enabled(false); - self.import_cancel_button.set_sensitive(true); - self.import_progress.set_fraction(0.0); - self.import_activity_title.set_label(&format!( + self.operation_cancel_button.set_sensitive(true); + self.operation_progress.set_fraction(0.0); + self.operation_activity_title.set_label(&format!( "Importing {}", package .file_name() .and_then(|value| value.to_str()) .unwrap_or("OneNote package") )); - self.import_activity_phase + self.operation_activity_phase .set_label("Preparing package import..."); - self.import_activity.set_reveal_child(true); + self.operation_activity.set_reveal_child(true); self.set_busy("Importing OneNote package"); - worker::extract(package, destination, cancel, self.events.clone()); + worker::extract( + operation_id, + package, + destination, + cancel, + self.events.clone(), + ); + } + + fn allocate_operation_id(&self) -> u64 { + let id = self.next_operation_id.get(); + self.next_operation_id.set(id.wrapping_add(1).max(1)); + id + } + + fn operation_is_active(&self, operation_id: u64) -> bool { + self.foreground_operation + .borrow() + .as_ref() + .is_some_and(|operation| operation.id == operation_id) } - fn finish_import_activity(&self) { - self.import_cancel.borrow_mut().take(); - self.import_cancel_button.set_sensitive(false); + fn operation_was_cancelled(&self, operation_id: u64) -> bool { + self.foreground_operation + .borrow() + .as_ref() + .is_some_and(|operation| operation.id == operation_id && operation.is_cancelled()) + } + + fn finish_operation(&self, operation_id: u64) { + let mut operation = self.foreground_operation.borrow_mut(); + if operation + .as_ref() + .is_none_or(|operation| operation.id != operation_id) + { + return; + } + operation.take(); + self.operation_pulsing.set(false); + self.operation_cancel_button.set_sensitive(false); self.import_package_action.set_enabled(true); - self.import_activity.set_reveal_child(false); + self.operation_activity.set_reveal_child(false); + } + + fn cancel_foreground_operation_for_shutdown(&self, timeout: Duration) { + let operation_id = { + let operation = self.foreground_operation.borrow(); + let Some(operation) = operation.as_ref() else { + return; + }; + operation.cancel(); + operation.id + }; + let deadline = std::time::Instant::now() + timeout; + while std::time::Instant::now() < deadline { + let remaining = deadline.saturating_duration_since(std::time::Instant::now()); + let wait = remaining.min(Duration::from_millis(100)); + match self.receiver.borrow().recv_timeout(wait) { + Ok( + Event::Extracted { + operation_id: completed, + .. + } + | Event::AttachmentCopied { + operation_id: completed, + .. + }, + ) if completed == operation_id => break, + Ok(_) | Err(mpsc::RecvTimeoutError::Timeout) => {} + Err(mpsc::RecvTimeoutError::Disconnected) => break, + } + } + self.finish_operation(operation_id); } - fn hide_import_activity_after(self: &Rc, delay: Duration) { + fn hide_operation_activity_after(self: &Rc, operation_id: u64, delay: Duration) { let weak = Rc::downgrade(self); glib::timeout_add_local_once(delay, move || { if let Some(viewer) = weak.upgrade() { - viewer.finish_import_activity(); + viewer.finish_operation(operation_id); } }); } @@ -1970,6 +2336,14 @@ fn extraction_phase_label(phase: ExtractionPhase) -> &'static str { } } +fn attachment_progress_fraction(copied_bytes: u64, total_bytes: u64) -> f64 { + let total = u32::try_from(total_bytes.min(crate::attachment::MAX_ATTACHMENT_BYTES)) + .unwrap_or(u32::MAX) + .max(1); + let copied = u32::try_from(copied_bytes.min(u64::from(total))).unwrap_or(total); + f64::from(copied) / f64::from(total) +} + fn list_view(model: >k::StringList, css_class: &str) -> (gtk::SingleSelection, gtk::ListView) { let selection = gtk::SingleSelection::new(Some(model.clone())); selection.set_autoselect(false); @@ -2551,7 +2925,7 @@ fn theme_css(theme: EffectiveTheme) -> String { color: @text; }} .empty-icon, .empty-icon:backdrop {{ color: @muted; }} - .import-activity, .import-activity:backdrop {{ + .operation-activity, .operation-activity:backdrop {{ background: @activity_bg; color: @text; border-bottom: 1px solid @activity_border; diff --git a/crates/onenote-viewer/src/attachment.rs b/crates/onenote-viewer/src/attachment.rs new file mode 100644 index 0000000..c9b9b83 --- /dev/null +++ b/crates/onenote-viewer/src/attachment.rs @@ -0,0 +1,563 @@ +use anyhow::{Context, Result}; +use gtk::gio; +use gtk::gio::prelude::*; +use gtk::glib; +use onenote_core::{ + LoadedNotebook, ResourceCopyControl, ResourceCopyOptions, ResourceCopyProgress, ResourceId, + SourceFingerprint, SourceId, +}; +use sanitize_filename::Options; +use std::io::{self, Write}; +use std::path::{Path, PathBuf}; +use std::sync::Arc; +use std::time::{Duration, SystemTime}; + +pub(crate) const MAX_ATTACHMENT_BYTES: u64 = 2 * 1024 * 1024 * 1024; +const MAX_CACHE_BYTES: u64 = 4 * 1024 * 1024 * 1024; +const MAX_CACHE_AGE: Duration = Duration::from_secs(30 * 24 * 60 * 60); +const MAX_FILENAME_BYTES: usize = 240; + +#[derive(Clone)] +pub(crate) struct CopyCancellation { + resource: ResourceCopyControl, + io: gio::Cancellable, +} + +impl CopyCancellation { + pub(crate) fn new() -> Self { + Self { + resource: ResourceCopyControl::new(), + io: gio::Cancellable::new(), + } + } + + pub(crate) fn cancel(&self) { + self.resource.cancel(); + self.io.cancel(); + } + + pub(crate) fn is_cancelled(&self) -> bool { + self.resource.is_cancelled() + } +} + +pub(crate) struct CopyRequest { + pub(crate) loaded: Arc, + pub(crate) resource_id: ResourceId, + pub(crate) destination: gio::File, + pub(crate) cancellation: CopyCancellation, +} + +pub(crate) fn copy_resource( + request: &CopyRequest, + progress: impl FnMut(ResourceCopyProgress), +) -> Result { + let (stream, destination_guard) = + replace_stream(&request.destination, &request.cancellation.io) + .with_context(|| format!("could not prepare {}", request.destination.parse_name()))?; + let mut writer = CancellableWriter { + stream, + cancellable: request.cancellation.io.clone(), + destination_guard, + published: false, + aborted: false, + }; + let result = request.loaded.resources.copy_to( + &request.resource_id, + &mut writer, + ResourceCopyOptions::new(MAX_ATTACHMENT_BYTES), + &request.cancellation.resource, + progress, + ); + let report = match result { + Ok(report) => report, + Err(error) => { + writer.abort(); + return Err(error.into()); + } + }; + if let Err(error) = writer.finish() { + if request.cancellation.is_cancelled() { + anyhow::bail!("Attachment copy was cancelled"); + } + return Err(error).context("could not publish the completed attachment"); + } + Ok(report.bytes_written) +} + +fn replace_stream( + destination: &gio::File, + cancellable: &gio::Cancellable, +) -> Result<(gio::FileOutputStream, DestinationGuard), glib::Error> { + let (version, existed) = match query_destination(destination, cancellable) { + Ok(version) => (Some(version), true), + Err(error) if error.matches(gio::IOErrorEnum::NotFound) => (None, false), + Err(error) => return Err(error), + }; + let etag = version.as_ref().and_then(|version| version.etag.as_deref()); + let stream = destination.replace( + etag, + false, + gio::FileCreateFlags::PRIVATE | gio::FileCreateFlags::REPLACE_DESTINATION, + Some(cancellable), + )?; + let destination_guard = DestinationGuard { + file: destination.clone(), + version: if existed { + version + } else { + query_destination(destination, cancellable).ok() + }, + delete_on_abort: !existed, + }; + Ok((stream, destination_guard)) +} + +fn query_destination( + file: &gio::File, + cancellable: &gio::Cancellable, +) -> Result { + const ATTRIBUTES: &str = + "etag::value,id::file,standard::size,time::modified,time::modified-usec"; + file.query_info( + ATTRIBUTES, + gio::FileQueryInfoFlags::NOFOLLOW_SYMLINKS, + Some(cancellable), + ) + .map(|info| DestinationVersion { + etag: info + .attribute_string(gio::FILE_ATTRIBUTE_ETAG_VALUE) + .map(|value| value.to_string()), + file_id: info + .attribute_string(gio::FILE_ATTRIBUTE_ID_FILE) + .map(|value| value.to_string()), + size: info + .has_attribute(gio::FILE_ATTRIBUTE_STANDARD_SIZE) + .then(|| info.size()), + modified: info + .has_attribute(gio::FILE_ATTRIBUTE_TIME_MODIFIED) + .then(|| info.attribute_uint64(gio::FILE_ATTRIBUTE_TIME_MODIFIED)), + modified_usec: info + .has_attribute(gio::FILE_ATTRIBUTE_TIME_MODIFIED_USEC) + .then(|| info.attribute_uint32(gio::FILE_ATTRIBUTE_TIME_MODIFIED_USEC)), + }) +} + +#[derive(Clone, Debug, Eq, PartialEq)] +struct DestinationVersion { + etag: Option, + file_id: Option, + size: Option, + modified: Option, + modified_usec: Option, +} + +struct DestinationGuard { + file: gio::File, + version: Option, + delete_on_abort: bool, +} + +struct CancellableWriter { + stream: gio::FileOutputStream, + cancellable: gio::Cancellable, + destination_guard: DestinationGuard, + published: bool, + aborted: bool, +} + +impl CancellableWriter { + fn finish(mut self) -> Result<()> { + // GIO owns publication semantics here. Some backends write through the + // visible target while others stage a sibling, so target metadata is + // not a portable indication of an external modification. + if let Err(error) = self.stream.flush(Some(&self.cancellable)) { + self.abort(); + return Err(error.into()); + } + if self.cancellable.is_cancelled() { + self.abort(); + anyhow::bail!("Attachment copy was cancelled"); + } + if let Err(error) = self.stream.close(Some(&self.cancellable)) { + self.abort(); + return Err(error.into()); + } + self.published = true; + Ok(()) + } + + fn abort(&mut self) { + if self.published || self.aborted { + return; + } + self.aborted = true; + let abort = gio::Cancellable::new(); + abort.cancel(); + let _ignored = self.stream.close(Some(&abort)); + let guard = &self.destination_guard; + let current = query_destination(&guard.file, &gio::Cancellable::new()).ok(); + if guard.delete_on_abort && same_destination_entry(current.as_ref(), guard.version.as_ref()) + { + let _ignored = guard.file.delete(None::<&gio::Cancellable>); + } + } +} + +impl Drop for CancellableWriter { + fn drop(&mut self) { + if !self.published { + self.abort(); + } + } +} + +fn same_destination_entry( + current: Option<&DestinationVersion>, + expected: Option<&DestinationVersion>, +) -> bool { + match (current, expected) { + (Some(current), Some(expected)) => match (¤t.file_id, &expected.file_id) { + (Some(current), Some(expected)) => current == expected, + _ => current == expected, + }, + _ => false, + } +} + +impl Write for CancellableWriter { + fn write(&mut self, buffer: &[u8]) -> io::Result { + self.stream + .write(buffer, Some(&self.cancellable)) + .and_then(|count| { + usize::try_from(count).map_err(|_| { + glib::Error::new(gio::IOErrorEnum::Failed, "invalid output byte count") + }) + }) + .map_err(|error| io::Error::other(error.to_string())) + } + + fn flush(&mut self) -> io::Result<()> { + self.stream + .flush(Some(&self.cancellable)) + .map_err(|error| io::Error::other(error.to_string())) + } +} + +pub(crate) fn format_size(bytes: u64) -> String { + const KIB: u64 = 1024; + const MIB: u64 = 1024 * KIB; + const GIB: u64 = 1024 * MIB; + let (unit, suffix) = if bytes >= GIB { + (GIB, "GiB") + } else if bytes >= MIB { + (MIB, "MiB") + } else if bytes >= KIB { + (KIB, "KiB") + } else { + return format!("{bytes} bytes"); + }; + let whole = bytes / unit; + let decimal = bytes % unit * 10 / unit; + format!("{whole}.{decimal} {suffix}") +} + +pub(crate) fn sanitized_filename(name: &str) -> String { + let source = name.trim(); + if source.is_empty() || matches!(source, "." | "..") { + return "attachment.bin".to_owned(); + } + let sanitized = sanitize_filename::sanitize_with_options( + source, + Options { + windows: true, + truncate: false, + replacement: "_", + }, + ); + let sanitized = sanitized.trim().trim_matches('.'); + let candidate = if sanitized.is_empty() { + "attachment.bin".to_owned() + } else if sanitized.starts_with('.') { + format!("attachment{sanitized}") + } else { + sanitized.to_owned() + }; + truncate_filename(&candidate, MAX_FILENAME_BYTES) +} + +fn truncate_filename(name: &str, max_bytes: usize) -> String { + if name.len() <= max_bytes { + return name.to_owned(); + } + let extension = name + .rsplit_once('.') + .filter(|(stem, extension)| !stem.is_empty() && extension.len() <= 32) + .map(|(_, extension)| format!(".{extension}")) + .unwrap_or_default(); + let stem_limit = max_bytes.saturating_sub(extension.len()).max(1); + let mut end = stem_limit.min(name.len()); + while !name.is_char_boundary(end) { + end -= 1; + } + format!("{}{}", &name[..end], extension) +} + +pub(crate) fn cache_file( + source_id: &SourceId, + fingerprint: &SourceFingerprint, + resource_id: &ResourceId, + display_name: &str, +) -> Result { + let root = cache_root(); + create_private_directory(&root)?; + let directory = root.join(cache_key(source_id, fingerprint, resource_id)); + create_private_directory(&directory)?; + Ok(gio::File::for_path( + directory.join(sanitized_filename(display_name)), + )) +} + +fn cache_root() -> PathBuf { + glib::user_cache_dir() + .join("onenote-viewer") + .join("attachments") +} + +fn cache_key( + source_id: &SourceId, + fingerprint: &SourceFingerprint, + resource_id: &ResourceId, +) -> String { + let mut hasher = blake3::Hasher::new(); + for value in [ + source_id.as_str(), + fingerprint.as_str(), + resource_id.as_str(), + ] { + hasher.update(&(value.len() as u64).to_le_bytes()); + hasher.update(value.as_bytes()); + } + hasher.finalize().to_hex().to_string() +} + +fn create_private_directory(path: &Path) -> Result<()> { + std::fs::create_dir_all(path) + .with_context(|| format!("could not create attachment cache {}", path.display()))?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o700)) + .with_context(|| format!("could not secure attachment cache {}", path.display()))?; + } + Ok(()) +} + +pub(crate) fn prune_cache() { + let root = cache_root(); + let Ok(entries) = std::fs::read_dir(&root) else { + return; + }; + let now = SystemTime::now(); + let mut retained = Vec::new(); + for entry in entries.flatten() { + let path = entry.path(); + let Ok(file_type) = entry.file_type() else { + continue; + }; + if !file_type.is_dir() || file_type.is_symlink() { + continue; + } + let Ok(metadata) = entry.metadata() else { + continue; + }; + let modified = metadata.modified().unwrap_or(SystemTime::UNIX_EPOCH); + if now.duration_since(modified).unwrap_or_default() > MAX_CACHE_AGE { + let _ignored = std::fs::remove_dir_all(path); + continue; + } + retained.push((modified, directory_size(&path), path)); + } + retained.sort_by_key(|(modified, _, _)| *modified); + let mut total = retained.iter().map(|(_, size, _)| *size).sum::(); + for (_, size, path) in retained { + if total <= MAX_CACHE_BYTES { + break; + } + if std::fs::remove_dir_all(path).is_ok() { + total = total.saturating_sub(size); + } + } +} + +fn directory_size(path: &Path) -> u64 { + std::fs::read_dir(path) + .into_iter() + .flatten() + .flatten() + .filter_map(|entry| { + let file_type = entry.file_type().ok()?; + if !file_type.is_file() || file_type.is_symlink() { + return None; + } + entry.metadata().ok().map(|metadata| metadata.len()) + }) + .sum() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn filenames_are_safe_portable_components() { + for source in [ + "../../report.pdf", + "/absolute/path", + "CON", + "a/b\\c\0.txt", + "..", + "", + ] { + let sanitized = sanitized_filename(source); + assert!(!sanitized.is_empty(), "{source:?}"); + assert_ne!(sanitized, ".", "{source:?}"); + assert_ne!(sanitized, "..", "{source:?}"); + assert!(!sanitized.contains(['/', '\\', '\0']), "{source:?}"); + assert!(sanitized.len() <= MAX_FILENAME_BYTES, "{source:?}"); + } + assert_eq!(sanitized_filename(".."), "attachment.bin"); + assert_eq!(sanitized_filename(""), "attachment.bin"); + } + + #[test] + fn long_unicode_filename_is_bounded_and_keeps_extension() { + let name = format!("{}.pdf", "ą".repeat(200)); + let sanitized = sanitized_filename(&name); + assert!(sanitized.len() <= MAX_FILENAME_BYTES); + assert!(sanitized.is_char_boundary(sanitized.len())); + assert!(Path::new(&sanitized) + .extension() + .is_some_and(|extension| extension.eq_ignore_ascii_case("pdf"))); + } + + #[test] + fn cache_keys_are_source_scoped_and_unambiguous() { + let first = cache_key( + &SourceId::new("a"), + &SourceFingerprint::new("bc"), + &ResourceId::new("d"), + ); + let second = cache_key( + &SourceId::new("ab"), + &SourceFingerprint::new("c"), + &ResourceId::new("d"), + ); + let third = cache_key( + &SourceId::new("a"), + &SourceFingerprint::new("bc"), + &ResourceId::new("different"), + ); + assert_ne!(first, second); + assert_ne!(first, third); + } + + #[test] + fn cancelled_replacement_preserves_existing_destination() { + let temporary = tempfile::tempdir().expect("temporary directory"); + let path = temporary.path().join("attachment.bin"); + std::fs::write(&path, b"original").expect("original destination"); + let file = gio::File::for_path(&path); + let cancellable = gio::Cancellable::new(); + let (stream, destination_guard) = + replace_stream(&file, &cancellable).expect("replacement stream"); + let mut writer = CancellableWriter { + stream, + cancellable, + destination_guard, + published: false, + aborted: false, + }; + writer.write_all(b"partial replacement").expect("write"); + writer.abort(); + + assert_eq!(std::fs::read(path).expect("destination"), b"original"); + } + + #[test] + fn cancelled_replacement_does_not_publish_new_destination() { + let temporary = tempfile::tempdir().expect("temporary directory"); + let path = temporary.path().join("attachment.bin"); + let file = gio::File::for_path(&path); + let cancellable = gio::Cancellable::new(); + let (stream, destination_guard) = + replace_stream(&file, &cancellable).expect("replacement stream"); + let mut writer = CancellableWriter { + stream, + cancellable, + destination_guard, + published: false, + aborted: false, + }; + writer.write_all(b"partial output").expect("write"); + writer.abort(); + + assert!(!path.exists()); + } + + #[test] + fn completed_replacement_publishes_exact_bytes() { + let temporary = tempfile::tempdir().expect("temporary directory"); + let path = temporary.path().join("attachment.bin"); + std::fs::write(&path, b"old").expect("old destination"); + let file = gio::File::for_path(&path); + let cancellable = gio::Cancellable::new(); + let (stream, destination_guard) = + replace_stream(&file, &cancellable).expect("replacement stream"); + let mut writer = CancellableWriter { + stream, + cancellable, + destination_guard, + published: false, + aborted: false, + }; + writer.write_all(b"complete replacement").expect("write"); + writer.finish().expect("publish"); + + assert_eq!( + std::fs::read(path).expect("destination"), + b"complete replacement" + ); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = std::fs::metadata(temporary.path().join("attachment.bin")) + .expect("metadata") + .permissions() + .mode() + & 0o777; + assert_eq!(mode, 0o600); + } + } + + #[test] + fn completed_new_destination_publishes_exact_bytes() { + let temporary = tempfile::tempdir().expect("temporary directory"); + let path = temporary.path().join("attachment.bin"); + let file = gio::File::for_path(&path); + let cancellable = gio::Cancellable::new(); + let (stream, destination_guard) = + replace_stream(&file, &cancellable).expect("replacement stream"); + let mut writer = CancellableWriter { + stream, + cancellable, + destination_guard, + published: false, + aborted: false, + }; + writer.write_all(b"new attachment").expect("write"); + writer.finish().expect("publish"); + + assert_eq!(std::fs::read(path).expect("destination"), b"new attachment"); + } +} diff --git a/crates/onenote-viewer/src/dialogs.rs b/crates/onenote-viewer/src/dialogs.rs index 9499a93..228e253 100644 --- a/crates/onenote-viewer/src/dialogs.rs +++ b/crates/onenote-viewer/src/dialogs.rs @@ -1,6 +1,7 @@ use crate::settings::ThemePreference; use gtk::gio; use gtk::prelude::*; +use onenote_core::{ResourceRef, ResourceStatus}; use std::cell::RefCell; use std::path::{Path, PathBuf}; use std::rc::Rc; @@ -32,6 +33,108 @@ pub(crate) fn present_about(parent_window: >k::ApplicationWindow) { dialog.present(); } +pub(crate) fn present_attachment( + parent_window: >k::ApplicationWindow, + resource: &ResourceRef, + on_open: O, + on_save: S, +) where + O: Fn() + 'static, + S: Fn() + 'static, +{ + let dialog = gtk::Window::builder() + .title("Attachment") + .transient_for(parent_window) + .modal(true) + .resizable(false) + .default_width(560) + .build(); + dialog.add_css_class("settings-dialog"); + + let content = dialog_content(14); + let filename = gtk_safe_text(&resource.name); + let heading = gtk::Label::builder() + .label(filename.as_ref()) + .xalign(0.0) + .selectable(true) + .wrap(true) + .wrap_mode(gtk::pango::WrapMode::WordChar) + .build(); + heading.add_css_class("dialog-title"); + content.append(&heading); + + let details = gtk::Label::builder() + .label(format!( + "Type: {}\nSize: {}", + gtk_safe_text(&resource.media_type), + crate::attachment::format_size(resource.size) + )) + .xalign(0.0) + .selectable(true) + .build(); + details.add_css_class("dim-label"); + content.append(&details); + + if resource.status != ResourceStatus::Available { + let status = match resource.status { + ResourceStatus::Missing => "The attachment data is missing from this OneNote source.", + ResourceStatus::Invalid => "The OneNote source marks this attachment data as invalid.", + ResourceStatus::Available => unreachable!(), + }; + let warning = gtk::Label::builder() + .label(status) + .xalign(0.0) + .selectable(true) + .wrap(true) + .build(); + warning.add_css_class("warning-label"); + content.append(&warning); + } + + let actions = gtk::Box::new(gtk::Orientation::Horizontal, 8); + actions.set_halign(gtk::Align::End); + let cancel = gtk::Button::with_label("Cancel"); + let save = gtk::Button::with_label("Save As..."); + let open = gtk::Button::with_label("Open"); + open.add_css_class("suggested-action"); + let available = resource.status == ResourceStatus::Available; + save.set_sensitive(available); + open.set_sensitive(available); + actions.append(&cancel); + actions.append(&save); + actions.append(&open); + content.append(&actions); + + let dialog_on_cancel = dialog.clone(); + cancel.connect_clicked(move |_| dialog_on_cancel.close()); + let dialog_on_save = dialog.clone(); + save.connect_clicked(move |_| { + dialog_on_save.close(); + on_save(); + }); + let dialog_on_open = dialog.clone(); + open.connect_clicked(move |_| { + dialog_on_open.close(); + on_open(); + }); + + dialog.set_child(Some(&content)); + if available { + open.grab_focus(); + } else { + cancel.grab_focus(); + } + dialog.present(); +} + +fn gtk_safe_text(value: &str) -> std::borrow::Cow<'_, str> { + if value.contains('\0') { + std::borrow::Cow::Owned(value.replace('\0', "\u{fffd}")) + } else { + std::borrow::Cow::Borrowed(value) + } +} + #[allow(clippy::too_many_lines)] pub(crate) fn present_package_import( parent_window: >k::ApplicationWindow, diff --git a/crates/onenote-viewer/src/main.rs b/crates/onenote-viewer/src/main.rs index 62aa9cc..f144002 100644 --- a/crates/onenote-viewer/src/main.rs +++ b/crates/onenote-viewer/src/main.rs @@ -3,6 +3,7 @@ #![forbid(unsafe_code)] mod app; +mod attachment; mod dialogs; mod navigation; mod navigation_state; diff --git a/crates/onenote-viewer/src/worker.rs b/crates/onenote-viewer/src/worker.rs index b949a7a..eac1345 100644 --- a/crates/onenote-viewer/src/worker.rs +++ b/crates/onenote-viewer/src/worker.rs @@ -1,3 +1,4 @@ +use gtk::gio; use onenote_core::{ ExtractionPhase, LoadOptions, LoadedNotebook, OneNoteLoader, OnePkgExtractor, SourceId, }; @@ -41,11 +42,30 @@ pub(crate) enum Event { result: Result, String>, }, Extracted { + operation_id: u64, result: Result, }, ExtractionProgress { + operation_id: u64, phase: ExtractionPhase, }, + AttachmentProgress { + operation_id: u64, + copied_bytes: u64, + declared_bytes: Option, + }, + AttachmentCopied { + operation_id: u64, + purpose: AttachmentPurpose, + destination: gio::File, + result: Result, + }, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum AttachmentPurpose { + Open, + Save, } pub(crate) fn start_index_worker( @@ -145,6 +165,7 @@ pub(crate) fn build_scene( } pub(crate) fn extract( + operation_id: u64, package: PathBuf, destination: PathBuf, cancel: Arc, @@ -155,11 +176,51 @@ pub(crate) fn extract( let result = OnePkgExtractor::detect() .and_then(|extractor| { extractor.extract_with_progress(&package, destination, &cancel, move |phase| { - let _ignored = progress_events.send(Event::ExtractionProgress { phase }); + let _ignored = progress_events.send(Event::ExtractionProgress { + operation_id, + phase, + }); }) }) .map(|report| report.destination) .map_err(|error| error.to_string()); - let _ignored = events.send(Event::Extracted { result }); + let _ignored = events.send(Event::Extracted { + operation_id, + result, + }); + }); +} + +pub(crate) fn copy_attachment( + operation_id: u64, + purpose: AttachmentPurpose, + request: crate::attachment::CopyRequest, + events: mpsc::Sender, +) { + std::thread::spawn(move || { + let progress_events = events.clone(); + let mut last_reported = 0_u64; + let result = crate::attachment::copy_resource(&request, move |progress| { + let completed = progress + .declared_bytes + .is_some_and(|declared| progress.copied_bytes == declared); + if progress.copied_bytes == 0 + || completed + || progress.copied_bytes.saturating_sub(last_reported) >= 1024 * 1024 + { + last_reported = progress.copied_bytes; + let _ignored = progress_events.send(Event::AttachmentProgress { + operation_id, + copied_bytes: progress.copied_bytes, + declared_bytes: progress.declared_bytes, + }); + } + }); + let _ignored = events.send(Event::AttachmentCopied { + operation_id, + purpose, + destination: request.destination, + result: result.map_err(|error| format!("{error:#}")), + }); }); } diff --git a/docs/MASTER-PLAN.md b/docs/MASTER-PLAN.md index b567bbd..a2e1114 100644 --- a/docs/MASTER-PLAN.md +++ b/docs/MASTER-PLAN.md @@ -133,8 +133,8 @@ The detailed milestone sequence and exit gates are in the - The five-crate Rust workspace and pinned Rust 1.85.1 toolchain are in place. - `onenote-core` projects native `.one`/`.onetoc2` sources into a public semantic and geometry model, fingerprints source trees, lazily exposes - bounded resources, and manages validated, atomic, on-disk `.onepkg` - extraction through `7zz`/`7z`. + bounded resources through cancellable streaming, and manages validated, + atomic, on-disk `.onepkg` extraction through `7zz`/`7z`. - `onenote-render` builds deterministic UI-neutral scenes, and `onenote-render-gtk` provides an independently runnable Pango/GSK page view with culling, pan, zoom, hit testing, bounded asynchronous image decoding, @@ -149,9 +149,12 @@ The detailed milestone sequence and exit gates are in the background parsing/indexing/scene construction, global search result navigation, a native freeform canvas, settings-backed default notebook location discovery, and package onboarding with destination confirmation, - phase progress, and cancellation. Its compact single-row shell exposes native - window controls, an application-command menu, and persisted System, Light, - and Dark themes under the [desktop UI requirements](specs/desktop-ui.md). + phase progress, and cancellation. Attachments can be saved or opened on + explicit request through bounded background streaming, safe destination + replacement, a private source-scoped cache, and desktop/portal delegation. + Its compact single-row shell exposes native window controls, an + application-command menu, and persisted System, Light, and Dark themes under + the [desktop UI requirements](specs/desktop-ui.md). Page title and creation time appear once in viewer chrome while the reusable renderer can still render the complete native title area for other hosts. - A manifest-free backup directory is not yet aggregated: the current @@ -184,10 +187,11 @@ The detailed milestone sequence and exit gates are in the 1. Implement the reusable manifest-free backup-folder loader and integrate its single synthetic notebook, reconstructed section groups, snapshot policy, workspace migration, and aggregate index generation. -2. Complete viewer workflows: attachment handling, package and long-operation - cancellation/progress, diagnostics, source refresh, and fuller workspace - restoration. Pointer-activated inline link handling is implemented; - keyboard access remains part of the accessibility work. +2. Complete remaining viewer workflows: package preflight/limits, + diagnostics, source refresh, tags, and fuller workspace restoration. + Pointer-activated inline link handling and safe on-demand attachment actions + are implemented; general canvas keyboard/screen-reader access remains part + of the accessibility work. 3. Establish visual oracles and measured tolerances for rich text, tables, images/printouts, ink, negative coordinates, overlap, and large pages. 4. Map scene semantics into GTK accessibility, add keyboard navigation, and diff --git a/docs/REMAINING-WORK.md b/docs/REMAINING-WORK.md index ddcec71..a884a1e 100644 --- a/docs/REMAINING-WORK.md +++ b/docs/REMAINING-WORK.md @@ -27,8 +27,9 @@ make visual regressions testable on fixed fonts and renderers. **Why open:** `PageScene` carries semantic labels and roles, and GTK navigation uses standard virtualized controls, but scene nodes are not exposed as -focusable GTK accessible children. Links and attachments cannot be reached by -keyboard. Orca has not been tested. +focusable GTK accessible children. Attachment hit regions have focus traversal +and keyboard activation; inline text links and general scene nodes do not. +Orca has not been tested. **Completion:** Implement a virtual accessible object/focus model synchronized with viewport and hit regions. Add keyboard reading order and activation, then @@ -37,16 +38,15 @@ record Orca tests under GNOME and KDE on Wayland and X11. ### 3. Complete Viewer Actions and Operations **Why open:** The viewer resolves search hits, opens pointer-activated inline -links, and imports packages, but does not yet implement attachment -extraction/opening, operation cancellation/progress, per-source diagnostics, -manual refresh, or automatic source-change refresh. Package cancellation -exists in the core API but is not wired to the UI. Active page and pane state -are not restored. +links, safely saves/opens attachments, and imports packages. Attachment and +package writes share cancellable progress UI, but per-source diagnostics, +manual refresh, automatic source-change refresh, and restoration of active +page/pane state remain incomplete. -**Completion:** Add explicit host policies for renderer actions; bounded, -sanitized attachment extraction; cancellable operations with progress; source -fingerprint monitoring and transactional refresh; diagnostics surfaces; and -versioned workspace state with corruption recovery tests. +**Completion:** Add source fingerprint monitoring and transactional refresh, +diagnostics surfaces, and versioned workspace state with corruption recovery +tests. Extend the operation coordinator only when another concrete long-running +workflow requires it. ### 4. Manifest-Free Backup Folder Aggregation @@ -143,6 +143,9 @@ The following are complete enough to build on, but remain pre-1.0: rendering with embedded fonts and visible fallback diagnostics; - embeddable GTK `PageView` with Pango/GSK/Cairo rendering, pan/zoom, and bounded asynchronous raster decode; +- bounded on-demand attachment Save As/Open with portable name sanitation, + GIO replacement, cancellation/progress, source-scoped private cache, desktop + delegation, and unchanged-destination failure tests; - transactional multi-source FTS5 indexing and structured Rust/JSONL queries; - native GTK multi-notebook shell with virtualized navigation, persistence, configurable XDG Documents library discovery, diff --git a/docs/architecture/system-architecture.md b/docs/architecture/system-architecture.md index 396d94a..1839bbe 100644 --- a/docs/architecture/system-architecture.md +++ b/docs/architecture/system-architecture.md @@ -64,6 +64,13 @@ Index/query API UI-neutral page scene parsed model becomes available to the UI. 6. A failed index replacement leaves its last good generation available. Full automatic/manual source refresh is not implemented yet. +7. Activating an attachment returns its stable resource action to the viewer. + The viewer presents explicit Open and Save As choices. Both stream the lazy + payload on a worker through the core resource API and a cancellable GIO + replacement stream. Save As writes to the portal-selected `GFile`; Open + writes to an XDG cache entry scoped by source identity, fingerprint, and + resource identity, then uses `GtkFileLauncher`. No attachment is extracted + during parsing, import, rendering, or indexing. Source files are opened read-only. There is no write-back code path. The immutable domain model preserves semantic objects and their source @@ -172,10 +179,17 @@ of application-global state, cancellable, and resource-bounded. See - The viewer currently serializes discovery, parsing, and indexing on one worker and runs scene/search jobs separately. It requires bounded scheduling and operation cancellation before release. +- Package imports and attachment copies share one foreground file-operation + coordinator. Operation identities reject stale worker progress/completion; + only one destination-writing operation runs at a time, while scene and + search jobs remain independent. - Reusable index, scene, and package APIs accept caller cancellation at their documented checkpoints; parser projection cancellation is not yet public. - Binary payloads are streamed and decoded lazily; attachments are not loaded while indexing unless a bounded extractor is explicitly enabled. +- Attachment copies use a fixed 64 KiB core buffer, an explicit per-file + ceiling, cooperative cancellation, declared-size validation, and + destination replacement rather than full-payload allocation. - Images use encoded/decoded size and dimension limits plus a bounded texture cache. - The index library reports progress through caller callbacks and the viewer @@ -191,6 +205,11 @@ configuration only after real corpus measurements. - Reject absolute paths, `..`, device paths, and symlink escapes referenced by notebook metadata. +- Treat attachment display names as untrusted metadata. Save/cache leaf names + are sanitized with portable Windows restrictions and bounded UTF-8 length; + cache directories use hashed source/resource identities and private modes. +- Never execute attachments in-process. Opening first materializes a completed + read-only cache file and delegates it to the desktop through `FileLauncher`. - Sniff content from bytes; extensions are hints only. - Bound every allocation derived from input lengths and dimensions. - Treat text, URLs, filenames, and metadata as data, never markup. diff --git a/docs/limitations.md b/docs/limitations.md index 2dd7814..0b12232 100644 --- a/docs/limitations.md +++ b/docs/limitations.md @@ -122,8 +122,8 @@ contains the user-facing behavior. private staging, cancellation, destination conflicts, and atomic publication are implemented. Tests cover unsafe paths, bounded listing capture, pre-launch cancellation, unchanged-source real extraction, and corpus - counts. The viewer imports packages but does not yet expose progress or - cancellation. + counts. The viewer exposes package phases and cancellation through its shared + foreground file-operation surface. - **Release gate:** Missing-tool, corrupt/truncated, path, limit, cancellation, partial-output, and source-unchanged cases pass. @@ -167,6 +167,10 @@ contains the user-facing behavior. persisted printout/preview images if available; open externally on request. - **Security boundary:** Never activate macros, OLE, scripts, or executables in-process. External applications assume responsibility after confirmation. +- **Current evidence:** The original bytes can be saved or materialized in a + private cache on explicit request using bounded streaming, cancellation, + portable name sanitation, and GIO replacement. The viewer delegates Open to + the desktop and does not preview or execute attachment content in-process. ### L-014: PDF and document printouts @@ -270,9 +274,10 @@ lost effects. - **Mitigation:** checked/bounded parsing, canonical root containment, payload streaming, content sniffing, no embedded web runtime, and fuzzing. - **Current evidence:** Canonical source access, projection/resource limits, - package containment checks, bounded image decode, inert URLs/attachments, - and no web runtime are implemented. Parser fuzzing, aggregate memory limits, - attachment extraction policy, and package expansion/disk ceilings remain. + package containment checks, bounded image decode, confirmed external links, + bounded/sanitized attachment materialization, and no web runtime are + implemented. Parser fuzzing, aggregate memory limits, and package + expansion/disk ceilings remain. ### L-024: Flatpak access to notebook directory trees @@ -332,11 +337,14 @@ Current defensive defaults are: - 1,000,000 extracted entries and a 16 MiB bounded archive listing; - 32 MiB encoded and 64 MiB decoded per image, maximum dimension 16,384, and a 128 MiB GTK texture cache; +- 2 GiB per attachment copy and a 4 GiB attachment-cache target; cache entries + older than 30 days are pruned at startup before size-based pruning; - 1,000,000 generated scene nodes per page; - 1,000 search results and 2,048 snippet characters per query. These ceilings are code defaults, not corpus-tuned release guarantees. -Section bytes, aggregate model memory, attachment extraction, expanded package -bytes/disk, coordinates, graph depth, and concurrency queues still need -measured limits. Until those and hostile-input tests pass, release builds must -not claim arbitrary untrusted notebook/package safety. +Section bytes, aggregate model memory, expanded package bytes/disk, +coordinates, graph depth, and concurrency queues still need measured limits. +The attachment ceilings are defensive defaults rather than corpus-tuned +guarantees. Until the remaining limits and hostile-input tests pass, release +builds must not claim arbitrary untrusted notebook/package safety. diff --git a/docs/plans/roadmap.md b/docs/plans/roadmap.md index 0272fb4..aa517f9 100644 --- a/docs/plans/roadmap.md +++ b/docs/plans/roadmap.md @@ -106,7 +106,10 @@ independent protocol fixtures pass. Recorded latency targets remain open. reusable scene construction retains a full-page option. - [x] Preserve and pointer-activate inline web, mail, file, and OneNote page links through host-owned policy. -- [ ] Complete tags and safe attachment extraction. +- [ ] Complete tags. +- [x] Add safe on-demand attachment Save As and Open actions with bounded + streaming, portable filename sanitation, progress, cancellation, private + cache materialization, and desktop/portal delegation. - [x] Project, render, and index basic OfficeMath with structured fallback. - [x] Global search with result navigation to matching object geometry. - [x] Persistent all-open-notebooks workspace and default global search scope. @@ -169,7 +172,7 @@ end-to-end package-import tests remain release work. ## Deferred Backlog - Password-protected sections. -- Sandboxed attachment body extraction. +- Optional in-application previews for explicitly supported attachment types. - Media playback with note timing. - Old-version/conflict browsing. - Additional distribution packages. diff --git a/docs/specs/product-requirements.md b/docs/specs/product-requirements.md index f517dd1..11d1108 100644 --- a/docs/specs/product-requirements.md +++ b/docs/specs/product-requirements.md @@ -171,6 +171,8 @@ The primary window provides: - persisted System, Light, and Dark application themes with readable active, inactive, selected, and disabled states; - scoped compatibility and source-refresh diagnostics. +- explicit attachment details with Open and Save As actions, bounded + background copying, progress/cancellation, and copyable failure diagnostics. Navigation lists must be virtualized for large notebooks. Loading, indexing, refresh, extraction, and cancellation expose progress without blocking the UI. @@ -202,6 +204,10 @@ The MVP cannot be called a native OneNote viewer until: remain free of application-global state; 11. a manifest-free backup folder opens as one source with reconstructed section groups, deterministic snapshot selection, and explicit provenance. +12. available attachments can be saved byte-for-byte or opened through the + desktop handler without changing the notebook source; cancellation, + integrity failure, unsafe names, and concurrent destination changes do not + publish partial output or overwrite a newer destination. ## Existing Linux Tools Are Not Substitutes diff --git a/docs/specs/public-api.md b/docs/specs/public-api.md index 938ef5b..3d532f7 100644 --- a/docs/specs/public-api.md +++ b/docs/specs/public-api.md @@ -44,6 +44,16 @@ not implemented; its contract and delivery gates are in the `OnePkgExtractor` is a separate optional operation and is never required to consume an already extracted source. +`ResourceStore::copy_to` is the reusable attachment/image payload boundary. +It performs blocking, bounded streaming into a caller-owned `Write`, reports +progress on the calling thread, accepts a cloneable cooperative cancellation +handle, enforces an explicit byte ceiling, and validates reliable declared +payload lengths. It returns typed read, write, cancellation, size-limit, and +size-mismatch errors. Callers choose their worker/thread model and own durable +destination publication; the core library never selects paths, creates a +cache, or launches an attachment. This additive contract is exposed by core +API version 6. + The public model includes: - source identity and fingerprint; @@ -125,6 +135,13 @@ does not own notebook navigation, search UI, window creation, recent files, or workspace persistence. The `standalone` example embeds it in a window without linking `onenote-viewer`. +Attachment hit regions provide theme-aware visuals, pointer/tooltips, bounded +focus traversal, and Enter/Space activation. The renderer still returns only +`HitAction::OpenAttachment`; the embedding host owns availability diagnostics, +destination selection, persistence, desktop launching, and execution policy. +This keyboard action support is not a claim that the custom canvas exposes a +complete virtual accessibility tree. + `PageView` exposes its effective bounded zoom and a change notification that covers built-in gestures as well as host-initiated changes. Hosts can therefore synchronize controls or persist a preference without duplicating zoom input