Merge pull request #860 from Kitware/segment-group-zip-names-slicer-io

PaulHax · web-flow · commit 5b2b4b93f5a9 · 2026-04-16T20:35:43.000Z
Use segment group name as filename in volview.zip
diff --git a/src/components/SaveSegmentGroupDialog.vue b/src/components/SaveSegmentGroupDialog.vue
@@ -7,7 +7,7 @@
       <v-form v-model="valid" @submit.prevent="saveSegmentGroup">
         <v-text-field
           v-model="fileName"
-          hint="Filename that will appear in downloads."
+          hint="Filename used for downloads."
           label="Filename"
           :rules="[validFileName]"
           required
@@ -37,12 +37,13 @@
 </template>
 
 <script setup lang="ts">
-import { onMounted, ref } from 'vue';
+import { computed, onMounted, ref } from 'vue';
 import { onKeyDown } from '@vueuse/core';
 import { saveAs } from 'file-saver';
 import { useSegmentGroupStore } from '@/src/store/segmentGroups';
 import { writeSegmentation } from '@/src/io/readWriteImage';
 import { useErrorMessage } from '@/src/composables/useErrorMessage';
+import { sanitizeSegmentGroupFileStem } from '@/src/io/state-file/segmentGroupArchivePath';
 
 const EXTENSIONS = [
   'seg.nrrd',
@@ -63,12 +64,18 @@ const props = defineProps<{
 
 const emit = defineEmits(['done']);
 
-const fileName = ref('');
+const fileNameValue = ref('');
 const valid = ref(true);
 const saving = ref(false);
 const fileFormat = ref(EXTENSIONS[0]);
 
 const segmentGroupStore = useSegmentGroupStore();
+const fileName = computed({
+  get: () => fileNameValue.value,
+  set: (value: string) => {
+    fileNameValue.value = sanitizeSegmentGroupFileStem(value, '');
+  },
+});
 
 async function saveSegmentGroup() {
   if (fileName.value.trim().length === 0) {
@@ -77,20 +84,24 @@ async function saveSegmentGroup() {
 
   saving.value = true;
   await useErrorMessage('Failed to save segment group', async () => {
+    const sanitizedFileName = sanitizeSegmentGroupFileStem(fileName.value);
+    fileNameValue.value = sanitizedFileName;
     const serialized = await writeSegmentation(
       fileFormat.value,
       segmentGroupStore.dataIndex[props.id],
       segmentGroupStore.metadataByID[props.id]
     );
-    saveAs(new Blob([serialized]), `${fileName.value}.${fileFormat.value}`);
+    saveAs(new Blob([serialized]), `${sanitizedFileName}.${fileFormat.value}`);
   });
   saving.value = false;
   emit('done');
 }
 
 onMounted(() => {
   // trigger form validation check so can immediately save with default value
-  fileName.value = segmentGroupStore.metadataByID[props.id].name;
+  fileNameValue.value = sanitizeSegmentGroupFileStem(
+    segmentGroupStore.metadataByID[props.id].name
+  );
 });
 
 onKeyDown('Enter', () => {
diff --git a/src/components/SegmentGroupControls.vue b/src/components/SegmentGroupControls.vue
@@ -335,6 +335,7 @@ function deleteSelected() {
             </v-tooltip>
           </v-btn>
           <v-btn
+            data-testid="segment-group-save-button"
             icon="mdi-content-save"
             size="small"
             variant="text"
diff --git a/src/composables/__tests__/useKeyboardShortcuts.spec.ts b/src/composables/__tests__/useKeyboardShortcuts.spec.ts
@@ -0,0 +1,25 @@
+import { describe, expect, it } from 'vitest';
+import { shouldIgnoreKeyboardShortcuts } from '../useKeyboardShortcuts';
+
+describe('shouldIgnoreKeyboardShortcuts', () => {
+  it('ignores shortcuts while an input is focused', () => {
+    const input = document.createElement('input');
+    expect(shouldIgnoreKeyboardShortcuts(input)).toBe(true);
+  });
+
+  it('ignores shortcuts while a textarea is focused', () => {
+    const textarea = document.createElement('textarea');
+    expect(shouldIgnoreKeyboardShortcuts(textarea)).toBe(true);
+  });
+
+  it('ignores shortcuts while a contenteditable element is focused', () => {
+    const editable = document.createElement('div');
+    editable.contentEditable = 'true';
+    expect(shouldIgnoreKeyboardShortcuts(editable)).toBe(true);
+  });
+
+  it('does not ignore shortcuts for non-editable controls', () => {
+    const button = document.createElement('button');
+    expect(shouldIgnoreKeyboardShortcuts(button)).toBe(false);
+  });
+});
diff --git a/src/composables/useKeyboardShortcuts.ts b/src/composables/useKeyboardShortcuts.ts
@@ -7,6 +7,19 @@ import { ACTION_TO_FUNC } from './actions';
 
 export const actionToKey = ref(ACTION_TO_KEY);
 
+export function shouldIgnoreKeyboardShortcuts(
+  activeElement: Element | null = document.activeElement
+) {
+  if (!(activeElement instanceof HTMLElement)) {
+    return false;
+  }
+
+  return (
+    activeElement.isContentEditable ||
+    activeElement.closest('input, textarea, select, [role="textbox"]') !== null
+  );
+}
+
 export function useKeyboardShortcuts() {
   const keys = useMagicKeys();
   let unwatchFuncs = [] as Array<ReturnType<typeof whenever>>;
@@ -21,6 +34,10 @@ export function useKeyboardShortcuts() {
         const lastKey = individualKeys[individualKeys.length - 1];
 
         return whenever(keys[key], () => {
+          if (shouldIgnoreKeyboardShortcuts()) {
+            return;
+          }
+
           const shiftPressed = keys.current.has('shift');
           const lastPressedKey = Array.from(keys.current).pop();
           const currentKeyWithCase = shiftPressed
diff --git a/src/io/__tests__/fileName.spec.ts b/src/io/__tests__/fileName.spec.ts
@@ -0,0 +1,32 @@
+import { DEFAULT_FILE_STEM, sanitizeFileStem } from '@/src/io/fileName';
+import { describe, expect, it } from 'vitest';
+
+describe('io/fileName', () => {
+  describe('sanitizeFileStem', () => {
+    it('replaces invalid filename characters with readable spacing', () => {
+      expect(sanitizeFileStem('Liver: left/right*?')).to.equal(
+        'Liver left right'
+      );
+    });
+
+    it('collapses repeated whitespace and trims trailing dots and spaces', () => {
+      expect(sanitizeFileStem('  Liver    left.   ')).to.equal('Liver left');
+    });
+
+    it('handles reserved Windows filenames', () => {
+      expect(sanitizeFileStem('CON')).to.equal('CON_');
+    });
+
+    it('falls back when the sanitized stem would be empty', () => {
+      expect(sanitizeFileStem('  ..../\\\\****   ')).to.equal(
+        DEFAULT_FILE_STEM
+      );
+    });
+
+    it('preserves already-valid names', () => {
+      expect(sanitizeFileStem('Prostate Segmentation')).to.equal(
+        'Prostate Segmentation'
+      );
+    });
+  });
+});
diff --git a/src/io/dicom.ts b/src/io/dicom.ts
@@ -47,11 +47,23 @@ async function runTask(
   inputs: any[],
   outputs: any[]
 ) {
-  return runPipeline(module, args, outputs, inputs, {
+  const result = await runPipeline(module, args, outputs, inputs, {
     webWorker: getWorker(),
     pipelineBaseUrl: itkConfig.pipelinesUrl,
     pipelineWorkerUrl: itkConfig.pipelineWorkerUrl,
+  }).catch((error) => {
+    throw new Error(
+      `itk-wasm pipeline "${module}" crashed (check browser console for details)`,
+      { cause: error }
+    );
   });
+  if (result.returnValue !== 0) {
+    const detail = result.stderr?.trim() || 'unknown error';
+    throw new Error(
+      `itk-wasm pipeline "${module}" exited with code ${result.returnValue}: ${detail}`
+    );
+  }
+  return result;
 }
 
 /**
diff --git a/src/io/fileName.ts b/src/io/fileName.ts
@@ -0,0 +1,37 @@
+const TRAILING_DOTS_AND_SPACES = /[. ]+$/g;
+const REPEATED_WHITESPACE = /\s+/g;
+const WINDOWS_RESERVED_FILE_NAME = /^(con|prn|aux|nul|com[1-9]|lpt[1-9])$/i;
+const INVALID_FILE_STEM_CHARS = new Set([
+  '<',
+  '>',
+  ':',
+  '"',
+  '/',
+  '\\',
+  '|',
+  '?',
+  '*',
+]);
+
+export const DEFAULT_FILE_STEM = 'File';
+
+export function sanitizeFileStem(name: string, fallback = DEFAULT_FILE_STEM) {
+  let sanitized = name
+    .split('')
+    .map((char) => {
+      const isControlCharacter = char.charCodeAt(0) < 32;
+      return isControlCharacter || INVALID_FILE_STEM_CHARS.has(char)
+        ? ' '
+        : char;
+    })
+    .join('')
+    .replace(REPEATED_WHITESPACE, ' ')
+    .trim()
+    .replace(TRAILING_DOTS_AND_SPACES, '');
+
+  if (WINDOWS_RESERVED_FILE_NAME.test(sanitized)) {
+    sanitized = `${sanitized}_`;
+  }
+
+  return sanitized || fallback;
+}
diff --git a/src/io/import/processors/handleDicomFile.ts b/src/io/import/processors/handleDicomFile.ts
@@ -6,6 +6,7 @@ import { ReadDicomTagsFunction } from '@/src/core/streaming/dicom/dicomMetaLoade
 import { ImportHandler, asIntermediateResult } from '@/src/io/import/common';
 import { getWorker } from '@/src/io/itk/worker';
 import { FILE_EXT_TO_MIME } from '@/src/io/mimeTypes';
+import { getErrorDetail } from '@/src/utils';
 import { readDicomTags } from '@itk-wasm/dicom';
 
 /**
@@ -22,8 +23,18 @@ const handleDicomFile: ImportHandler = async (dataSource) => {
   }
 
   const readTags: ReadDicomTagsFunction = async (file) => {
-    const result = await readDicomTags(file, { webWorker: getWorker() });
-    return result.tags;
+    try {
+      const result = await readDicomTags(file, { webWorker: getWorker() });
+      return result.tags;
+    } catch (error) {
+      const detail = getErrorDetail(
+        error,
+        'the file could not be parsed as valid DICOM (check browser console for details)'
+      );
+      throw new Error(`Failed to read DICOM tags: ${detail}`, {
+        cause: error,
+      });
+    }
   };
 
   const metaLoader = new DicomFileMetaLoader(dataSource.file, readTags);
diff --git a/src/io/import/processors/handleDicomStream.ts b/src/io/import/processors/handleDicomStream.ts
@@ -14,6 +14,7 @@ import {
 } from '@/src/io/import/common';
 import { getWorker } from '@/src/io/itk/worker';
 import { FILE_EXT_TO_MIME } from '@/src/io/mimeTypes';
+import { getErrorDetail } from '@/src/utils';
 import { readDicomTags } from '@itk-wasm/dicom';
 import { Tags } from '@/src/core/dicomTags';
 import { useMessageStore } from '@/src/store/messages';
@@ -34,8 +35,13 @@ const handleDicomStream: ImportHandler = async (dataSource) => {
       const result = await readDicomTags(file, { webWorker: getWorker() });
       return result.tags;
     } catch (error) {
+      const detail = getErrorDetail(
+        error,
+        'the file could not be parsed as valid DICOM (check browser console for details)'
+      );
       throw new Error(
-        `Failed to read DICOM tags from ${dataSource.uri}: ${error}`
+        `Failed to read DICOM tags from ${dataSource.uri}: ${detail}`,
+        { cause: error }
       );
     }
   };
diff --git a/src/io/state-file/__tests__/segmentGroupArchivePath.spec.ts b/src/io/state-file/__tests__/segmentGroupArchivePath.spec.ts
@@ -0,0 +1,25 @@
+import { makeSegmentGroupArchivePath } from '@/src/io/state-file/segmentGroupArchivePath';
+import { describe, expect, it } from 'vitest';
+
+describe('io/state-file/segmentGroupArchivePath', () => {
+  describe('makeSegmentGroupArchivePath', () => {
+    it('uses a sanitized segment group stem in the archive path', () => {
+      const usedPaths = new Set<string>();
+
+      expect(
+        makeSegmentGroupArchivePath('Liver: left/right*?', 'vti', usedPaths)
+      ).to.equal('segmentations/Liver left right.vti');
+    });
+
+    it('deduplicates colliding sanitized names case-insensitively', () => {
+      const usedPaths = new Set<string>();
+
+      expect(
+        makeSegmentGroupArchivePath('Liver/Left', 'vti', usedPaths)
+      ).to.equal('segmentations/Liver Left.vti');
+      expect(
+        makeSegmentGroupArchivePath('liver:left', 'vti', usedPaths)
+      ).to.equal('segmentations/liver left (2).vti');
+    });
+  });
+});
diff --git a/src/io/state-file/segmentGroupArchivePath.ts b/src/io/state-file/segmentGroupArchivePath.ts
@@ -0,0 +1,36 @@
+import { normalize } from '@/src/utils/path';
+import { sanitizeFileStem } from '@/src/io/fileName';
+
+export const DEFAULT_SEGMENT_GROUP_ARCHIVE_STEM = 'Segment Group';
+const SEGMENT_GROUP_ARCHIVE_DIR = 'segmentations';
+
+export function sanitizeSegmentGroupFileStem(
+  name: string,
+  fallback = DEFAULT_SEGMENT_GROUP_ARCHIVE_STEM
+) {
+  return sanitizeFileStem(name, fallback);
+}
+
+function makeArchivePathKey(path: string) {
+  return normalize(path).toLowerCase();
+}
+
+export function makeSegmentGroupArchivePath(
+  name: string,
+  extension: string,
+  usedPaths: Set<string>
+) {
+  const stem = sanitizeSegmentGroupFileStem(name);
+
+  let index = 1;
+  let path = normalize(`${SEGMENT_GROUP_ARCHIVE_DIR}/${stem}.${extension}`);
+  while (usedPaths.has(makeArchivePathKey(path))) {
+    index += 1;
+    path = normalize(
+      `${SEGMENT_GROUP_ARCHIVE_DIR}/${stem} (${index}).${extension}`
+    );
+  }
+
+  usedPaths.add(makeArchivePathKey(path));
+  return path;
+}
diff --git a/src/store/segmentGroups.ts b/src/store/segmentGroups.ts
@@ -26,6 +26,7 @@ import {
   SegmentGroupMetadata,
   SegmentGroup,
 } from '../io/state-file/schema';
+import { makeSegmentGroupArchivePath } from '../io/state-file/segmentGroupArchivePath';
 import { FileEntry } from '../io/types';
 import { ensureSameSpace } from '../io/resample/resample';
 import { untilLoaded } from '../composables/untilLoaded';
@@ -458,6 +459,7 @@ export const useSegmentGroupStore = defineStore('segmentGroup', () => {
    */
   async function serialize(state: StateFile) {
     const { zip } = state;
+    const usedArchivePaths = new Set<string>();
 
     // orderByParent is implicitly preserved based on
     // the order of serialized entries.
@@ -469,7 +471,11 @@ export const useSegmentGroupStore = defineStore('segmentGroup', () => {
         const metadata = metadataByID[id];
         return {
           id,
-          path: `labels/${id}.${saveFormat.value}`,
+          path: makeSegmentGroupArchivePath(
+            metadata.name,
+            saveFormat.value,
+            usedArchivePaths
+          ),
           metadata: {
             ...metadata,
             parentImage: metadata.parentImage,
diff --git a/src/utils/index.ts b/src/utils/index.ts
@@ -215,6 +215,10 @@ export function ensureError(e: unknown) {
   return e instanceof Error ? e : new Error(JSON.stringify(e));
 }
 
+export function getErrorDetail(error: unknown, fallback: string): string {
+  return error instanceof Error && error.message ? error.message : fallback;
+}
+
 // remove undefined properties
 export function cleanUndefined(obj: Object) {
   return Object.entries(obj).reduce(
diff --git a/tests/pageobjects/volview.page.ts b/tests/pageobjects/volview.page.ts
diff --git a/tests/specs/segment-group-download.e2e.ts b/tests/specs/segment-group-download.e2e.ts
diff --git a/tests/specs/session-large-uri-base.e2e.ts b/tests/specs/session-large-uri-base.e2e.ts
diff --git a/tests/specs/session-state-lifecycle.e2e.ts b/tests/specs/session-state-lifecycle.e2e.ts
