[SunkenInTime]

ci: test web

Three people lost the file type on an export. On Windows with file extensions hidden, which is the default, the save dialog offers Retakes with no .ica showing. Change the name and press Save, and the file is written with no extension at all. A screenshot saved that way won't open, and an .ica couldn't be picked in Import .ica until #426 let import read such files.

I reproduced this in the real Windows save dialog on main. Typing a new name gave renamed-ica and renamed-shot with no extension. Video kept its extension because it already had ensureMp4Extension and ensureMovExtension.

The cause is what the crosscheck read in the code. file_picker 9.2.3 never sets a default extension on GetSaveFileNameW, so the dialog returns exactly what was typed. Screenshots had a second problem: file_picker writes the bytes itself, before Icarus sees the path.

The fix

chooseExportPath in lib/services/export_save_path.dart opens the dialog and adds the extension if it's missing, taking the extension from the suggested file name. All six desktop exports go through it: .ica (local and cloud), folder backup, library backup, screenshot, MP4 and .mov. It replaces the two video helpers. saveExportBytes picks the path first and then writes the bytes, so a screenshot lands only under the corrected name. The browser still downloads through file_picker.

The dialog only asks about replacing the name it returns. Typing retakes over an existing retakes.ica would have overwritten that file with no prompt, and the video helpers did exactly that. So when the corrected name is taken, the dialog opens again in the same folder with that name, and Save brings up Windows' own replace prompt.

Reopening the dialog felt better than adding an in-app prompt. The user stays in the save flow, can still pick another name, and sees the wording Windows always uses. It adds no new text to the app. The cost is a dialog that appears a second time, only in this one case.

How far this got

I ran driver builds of main and of this branch with the real dialogs up, not the driver's fake one. A script typed into the File name box and pressed Save.

Typed in the dialog	main	this branch
renamed-ica	renamed-ica	renamed-ica.ica
renamed-shot	renamed-shot	renamed-shot.png
renamed-mov (renamed-video on main)	renamed-video.mov	renamed-mov.mov
renamed-mp4	not run	renamed-mp4.mp4
renamed-library	not run	renamed-library.zip
renamed-folder	not run	renamed-folder.zip
dots.	dots	dots.ica
trail.ica.	trail.ica	trail.ica
space	space	space.ica
ascent v1.2	ascent v1.2	ascent v1.2.ica
UPPER.ICA	not run	UPPER.ICA

Windows strips trailing dots and spaces itself, so nothing doubled and the code needs no handling for them.

The files are real: ffprobe reads the MP4 and the .mov, the PNG has a PNG header, and the three archives open as zips. In a fresh library, Import .ica brought renamed-ica.ica back with its agent, and Import Backup restored both zips. Typing renamed-ica over the existing file reopened the dialog in the same folder, Save brought up the prompt above, Yes replaced the file, and Cancel on a second try stopped the export.

tool/drive/scripts/export-file-types.json repeats the .ica, screenshot and .mov cases with the driver's fake dialog, which can now type a name ("as": "renamed"). That run was clean on this branch.

After the rebase onto #426

The table above comes from the first build. I rebased onto main at 18306507 and built ff530466. The only conflict was the driver's step list in a comment, and both sides are kept.

On that build I repeated the real-dialog runs. All six exports kept the right extension and UPPER.ICA stayed as typed. Typing a taken name reopened the dialog, Save brought up Windows' prompt, and Cancel on a second try stopped the export. ffprobe reads the MP4. In a library with a strategy in it, Import .ica brought back renamed-ica.ica and Import Backup brought back the library zip.

The two changes also work together. I copied that .ica and that zip to old-export and old-backup, with no extension, and imported them through the real Open dialogs. Import .ica added old-export and Import Backup restored the strategy inside the backup. Exports from now on get their extension, and files saved before this fix still import.

Not covered

• The cloud .ica path. It calls the same function, and library_export_cloud_test passes, but I didn't run it on a dev account.
• macOS and Linux. The check does nothing when the extension is already there, which it should be on macOS. I didn't run either.
• Renaming files already saved without an extension. They keep their names, and Import a file as what it holds, not what it's named #426 imports them as they are. A screenshot saved that way still needs renaming by hand before it opens.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 54 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 5448c1f8-bcdc-40a3-8b70-02a3fac5d24d

Commits

Reviewing files that changed from the base of the PR and between 1830650 and ff53046.

Files selected for processing: 12

• lib/dev/drive/drive.dart
• lib/services/export_save_path.dart
• lib/services/video_export/ffmpeg_video_encoder.dart
• lib/services/video_export/quicktime_animation_sink.dart
• lib/strategy/strategy_import_export.dart
• lib/widgets/dialogs/export_video_dialog.dart
• lib/widgets/dialogs/screenshot_dialog.dart
• test/export_save_path_test.dart
• test/ffmpeg_video_encoder_test.dart
• test/page_screenshot_test.dart
• test/page_screenshots_test.dart
• tool/drive/scripts/export-file-types.json

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn