From 3b11b0534e3cc17c504aa6531661743243989249 Mon Sep 17 00:00:00 2001 From: Sophie Herold Date: Wed, 3 Dec 2025 22:35:13 +0100 Subject: [PATCH] metadata: Drop unreliable mime-type Only use a reliable mime-type. This requires that we load images earlier in the pipe. This also fixes warnings about mime types not being available on network drives like SMB. Fixes #513 Issue #534 --- news.d/50.alpha/fixed-wrong-image-format | 1 + src/decoder.rs | 50 +++++++++++++++++------- src/decoder/formats/glycin_proxy.rs | 14 +------ src/decoder/formats/svg.rs | 11 +----- src/metadata.rs | 16 +------- src/metadata/file.rs | 4 -- src/widgets/image/loading.rs | 27 +++++++------ src/widgets/image_page.rs | 2 +- 8 files changed, 57 insertions(+), 68 deletions(-) create mode 100644 news.d/50.alpha/fixed-wrong-image-format diff --git a/news.d/50.alpha/fixed-wrong-image-format b/news.d/50.alpha/fixed-wrong-image-format new file mode 100644 index 0000000..3e10586 --- /dev/null +++ b/news.d/50.alpha/fixed-wrong-image-format @@ -0,0 +1 @@ +Showing wrong mime type in error message if the file ending for an image is wrong. diff --git a/src/decoder.rs b/src/decoder.rs index 1827bf6..122af38 100644 --- a/src/decoder.rs +++ b/src/decoder.rs @@ -124,7 +124,11 @@ impl UpdateSender { } pub fn send_error(&self, err: glycin::ErrorCtx) { - if err.unsupported_format().is_some() { + if let Some(mime_type) = err.unsupported_format() { + let mut metadata = Metadata::default(); + metadata.set_mime_type(mime_type); + + self.send(DecoderUpdate::Metadata(Box::new(metadata))); self.send(DecoderUpdate::SpecificError( DecoderError::UnsupportedFormat, )); @@ -150,6 +154,7 @@ pub struct Decoder { enum FormatDecoder { Glycin(Glycin), Svg(Svg), + Failed, } impl Decoder { @@ -159,7 +164,6 @@ impl Decoder { /// The renderer should listen to updates from the returned receiver. pub async fn new( file: gio::File, - mime_type: Option, tiles: Arc, ) -> (Self, mpsc::Receiver) { let (sender, receiver) = mpsc::unbounded(); @@ -167,7 +171,26 @@ impl Decoder { let update_sender = UpdateSender { sender }; tiles.set_update_sender(update_sender.clone()); - let format_decoder = Self::format_decoder(update_sender.clone(), file, mime_type, tiles); + log::trace!("Setting up loader"); + let mut loader = glycin::Loader::new(file); + loader.apply_transformations(false); + + let image = match loader.load().await { + Ok(image) => image, + Err(err) => { + update_sender.send_error(err); + return ( + Self { + decoder: FormatDecoder::Failed, + update_sender, + }, + receiver, + ); + } + }; + + let format_decoder = Self::format_decoder(update_sender.clone(), image, tiles); + let decoder = Self { decoder: format_decoder, update_sender, @@ -178,20 +201,19 @@ impl Decoder { fn format_decoder( update_sender: UpdateSender, - file: gio::File, - mime_type: Option, + image: glycin::Image, tiles: Arc, ) -> FormatDecoder { - if let Some(mime_type) = mime_type { - // Known things we want to match here are - // - image/svg+xml - // - image/svg+xml-compressed - if mime_type.split('+').next() == Some("image/svg") { - return FormatDecoder::Svg(Svg::new(file, update_sender, tiles)); - } - } + let mime_type = image.mime_type().to_string(); - FormatDecoder::Glycin(Glycin::new(file, update_sender, tiles)) + // Known things we want to match here are + // - image/svg+xml + // - image/svg+xml-compressed + if mime_type.split('+').next() == Some("image/svg") { + return FormatDecoder::Svg(Svg::new(image, update_sender, tiles)); + } else { + FormatDecoder::Glycin(Glycin::new(image, update_sender, tiles)) + } } /// Request missing tiles diff --git a/src/decoder/formats/glycin_proxy.rs b/src/decoder/formats/glycin_proxy.rs index 0281717..953ebdc 100644 --- a/src/decoder/formats/glycin_proxy.rs +++ b/src/decoder/formats/glycin_proxy.rs @@ -42,22 +42,12 @@ impl Drop for Glycin { } impl Glycin { - pub fn new(file: gio::File, updater: UpdateSender, tiles: Arc) -> Self { - let cancellable = gio::Cancellable::new(); - let cancellable_ = cancellable.clone(); + pub fn new(image: glycin::Image, updater: UpdateSender, tiles: Arc) -> Self { + let cancellable = image.cancellable(); let (next_frame_send, next_frame_recv) = async_channel::bounded(2); updater.clone().spawn_error_handled(async move { - log::trace!("Setting up loader"); - let mut loader = glycin::Loader::new(file); - loader.apply_transformations(false); - - loader.cancellable(cancellable_); - - let image = loader.load().await?; - log::trace!("Image info received"); - let mut metadata: Metadata = Metadata::default(); metadata.set_image_info(image.details().clone()); metadata.set_mime_type(image.mime_type().to_string()); diff --git a/src/decoder/formats/svg.rs b/src/decoder/formats/svg.rs index ab58cc8..5b84f23 100644 --- a/src/decoder/formats/svg.rs +++ b/src/decoder/formats/svg.rs @@ -61,21 +61,14 @@ impl Drop for Svg { } impl Svg { - pub fn new(file: gio::File, updater: UpdateSender, tiles: Arc) -> Self { + pub fn new(image: glycin::Image, updater: UpdateSender, tiles: Arc) -> Self { let current_request: Arc> = Default::default(); let request_store = current_request.clone(); - let cancellable = gio::Cancellable::new(); + let cancellable = image.cancellable(); let cancellable_ = cancellable.clone(); let (wakeup, wakeup_resv) = mpsc::unbounded(); updater.clone().spawn_error_handled(async move { - log::trace!("Setting up SVG loader"); - let mut loader = glycin::Loader::new(file); - - loader.cancellable(cancellable.clone()); - - let image = loader.load().await?; - let mut metadata: Metadata = Metadata::default(); metadata.set_image_info(image.details().clone()); metadata.set_mime_type(image.mime_type().to_string()); diff --git a/src/metadata.rs b/src/metadata.rs index 75baa1b..bc79a11 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -116,25 +116,11 @@ impl Metadata { self.mime_type.clone() } - /// Unreliable mime type - /// - /// Before the image is successfully loaded, this mime type might be wrong - /// since it is not yet based on the more complex mime type detection used - /// by glyin. - pub fn unreliable_mime_type(&self) -> Option { - self.mime_type().or_else(|| { - self.file_info - .as_ref() - .and_then(|x| x.mime_type.as_ref()) - .map(|x| x.to_string()) - }) - } - pub fn format_name(&self) -> Option { self.image_info .as_ref() .and_then(|x| x.info_format_name().map(|x| x.to_string())) - .or_else(|| self.unreliable_mime_type()) + .or_else(|| self.mime_type()) } pub fn alpha_channel(&self) -> Option { diff --git a/src/metadata/file.rs b/src/metadata/file.rs index 4f92c5b..3b27c19 100644 --- a/src/metadata/file.rs +++ b/src/metadata/file.rs @@ -23,7 +23,6 @@ const ATTRIBUTE_HOST_PATH: &str = "xattr::document-portal.host-path"; pub struct FileInfo { pub(super) display_name: glib::GString, pub(super) file_size: Option, - pub(super) mime_type: Option, pub(super) created: Option, pub(super) modified: Option, pub(super) host_path: Option, @@ -50,9 +49,6 @@ impl FileInfo { Ok(Self { display_name: file_info.display_name(), file_size, - mime_type: file_info - .content_type() - .and_then(|x| gio::content_type_get_mime_type(&x)), created: file_info.creation_date_time(), modified: file_info.modification_date_time(), host_path: file_info.attribute_as_string(ATTRIBUTE_HOST_PATH), diff --git a/src/widgets/image/loading.rs b/src/widgets/image/loading.rs index a9fb466..df06cc4 100644 --- a/src/widgets/image/loading.rs +++ b/src/widgets/image/loading.rs @@ -32,17 +32,22 @@ impl imp::LpImage { } /// Set filename after filename changed - async fn set_file_changed(&self, file: &gio::File) { + async fn set_file_changed(&self, old_file: &gio::File, new_file: &gio::File) { let obj = self.obj(); - let prev_mime_type = obj.metadata().unreliable_mime_type(); + let old_filename_ext = old_file + .basename() + .map(|x| x.extension().map(|x| x.to_owned())); + let new_filename_ext = new_file + .basename() + .map(|x| x.extension().map(|x| x.to_owned())); - self.file.replace(Some(file.clone())); + self.file.replace(Some(new_file.clone())); self.reload_file_info().await; - // Reload if mime type changed since other decoding path might be responsible + // Reload if file extension changed since other decoding path might be responsible // now - if obj.metadata().unreliable_mime_type() != prev_mime_type { - obj.load(file).await; + if old_filename_ext != new_filename_ext { + obj.load(new_file).await; return; } @@ -223,6 +228,7 @@ impl imp::LpImage { obj.load(&file).await; }); } else { + let old_file = file_a.clone(); glib::spawn_future_local(async move { if obj.error().is_some() { // Tmp files might be renamed quickly after creation. In this case @@ -230,7 +236,7 @@ impl imp::LpImage { // Therefore reload on name change loading caused an error. obj.load(&file).await; } else { - obj.imp().set_file_changed(&file).await; + obj.imp().set_file_changed(&old_file, &file).await; } }); } @@ -365,12 +371,7 @@ impl LpImage { // Reset background color for reloads imp.set_background_color(None); - let (decoder, decoder_update) = Decoder::new( - file.clone(), - self.metadata().unreliable_mime_type(), - tiles.clone(), - ) - .await; + let (decoder, decoder_update) = Decoder::new(file.clone(), tiles.clone()).await; let weak_obj = self.downgrade(); glib::spawn_future_local(async move { diff --git a/src/widgets/image_page.rs b/src/widgets/image_page.rs index c52d715..328f883 100644 --- a/src/widgets/image_page.rs +++ b/src/widgets/image_page.rs @@ -231,7 +231,7 @@ impl LpImagePage { imp.stack.set_visible_child(&*imp.error_page); imp.error_more_info.set_visible(false); } else if image.specific_error() == DecoderError::UnsupportedFormat { - let mime_type = image.metadata().unreliable_mime_type().unwrap_or_default(); + let mime_type = image.metadata().mime_type().unwrap_or_default(); let content_type = gio::content_type_from_mime_type(&mime_type).unwrap_or_default(); let description = gio::content_type_get_description(&content_type).to_string();