From 09890b0afc1bdeaeb71feae0678bfa9e2ad2975c Mon Sep 17 00:00:00 2001 From: ronibalthazar <61232335+ronibalthazar@users.noreply.github.com> Date: Thu, 22 Oct 2026 08:07:32 +0100 Subject: [PATCH] LR-MAP-TRACKLOG: decode GPX character references --- crates/engine/src/tests_libops.rs | 35 +++++++++++++++ crates/meta/src/gpx.rs | 75 +++++++++++++++++++++++++++++-- docs/gaps.md | 6 ++- docs/parity-checklist.md | 5 ++- docs/whats-new.md | 8 ++++ 5 files changed, 123 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/tests_libops.rs b/crates/engine/src/tests_libops.rs index b35167c1b..05eb5dbb8 100644 --- a/crates/engine/src/tests_libops.rs +++ b/crates/engine/src/tests_libops.rs @@ -467,6 +467,41 @@ fn tracklog_auto_tag() { let _ = std::fs::remove_dir_all(&dir); } +#[test] +fn tracklog_auto_tag_decodes_character_references_and_undoes_once() { + use lightcraft_catalog::Op; + + const PLAIN: &str = r#" + + + "#; + const ENCODED: &str = r#" + + + "#; + + let mut s = Session::with_demo(); + let Some(id) = s.visible_cloned().first().copied() else { + panic!("the demo library has a photo"); + }; + s.commit("test capture time", Op::SetCaptured { id, captured: Some("2026-01-01T00:01:00".into()) }).unwrap(); + let original_gps = s.catalog.photo(id).unwrap().meta.gps; + let params = |gpx: &str, dry_run| json!({"gpx": gpx, "ids": [id.0], "dryRun": dry_run}); + + let plain = s.execute("photo.autoTagTracklog", ¶ms(PLAIN, true)).unwrap(); + let encoded = s.execute("photo.autoTagTracklog", ¶ms(ENCODED, true)).unwrap(); + assert_eq!(plain, encoded); + assert_eq!(encoded["photos"][0]["gps"], json!([46.0, 8.0])); + assert_eq!(s.catalog.photo(id).unwrap().meta.gps, original_gps, "dry run leaves GPS unchanged"); + + let undo_depth = s.undo.len(); + s.execute("photo.autoTagTracklog", ¶ms(ENCODED, false)).unwrap(); + assert_eq!(s.undo.len(), undo_depth + 1); + assert_eq!(s.catalog.photo(id).unwrap().meta.gps, Some((46.0, 8.0))); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(s.catalog.photo(id).unwrap().meta.gps, original_gps); +} + // ---- random sort: Given the library, When the user picks Random, Then the grid is a stable shuffle #[test] diff --git a/crates/meta/src/gpx.rs b/crates/meta/src/gpx.rs index 204e39116..817d59d65 100644 --- a/crates/meta/src/gpx.rs +++ b/crates/meta/src/gpx.rs @@ -118,10 +118,15 @@ pub fn parse_gpx(s: &str) -> Result { b"trkpt" if parent == Some(b"trkseg") => { let mut p = Pending::default(); for a in e.attributes().with_checks(false).flatten() { - let v = String::from_utf8_lossy(&a.value); match local(a.key.as_ref()) { - b"lat" => p.lat = v.trim().parse().ok(), - b"lon" => p.lon = v.trim().parse().ok(), + b"lat" => { + let v = a.decode_and_unescape_value(reader.decoder()).map_err(|e| GpxError::Xml(e.to_string()))?; + p.lat = v.trim().parse().ok(); + } + b"lon" => { + let v = a.decode_and_unescape_value(reader.decoder()).map_err(|e| GpxError::Xml(e.to_string()))?; + p.lon = v.trim().parse().ok(); + } _ => {} } } @@ -141,6 +146,16 @@ pub fn parse_gpx(s: &str) -> Result { } Event::Text(t) => text.push_str(&t.decode().map_err(|e| GpxError::Xml(e.to_string()))?), Event::CData(t) => text.push_str(&String::from_utf8_lossy(&t)), + Event::GeneralRef(r) => { + if let Some(c) = r.resolve_char_ref().map_err(|e| GpxError::Xml(e.to_string()))? { + text.push(c); + } else { + let name = r.decode().map_err(|e| GpxError::Xml(e.to_string()))?; + let entity = format!("&{name};"); + let value = quick_xml::escape::unescape(&entity).map_err(|e| GpxError::Xml(e.to_string()))?; + text.push_str(value.as_ref()); + } + } Event::End(_) => { let Some(name) = path.pop() else { continue }; let parent = path.last().map(Vec::as_slice); @@ -351,6 +366,60 @@ mod tests { assert_eq!(log.points[1].time - log.points[0].time, 60.0); } + fn point_document(lat: &str, lon: &str, ele: &str, time: &str) -> String { + format!( + r#"{ele}"# + ) + } + + #[test] + fn character_reference_elevation_is_not_truncated() { + let log = parse_gpx(&point_document("45", "7", "123", "2026-01-01T00:00:00Z")).unwrap(); + assert_eq!(log.points.first().and_then(|p| p.elevation), Some(123.0)); + } + + #[test] + fn character_reference_latitude_is_not_dropped() { + let log = parse_gpx(&point_document("45", "7.5", "123", "2026-01-01T00:00:00Z")).unwrap(); + let point = log.points.first().copied(); + assert_eq!(log.points.len(), 1); + assert_eq!(point.map(|p| (p.latitude, p.longitude)), Some((45.0, 7.5))); + } + + #[test] + fn character_reference_time_is_not_shifted() { + let encoded = parse_gpx(&point_document("45", "7", "123", "2026-01-01T00:00:30Z")).unwrap(); + let plain = parse_gpx(&point_document("45", "7", "123", "2026-01-01T00:00:30Z")).unwrap(); + assert_eq!(encoded.points.first().map(|p| p.time), plain.points.first().map(|p| p.time)); + assert_eq!(encoded.points.first().map(|p| p.time), Some(1_767_225_630.0)); + } + + #[test] + fn predefined_entities_are_decoded_once_and_cdata_stays_literal() { + let with_predefined_entity = point_document("45", "7", "1&23", "2026-01-01T00:00:00Z"); + let log = parse_gpx(&with_predefined_entity).unwrap(); + assert_eq!(log.points.first().and_then(|p| p.elevation), None); + + let cdata = point_document("45", "7", "", "2026-01-01T00:00:00Z"); + let log = parse_gpx(&cdata).unwrap(); + assert_eq!(log.points.first().and_then(|p| p.elevation), None); + } + + #[test] + fn unknown_and_malformed_character_references_are_rejected() { + let unknown = point_document("45", "7", "&undeclared;", "2026-01-01T00:00:00Z"); + assert!(matches!(parse_gpx(&unknown), Err(GpxError::Xml(_)))); + + let malformed = point_document("45", "7", "&#xZZ;", "2026-01-01T00:00:00Z"); + assert!(matches!(parse_gpx(&malformed), Err(GpxError::Xml(_)))); + + let unknown_attribute = point_document("4&undeclared;", "7", "123", "2026-01-01T00:00:00Z"); + assert!(matches!(parse_gpx(&unknown_attribute), Err(GpxError::Xml(_)))); + + let malformed_attribute = point_document("&#xZZ;", "7", "123", "2026-01-01T00:00:00Z"); + assert!(matches!(parse_gpx(&malformed_attribute), Err(GpxError::Xml(_)))); + } + #[test] fn non_finite_elevation_is_dropped() { for ele in ["inf", "-inf", "NaN", "infinity"] { diff --git a/docs/gaps.md b/docs/gaps.md index be411a205..a258a1eb4 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-21 · **Change:** minor (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 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) +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-22 · **Change:** patch (LR-MAP-TRACKLOG decodes GPX character references in coordinates, elevations and timestamps; malformed references error; CDATA stays literal; ranked gaps unchanged) 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 @@ -60,6 +60,9 @@ Beta-blocking gaps (B) total **~230–430 h**; with the stability and platform w ## Closed recently (kept for calibration) +- 2026-10-22: GPX track points decode XML character references in coordinate attributes and + element text before parsing. Malformed or undeclared references return an error; CDATA stays + literal. Track positions and timestamps no longer silently change or lose points. - 2026-10-20: the import review album picker follows the sidebar folder hierarchy and searches album paths (#2362, LR-IMP-ADD-DIALOG), retaining None/New album and existing destination IDs. This closes a usability subgap; no ranked gap or headline feature count closes. - 2026-10-12: original-pixel detail at 100% (gap 4, LR-VIEW-ZOOM, #323): soft proofing gets zoom windows like @@ -79,6 +82,7 @@ Beta-blocking gaps (B) total **~230–430 h**; with the stability and platform w | Date | Change | Summary | |---|---|---| +| 2026-10-22 | patch | LR-MAP-TRACKLOG: GPX coordinate attributes and point text decode XML references; malformed / unknown references return errors; CDATA remains literal; ranked gaps unchanged | | 2026-10-21 | minor | Gap 28 shrinks: F8 shows / hides the right panel (`view.rightPanel`); rating `[` `]`, flag cycling and F1 help were already bound, so the entry lists only filter-bar keys, Tab / ⇧Tab and F5; 2–4 h (was 4–8) | | 2026-10-20 | minor | #2362: import destination albums use the shared hierarchy/search picker instead of a flat name list; existing import workflows retained; ranked gaps/readiness counts unchanged | | 2026-10-19 | minor | Gap 21: Indonesian (`id`) shipped with every catalog message but What's New; four new languages left (Hindi, Arabic, Korean, Vietnamese), 135–239 h | diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 99f17561e..f891f3e65 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-21 · **Change:** minor (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-22 · **Change:** patch (LR-MAP-TRACKLOG decodes GPX character references in point fields; malformed references error; CDATA stays literal) · **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 @@ -736,7 +736,7 @@ From `08-lightroom-classic-extras.md` (Classic-only features) and part 2 of `06- | LRC-MAP-VIEW | Map view | P2 | ⬜ | | | | LRC-MAP-GEOTAG | Drag photos onto the map | P2 | ⬜ | | | | LRC-MAP-LOCATIONS | Saved locations | P2 | ⬜ | | | -| LRC-MAP-TRACKLOG | GPS track logs | P2 | ✅ | `cmd:photo.autoTagTracklog`, `cmd:photo.tagFromTracklog`, `crates/meta/src/gpx.rs` | Photo ▸ Auto-Tag from Tracklog…: a GPX 1.0 / 1.1 track log sets the GPS of the selected photos by capture time — interpolated between the points of a track segment, else the nearest point within `maxGap` (10 min); never across segment breaks. The camera's time zone comes from the photo (Exif offset) or is asked for; photos that already have a location keep it unless `replace`; `dryRun` previews; one undo step. No track drawn on a map (no Map module) | +| LRC-MAP-TRACKLOG | GPS track logs | P2 | ✅ | `cmd:photo.autoTagTracklog`, `cmd:photo.tagFromTracklog`, `crates/meta/src/gpx.rs` | Photo ▸ Auto-Tag from Tracklog…: a GPX 1.0 / 1.1 track log sets the GPS of the selected photos by capture time — interpolated between the points of a track segment, else the nearest point within `maxGap` (10 min); never across segment breaks. XML character references in coordinate attributes and point text decode before parsing; malformed or unknown references return an error; CDATA stays literal. The camera's time zone comes from the photo (Exif offset) or is asked for; photos that already have a location keep it unless `replace`; `dryRun` previews; one undo step. No track drawn on a map (no Map module) | | LRC-MAP-FILTER | Location filter bar | P2 | ⬜ | | | | LRC-MAP-REVGEO | Reverse geocoding | P2 | 🟡 | `cmd:metadata.reverseGeocode` | offline from GeoNames (City / State / Country, no sublocation); the list holds towns of 15 000 people or more, so a smaller town gets the nearest larger one (the tooltip and command help say so); `maxDistance` leaves City empty past a distance and names the town as `nearestCity` (issue #2242); no online provider | | LRC-BOOK-SETTINGS | Book settings | P2 | ⬜ | | | @@ -798,6 +798,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 2026-10-22 | patch | LR-MAP-TRACKLOG: character references in GPX coordinate attributes and point text decode correctly; malformed / unknown references fail cleanly; CDATA stays literal; command dry-run, tagging and undo covered | | 2026-10-21 | patch | LR-IMP-ADD-DIALOG: reconcile #2362 with current main; deleted-album guard and hierarchical picker retained; counts unchanged | | 2026-10-20 | minor | LR-IMP-ADD-DIALOG: #2362 gives the import review the shared hierarchy/search album picker while retaining None/New album and existing destination IDs; status and feature counts unchanged | | 2026-10-21 | patch | KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the slider's row; leaving it (even between two frames) or Tab focus alone does not arm them (issue #1355); status unchanged | diff --git a/docs/whats-new.md b/docs/whats-new.md index cffff83e2..892cd335c 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,13 @@ # What's new in LightCraft +## 2026-10-22 + +### GPX track logs read escaped values correctly +- Coordinates, elevations and times in a GPX track can contain XML character references such as + `5`. LightCraft now decodes those characters before parsing values, reports malformed or + undeclared references as an error, and keeps CDATA literal. Auto-Tag places photos at the track + position represented by their capture time. + ## 2026-10-21 ### Sharper resampling in HDR and panorama merges