From 4f94e0c5928a0d76d0d77d9cf3606504658419e3 Mon Sep 17 00:00:00 2001 From: ronibalthazar <61232335+ronibalthazar@users.noreply.github.com> Date: Sat, 24 Oct 2026 16:42:22 +0100 Subject: [PATCH 1/5] LR-META-LOCATION: validate typed GPS coordinates --- crates/engine/src/cmd/library.rs | 283 ++++++++++++++++++++++++++++--- docs/gaps.md | 3 +- docs/parity-checklist.md | 5 +- docs/whats-new.md | 3 + 4 files changed, 263 insertions(+), 31 deletions(-) diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index 9d11cc5b3..82070f498 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -1821,43 +1821,177 @@ fn has_library(s: &Session) -> std::result::Result<(), String> { /// "51°30'26\"N 0°7'39\"W" (degrees, minutes, seconds; N/S/E/W or signs). pub(crate) fn parse_gps(t: &str) -> Option<(f64, f64)> { // typographic minus signs and dashes mean a negative value, like ASCII '-' - let t = t.replace(['\u{2212}', '\u{2012}', '\u{2013}', '\u{FE63}', '\u{FF0D}'], "-"); let t = t.trim(); - // split into the two coordinates: at a comma, else after the first N/S hemisphere letter - let (a, b) = match t.split_once(',') { - Some((a, b)) => (a.trim().to_string(), b.trim().to_string()), - None => { - let lead = t.starts_with(['N', 'S', 'n', 's']); - let i = if lead { - // hemisphere letter first: "N 51.5 W 0.1" splits at the E/W letter - t.char_indices().skip(1).find(|(_, c)| matches!(c, 'E' | 'W' | 'e' | 'w')).map(|(i, _)| i) - } else { - t.find(['N', 'S', 'n', 's']).map(|i| i + 1) - } - .or_else(|| t.find(char::is_whitespace))?; - (t[..i].trim().to_string(), t[i..].trim().to_string()) + if t.is_empty() || t.chars().take(129).count() > 128 { + return None; + } + let normalized = t.replace(['\u{2212}', '\u{2012}', '\u{2013}', '\u{FE63}', '\u{FF0D}'], "-"); + let t = normalized.as_str(); + + if let Some((a, b)) = t.split_once(',') { + if b.contains(',') { + return None; } + return parse_gps_pair(a, b); + } + + // A suffix N/S marks the end of latitude; a prefix N/S uses the next hemisphere prefix as + // the boundary. If only longitude has a hemisphere, the whitespace pass below handles it. + let mut latitude_markers = t.char_indices().filter(|(_, c)| matches!(c, 'N' | 'S' | 'n' | 's')); + if let Some((first, marker)) = latitude_markers.next() { + if first == 0 { + if let Some((second, _)) = t.char_indices().skip(1).find(|(_, c)| is_gps_hemisphere(*c)) { + return parse_gps_pair(t.get(..second)?, t.get(second..)?); + } + } else { + let end = first.checked_add(marker.len_utf8())?; + return parse_gps_pair(t.get(..end)?, t.get(end..)?); + } + } + + // Without hemisphere letters, a comma-free pair is separated by whitespace. Trying each + // boundary also permits spaces between DMS components without letting unconsumed text vanish. + for (at, separator) in t.char_indices().filter(|(_, c)| c.is_whitespace()) { + let after = at.checked_add(separator.len_utf8())?; + if let Some(pair) = parse_gps_pair(t.get(..at)?, t.get(after..)?) { + return Some(pair); + } + } + None +} + +#[derive(Clone, Copy)] +enum GpsAxis { + Latitude, + Longitude, +} + +fn is_gps_hemisphere(c: char) -> bool { + matches!(c, 'N' | 'S' | 'E' | 'W' | 'n' | 's' | 'e' | 'w') +} + +fn parse_gps_pair(latitude: &str, longitude: &str) -> Option<(f64, f64)> { + Some((parse_gps_coordinate(latitude, GpsAxis::Latitude)?, parse_gps_coordinate(longitude, GpsAxis::Longitude)?)) +} + +fn parse_gps_coordinate(text: &str, axis: GpsAxis) -> Option { + let text = text.trim(); + if text.is_empty() { + return None; + } + + let prefix = ['N', 'S', 'E', 'W'] + .into_iter() + .find_map(|h| text.strip_prefix(h).or_else(|| text.strip_prefix(h.to_ascii_lowercase())).map(|rest| (h, rest))); + let suffix = ['N', 'S', 'E', 'W'] + .into_iter() + .find_map(|h| text.strip_suffix(h).or_else(|| text.strip_suffix(h.to_ascii_lowercase())).map(|rest| (rest, h))); + let (body, hemisphere) = if let Some((h, body)) = prefix { + (body, Some(h)) + } else if let Some((body, h)) = suffix { + (body, Some(h)) + } else { + (text, None) }; - let one = |s: &str, pos: char, neg: char| -> Option { - let up = s.to_ascii_uppercase(); - let sign = if up.contains(neg) || up.trim_start().starts_with('-') { -1.0 } else { 1.0 }; - let nums: Vec = - up.split(|c: char| !(c.is_ascii_digit() || c == '.')).filter(|x| !x.is_empty()).map(|x| x.parse::().ok()).collect::>()?; - let v = match nums.as_slice() { - [d] => *d, - [d, m] => d + m / 60.0, - [d, m, s] => d + m / 60.0 + s / 3600.0, + if body.chars().any(is_gps_hemisphere) { + return None; + } + let body = body.trim(); + if body.is_empty() { + return None; + } + + let hemisphere_negative = match (axis, hemisphere) { + (_, None) => None, + (GpsAxis::Latitude, Some('N')) | (GpsAxis::Longitude, Some('E')) => Some(false), + (GpsAxis::Latitude, Some('S')) | (GpsAxis::Longitude, Some('W')) => Some(true), + _ => return None, + }; + + let mut components = Vec::with_capacity(3); + let mut token = String::new(); + let mut explicit_sign = None; + let mut degree_mark = false; + let mut minute_mark = false; + let mut second_mark = false; + + for c in body.chars() { + match c { + '0'..='9' | '.' => token.push(c), + '+' | '-' if components.is_empty() && token.is_empty() => token.push(c), + c if c.is_whitespace() => flush_gps_number(&mut token, &mut components, &mut explicit_sign)?, + '°' => { + flush_gps_number(&mut token, &mut components, &mut explicit_sign)?; + if components.len() != 1 || degree_mark || minute_mark || second_mark { + return None; + } + degree_mark = true; + } + '\'' | '′' => { + flush_gps_number(&mut token, &mut components, &mut explicit_sign)?; + if components.len() != 2 || minute_mark || second_mark { + return None; + } + minute_mark = true; + } + '"' | '″' => { + flush_gps_number(&mut token, &mut components, &mut explicit_sign)?; + if components.len() != 3 || !minute_mark || second_mark { + return None; + } + second_mark = true; + } _ => return None, - }; - let _ = pos; - Some(sign * v) + } + } + flush_gps_number(&mut token, &mut components, &mut explicit_sign)?; + + let [degrees, rest @ ..] = components.as_slice() else { return None }; + if rest.iter().any(|v| *v < 0.0 || *v >= 60.0) { + return None; + } + let magnitude = degrees.abs() + rest.first().copied().unwrap_or(0.0) / 60.0 + rest.get(1).copied().unwrap_or(0.0) / 3600.0; + if !magnitude.is_finite() { + return None; + } + + let negative = match (hemisphere_negative, explicit_sign) { + (Some(hemisphere_negative), Some(explicit_negative)) if hemisphere_negative != explicit_negative => return None, + (Some(hemisphere_negative), _) => hemisphere_negative, + (None, Some(explicit_negative)) => explicit_negative, + (None, None) => false, }; - let (la, lo) = (one(&a, 'N', 'S')?, one(&b, 'E', 'W')?); - (la.abs() <= 90.0 && lo.abs() <= 180.0).then_some((la, lo)) + let value = if negative { -magnitude } else { magnitude }; + let limit = match axis { + GpsAxis::Latitude => 90.0, + GpsAxis::Longitude => 180.0, + }; + (value.is_finite() && value.abs() <= limit).then_some(value) +} + +fn flush_gps_number(token: &mut String, components: &mut Vec, explicit_sign: &mut Option) -> Option<()> { + if token.is_empty() { + return Some(()); + } + if components.len() == 3 { + return None; + } + let value = token.parse::().ok()?; + if !value.is_finite() { + return None; + } + if components.is_empty() && (token.starts_with('-') || token.starts_with('+')) { + *explicit_sign = Some(token.starts_with('-')); + } + components.push(value); + token.clear(); + Some(()) } #[cfg(test)] mod gps_tests { + use serde_json::json; + #[test] fn coordinates_people_type() { let p = super::parse_gps; @@ -1870,8 +2004,101 @@ mod gps_tests { assert_eq!(p("N 51.5072 W 0.1276"), Some((51.5072, -0.1276))); assert_eq!(p("S 33.8688 E 151.2093"), Some((-33.8688, 151.2093))); assert_eq!(p("\u{2212}33.8688, 151.2093"), Some((-33.8688, 151.2093))); + assert_eq!(p("51.5072 -0.1276"), Some((51.5072, -0.1276))); + assert_eq!(p("51.5072 0.1276 W"), Some((51.5072, -0.1276))); + let (la, lo) = p("51° 30' 26\" 0° 7' 39\"").unwrap(); + assert!((la - 51.50722).abs() < 1e-4 && (lo - 0.1275).abs() < 1e-4, "{la} {lo}"); assert_eq!(p("95, 10"), None); assert_eq!(p("hello"), None); + assert_eq!(p("s 33.8688 e 151.2093"), Some((-33.8688, 151.2093))); + assert_eq!(p("0N, 0E"), Some((0.0, 0.0))); + assert_eq!(p("90N, 180E"), Some((90.0, 180.0))); + assert_eq!(p("90S, 180W"), Some((-90.0, -180.0))); + } + + #[test] + fn gps_does_not_reinterpret_exponents() { + assert_eq!(super::parse_gps("1e-3, 2e-3"), None); + } + + #[test] + fn gps_rejects_wrong_axis_or_conflicting_hemispheres() { + let p = super::parse_gps; + assert_eq!(p("45E, 7N"), None); + assert_eq!(p("45E 7N"), None); + assert_eq!(p("45NS, 7E"), None); + assert_eq!(p("-45N, 7E"), None); + assert_eq!(p("45N 7NE"), None); + } + + #[test] + fn gps_rejects_unconsumed_coordinate_text() { + assert_eq!(super::parse_gps("garbage45, 7junk"), None); + } + + #[test] + fn gps_rejects_oversized_coordinate_text() { + let input = format!("{}, 0", "0".repeat(129)); + assert_eq!(super::parse_gps(&input), None); + } + + #[test] + fn gps_rejects_invalid_dms_components() { + let p = super::parse_gps; + assert_eq!(p("45°60'0\"N, 7E"), None); + assert_eq!(p("45°0'60\"N, 7E"), None); + assert_eq!(p("45°-1'0\"N, 7E"), None); + assert_eq!(p("45°0'-1\"N, 7E"), None); + } + + #[test] + fn gps_rejected_text_does_not_change_metadata() { + let mut s = crate::Session::with_demo(); + let ids: Vec<_> = s.catalog.photos().take(2).map(|photo| photo.id).collect(); + assert_eq!(ids.len(), 2, "demo has two records for the batch check"); + let id_values: Vec<_> = ids.iter().map(|id| id.0).collect(); + s.execute("library.select", &json!({"ids": id_values})).unwrap(); + s.execute("photo.setMeta", &json!({"title": "Before"})).unwrap(); + s.execute("develop.set", &json!({"control": "light.exposure", "value": 0.25})).unwrap(); + s.execute("edit.undo", &json!({})).unwrap(); + assert!(!s.redo.is_empty(), "rejection must preserve a populated redo history"); + + let snapshot = s.catalog.to_snapshot(); + let revision = s.catalog.revision; + let selection = s.selection.clone(); + let commits = s.commits(); + let undo = format!("{:?}", s.undo); + let redo = format!("{:?}", s.redo); + let error = s.execute("photo.setMeta", &json!({"gps": "garbage45, 7junk", "title": "After"})).unwrap_err(); + + assert!(matches!(error, crate::EngineError::BadParams { ref cmd, .. } if cmd == "photo.setMeta"), "{error}"); + assert_eq!(s.catalog.to_snapshot(), snapshot, "catalog data is unchanged"); + assert_eq!(s.catalog.revision, revision, "catalog revision is unchanged"); + assert_eq!(s.selection, selection, "selection is unchanged"); + assert_eq!(s.commits(), commits, "commit count is unchanged"); + assert_eq!(format!("{:?}", s.undo), undo, "undo history is unchanged"); + assert_eq!(format!("{:?}", s.redo), redo, "redo history is unchanged"); + for id in ids { + assert_eq!(s.catalog.photo(id).map(|photo| photo.meta.title.as_str()), Some("Before")); + } + } + + #[test] + fn gps_valid_edit_is_one_undo_step() { + let mut s = crate::Session::with_demo(); + let id = s.active().or_else(|| s.catalog.photos().next().map(|photo| photo.id)).unwrap(); + let previous = s.catalog.photo(id).unwrap().meta.clone(); + let commits = s.commits(); + s.execute("photo.setMeta", &json!({"gps": "48°51'30\"N 2°17'40\"E", "title": "GPS edited"})).unwrap(); + let photo = s.catalog.photo(id).unwrap(); + assert_eq!(photo.meta.title, "GPS edited"); + assert!(photo.meta.gps.is_some()); + assert_eq!(s.commits(), commits + 1); + + s.execute("edit.undo", &json!({})).unwrap(); + let photo = s.catalog.photo(id).unwrap(); + assert_eq!(photo.meta.title, previous.title); + assert_eq!(photo.meta.gps, previous.gps); } } diff --git a/docs/gaps.md b/docs/gaps.md index ceea3724f..2ecd1d2de 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -1,6 +1,6 @@ # Gaps: where LightCraft falls short of Lightroom -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-23 · **Change:** minor (gap 13: before a Lightroom catalog import the app asks merge or new library, #2672; gap 28: F8 shows / hides the right panel; import review album hierarchy, #2362; gap 20: Minolta MRW decodes; gap 10: 551 models verified; gap 15: Print Page Options — page numbers, page info, crop marks; gap 28: `\` shows / hides the Filter Bar in the grids; gap 21: Indonesian interface shipped, every message but What's New; gap 27: the Windows x64 installer verified on Windows 11; the Settings → Apps uninstall confirmation remains; gap 28: ⌘L Enable Filters switches the filter off and on; gap 15: Japanese photo and postcard paper sizes in the Print page setup and PDF) +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-24 · **Change:** patch (typed GPS input now rejects malformed coordinates before metadata changes; no ranked-gap or readiness-count changes; earlier: gap 13: before a Lightroom catalog import the app asks merge or new library, #2672; gap 28: F8 shows / hides the right panel; import review album hierarchy, #2362; gap 20: Minolta MRW decodes; gap 10: 551 models verified; gap 15: Print Page Options — page numbers, page info, crop marks; gap 28: `\` shows / hides the Filter Bar in the grids; gap 21: Indonesian interface shipped, every message but What's New; gap 27: the Windows x64 installer verified on Windows 11; the Settings → Apps uninstall confirmation remains; gap 28: ⌘L Enable Filters switches the filter off and on; gap 15: Japanese photo and postcard paper sizes in the Print page setup and PDF) Every known shortfall, one entry each, ranked by user impact (how many Lightroom users hit it, and how hard it blocks them), then by effort. **Agents pick work from the top** (the first entry nobody is @@ -84,6 +84,7 @@ Beta-blocking gaps (B) total **~230–430 h**; with the stability and platform w | Date | Change | Summary | |---|---|---| +| 2026-10-24 | patch | Typed GPS input now validates the complete coordinate expression, rejecting unknown text, wrong-axis or conflicting hemispheres, and invalid DMS minutes/seconds before changing metadata; ranked gaps, estimates and readiness counts unchanged | | 2026-10-23 | minor | Gap 13: before a Lightroom catalog import the app asks whether it merges into the open library or goes into a new one (opt-out in Settings, #2672); hours unchanged | | 2026-10-22 | patch | Gap 28: View shows the current grid-cycle key beside Photo Grid and Square Grid (#2500); no new binding and no change to the remaining shortcut gap or hours | | 2026-10-22 | minor | Gap 15: the Print page setup and PDF gain the Japanese photo and postcard paper sizes (L判 89×127, 2L判 127×178, KG判 102×152, はがき 100×148, 往復はがき 148×200, 六切 203×254, 四切 254×305, 名刺 55×91 mm), by their customary names; gap not closed | diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index d059494ce..4392d6db9 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -1,6 +1,6 @@ # Lightroom parity checklist -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-24 · **Change:** minor (LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-24 · **Change:** patch (LR-META-LOCATION: Info GPS text is fully validated before metadata changes; readiness unchanged; earlier: LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 The measured, row-by-row checklist: one row per Lightroom feature, menu item and shortcut, what we have and what is missing. It answers *does the feature exist?* (feature breadth). Whether it is good @@ -166,7 +166,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). |---|---|---|---|---|---| | LR-META-INFO | Info panel | P0 | ✅ | `cmd:panel.info`, `cmd:photo.setMeta`, `crates/ui-egui/src/panels/right.rs` (`info`, `camera_card`) | camera card (camera, lens, size, format, focal length / shutter / aperture / ISO); title, caption, alt text, extended description, copyright, copyright status, rights usage terms, copyright info URL, creator; file name (rename), file path (reveal), capture time (edit); location, city, state, country; GPS. No flash, map snippet or people; a raw with no lens name (Nikon DSLRs) shows its range from Exif LensSpecification or the Nikon maker note's Lens tag, written like Lightroom's `24.0-70.0 mm f/2.8` | | LR-META-COPYRIGHT-DEFAULT | Default copyright on import | P1 | ✅ | `cmd:library.preferences` (`import.copyright`, `import.creator`), `crates/ui-egui/src/panels/settings.rs` | Settings → Import → Metadata; fills only photos without their own | -| LR-META-LOCATION | Location editing | P2 | 🟡 | `cmd:photo.setMeta` (`location`, `city`, `state`, `country`, `gps`), `cmd:metadata.reverseGeocode` | place fields and GPS (decimal or degrees / minutes / seconds) edited in Info, read/written as IPTC Core / Photoshop / EXIF XMP; Show on Map opens OpenStreetMap; Fill Location fills empty City / State / Country from GPS offline (GeoNames, downloaded on request: `cmd:metadata.downloadPlaceNames`); no embedded map | +| LR-META-LOCATION | Location editing | P2 | 🟡 | `cmd:photo.setMeta` (`location`, `city`, `state`, `country`, `gps`), `cmd:metadata.reverseGeocode` | place fields and GPS (decimal or degrees / minutes / seconds) edited in Info, read/written as IPTC Core / Photoshop / EXIF XMP; typed GPS consumes the full expression, accepts case-insensitive axis-appropriate hemisphere markers, and refuses malformed text or DMS minutes/seconds outside [0,60) before changing metadata; Show on Map opens OpenStreetMap; Fill Location fills empty City / State / Country from GPS offline (GeoNames, downloaded on request: `cmd:metadata.downloadPlaceNames`); no embedded map | | LR-META-COPYPASTE | Copy / paste metadata | P2 | ✅ | `cmd:photo.copyMetadata`, `cmd:photo.pasteMetadata` | title, caption, alt text, extended description, copyright (notice, status, usage terms, info URL), creator, place fields, keywords; `fields` picks a subset | | LR-META-XMP | XMP read/write | P0 | ✅ | `cmd:photo.saveMetadataToFile`, `cmd:photo.readMetadataFromFile`, `cmd:library.xmpPreferences`, `crates/engine/src/sidecar.rs`, `docs/xmp-interop.md` | manual rereading also reads sidecar-free JPEG embedded XMP (first complete standard APP1 before SOS/EOI, within 16 MiB of headers and 4096 marker/fill steps; no extended assembly or between-scan packets, not strict whole-JPEG validation; reader cases in `crates/engine/src/sidecar.rs`); a discovered unreadable sidecar fails without embedded fallback; procedural refresh/undo, non-UTF-8 sidecar and two explicit malformed fixtures in `crates/engine/src/tests_xmp.rs` (file-write checks use auto-write off); sidecar wins for metadata; a sidecar capture time (`exif:DateTimeOriginal` / `photoshop:DateCreated` / `xmp:CreateDate`) fills in only when the file has none; saving merges into an existing sidecar (`crates/meta/src/xmp_merge.rs`): another app's `crs:`, `xmpMM:History` and unknown namespaces are kept byte for byte (ownership follows namespace URI: foreign namespaces using `lc:` or `xmp:` cannot supply native settings or ratings and survive saving), an unreadable sidecar is backed up first; with stem naming, files sharing a stem (raw + JPEG) get separate sidecars (`.xmp` for all but the raw; an import ranks the files it is adding the same way, so a TIFF or JPEG arriving beside its raw reads its own `.xmp` and never the raw's `IMG_0001.xmp`, issue #1905); a `crs:` custom white balance on a relative-WB raw is shifted from `crs:AsShotTemperature` rather than read as Kelvin, or kept As Shot and reported when the packet has no as-shot white (issue #510); heal and clone spots (`crs:RetouchAreas`, legacy `crs:RetouchInfo`) are read into our spots (`crates/engine/src/crs_masks.rs`, see `docs/xmp-interop.md` → Spot removal); radial masks (`Mask/CircularGradient`) keep Lightroom's inside / outside: `crs:Flipped="true"` is the effect-inside flag of current masks and of the legacy Radial Filter, `crs:MaskInverted` the inversion, checked against Lightroom Classic's renders (issue #1815); Read Metadata from File never writes the sidecar back, even with auto-write on, so another app's later `crs:` edits are still read (issue #934); settings Lightroom writes only for itself (`CompatibleVersion`, the profile digests, the lens profile's name / file / setup, `CropConstrainToUnitSquare`, the lens profile's strength at 100 %) are not reported as unimported (`crates/engine/src/crs.rs`) | | LR-META-EXIF-FULL | Full EXIF/IPTC [Classic] | P1 | ✅ | `cmd:photo.allMetadata`, `cmd:dialog.allMetadata`, `crates/meta/src/tags.rs`, `crates/meta/src/exif.rs`, `crates/meta/src/iptc.rs` | `photo.allMetadata` reuses loaded original bytes for embedded XMP and keeps sidecar precedence (`crates/engine/src/tests_xmp.rs`); read and written on export; Info ▸ All Metadata…: every TIFF / EXIF / GPS / interop tag (named, common values spelled out) and the XMP fields, searchable; PSD/PSB EXIF resource 1058 and XMP resource 1060 feed the same metadata rows and preserve sidecar precedence; metadata presets | @@ -799,6 +799,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 2026-10-24 | patch | LR-META-LOCATION: Info GPS text validates the complete decimal/DMS expression, axis markers and component ranges before metadata changes; readiness and feature counts unchanged | | 2026-10-24 | patch | LR-LIB-RATING, LR-LIB-DELETE: a slider drag still open when a rating, flag or label takes the photo out of the view, or when the photo is deleted, ends as its own undo step before the selection moves on, as moving to the next photo does; LR-BEHAV-LOCALIZE lists all 18 languages; status unchanged | | 2026-10-24 | patch | Integration batch 43: Integrated main 98702b5c, preserving the new fail-closed GPS metadata check for Original export. Watermark geometry/orientation and HDR JPEG size-limit fixes remain in their shared helpers; no feature-count changes. | | 2026-10-23 | patch | Export integration: Integrated main 5ca49b3e, retaining Width & Height resizing, export format/bit-depth validation and batch panic cancellation alongside the existing watermark and HDR JPEG size-limit fixes. No feature-count changes. | diff --git a/docs/whats-new.md b/docs/whats-new.md index c6c259200..a423c3ad3 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -2,6 +2,9 @@ ## 2026-10-24 +### Typed GPS coordinates are validated before editing metadata +- Entered coordinates must contain a complete decimal or degrees/minutes/seconds value. Unknown text, exponent notation, wrong-axis or conflicting hemispheres, and minutes or seconds outside [0,60) are rejected instead of being silently ignored and reinterpreted. A rejected GPS value also leaves the other fields in the same metadata edit unchanged. + ### Relative-date filters and smart albums follow the clock - A current view or source count using “in the last” rules now refreshes when the session clock moves into a new minute, even when no catalog edit occurred. This fixes stale photos remaining in the grid and stale counts for From 64712276bcf5edb2b154cddce3be8866a4cfcb4c Mon Sep 17 00:00:00 2001 From: ronibalthazar <61232335+ronibalthazar@users.noreply.github.com> Date: Sat, 24 Oct 2026 17:33:50 +0100 Subject: [PATCH 2/5] LR-META-LOCATION: reject ambiguous whitespace GPS --- crates/engine/src/cmd/library.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index 82070f498..af888efda 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -1851,13 +1851,19 @@ pub(crate) fn parse_gps(t: &str) -> Option<(f64, f64)> { // Without hemisphere letters, a comma-free pair is separated by whitespace. Trying each // boundary also permits spaces between DMS components without letting unconsumed text vanish. + // If multiple boundaries parse, they must agree so ambiguous text cannot be reinterpreted. + let mut parsed: Option<(f64, f64)> = None; for (at, separator) in t.char_indices().filter(|(_, c)| c.is_whitespace()) { let after = at.checked_add(separator.len_utf8())?; if let Some(pair) = parse_gps_pair(t.get(..at)?, t.get(after..)?) { - return Some(pair); + match parsed { + Some(previous) if previous != pair => return None, + None => parsed = Some(pair), + Some(_) => {} + } } } - None + parsed } #[derive(Clone, Copy)] @@ -1996,6 +2002,8 @@ mod gps_tests { fn coordinates_people_type() { let p = super::parse_gps; assert_eq!(p("51.5072, -0.1276"), Some((51.5072, -0.1276))); + assert_eq!(p("10 20 30"), None); + assert_eq!(p("10 20"), Some((10.0, 20.0))); let (la, lo) = p("51°30'26\"N 0°7'39\"W").unwrap(); assert!((la - 51.50722).abs() < 1e-4 && (lo + 0.1275).abs() < 1e-4, "{la} {lo}"); let (la, lo) = p("33.8688 S, 151.2093 E").unwrap(); From 96d7a38b1ffb203fea69d4f44dccd2195ef8f59d Mon Sep 17 00:00:00 2001 From: ronibalthazar <61232335+ronibalthazar@users.noreply.github.com> Date: Sat, 24 Oct 2026 18:19:28 +0100 Subject: [PATCH 3/5] LR-META-LOCATION: check prefix GPS split ambiguity --- crates/engine/src/cmd/library.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index af888efda..7ba7efc41 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -1837,11 +1837,12 @@ pub(crate) fn parse_gps(t: &str) -> Option<(f64, f64)> { // A suffix N/S marks the end of latitude; a prefix N/S uses the next hemisphere prefix as // the boundary. If only longitude has a hemisphere, the whitespace pass below handles it. + let mut parsed: Option<(f64, f64)> = None; let mut latitude_markers = t.char_indices().filter(|(_, c)| matches!(c, 'N' | 'S' | 'n' | 's')); if let Some((first, marker)) = latitude_markers.next() { if first == 0 { if let Some((second, _)) = t.char_indices().skip(1).find(|(_, c)| is_gps_hemisphere(*c)) { - return parse_gps_pair(t.get(..second)?, t.get(second..)?); + parsed = Some(parse_gps_pair(t.get(..second)?, t.get(second..)?)?); } } else { let end = first.checked_add(marker.len_utf8())?; @@ -1852,7 +1853,6 @@ pub(crate) fn parse_gps(t: &str) -> Option<(f64, f64)> { // Without hemisphere letters, a comma-free pair is separated by whitespace. Trying each // boundary also permits spaces between DMS components without letting unconsumed text vanish. // If multiple boundaries parse, they must agree so ambiguous text cannot be reinterpreted. - let mut parsed: Option<(f64, f64)> = None; for (at, separator) in t.char_indices().filter(|(_, c)| c.is_whitespace()) { let after = at.checked_add(separator.len_utf8())?; if let Some(pair) = parse_gps_pair(t.get(..at)?, t.get(after..)?) { From 647ed93df259822ed4884146f2aee41e627ae0af Mon Sep 17 00:00:00 2001 From: ronibalthazar <61232335+ronibalthazar@users.noreply.github.com> Date: Sat, 24 Oct 2026 18:20:38 +0100 Subject: [PATCH 4/5] LR-META-LOCATION: clarify GPS split handling --- crates/engine/src/cmd/library.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index 7ba7efc41..ee6fecb9e 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -1850,9 +1850,9 @@ pub(crate) fn parse_gps(t: &str) -> Option<(f64, f64)> { } } - // Without hemisphere letters, a comma-free pair is separated by whitespace. Trying each - // boundary also permits spaces between DMS components without letting unconsumed text vanish. - // If multiple boundaries parse, they must agree so ambiguous text cannot be reinterpreted. + // A comma-free pair can be separated by whitespace. Trying each boundary also permits spaces + // between DMS components without letting unconsumed text vanish. If multiple boundaries parse, + // they must agree so ambiguous text cannot be reinterpreted. for (at, separator) in t.char_indices().filter(|(_, c)| c.is_whitespace()) { let after = at.checked_add(separator.len_utf8())?; if let Some(pair) = parse_gps_pair(t.get(..at)?, t.get(after..)?) { From 6ea7906109b263906ffc385ddd5fba42a6a2adc3 Mon Sep 17 00:00:00 2001 From: ronibalthazar <61232335+ronibalthazar@users.noreply.github.com> Date: Sat, 24 Oct 2026 20:04:04 +0100 Subject: [PATCH 5/5] LR-META-LOCATION: fall back to whitespace GPS split --- crates/engine/src/cmd/library.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index ee6fecb9e..306e7a362 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -1841,8 +1841,10 @@ pub(crate) fn parse_gps(t: &str) -> Option<(f64, f64)> { let mut latitude_markers = t.char_indices().filter(|(_, c)| matches!(c, 'N' | 'S' | 'n' | 's')); if let Some((first, marker)) = latitude_markers.next() { if first == 0 { - if let Some((second, _)) = t.char_indices().skip(1).find(|(_, c)| is_gps_hemisphere(*c)) { - parsed = Some(parse_gps_pair(t.get(..second)?, t.get(second..)?)?); + if let Some((second, _)) = t.char_indices().skip(1).find(|(_, c)| is_gps_hemisphere(*c)) + && let Some(pair) = parse_gps_pair(t.get(..second)?, t.get(second..)?) + { + parsed = Some(pair); } } else { let end = first.checked_add(marker.len_utf8())?; @@ -2039,6 +2041,11 @@ mod gps_tests { assert_eq!(p("45N 7NE"), None); } + #[test] + fn gps_tries_whitespace_split_when_prefix_marker_split_fails() { + assert_eq!(super::parse_gps("N51 0E"), Some((51.0, 0.0))); + } + #[test] fn gps_rejects_unconsumed_coordinate_text() { assert_eq!(super::parse_gps("garbage45, 7junk"), None);