Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simulate a FileOk handler when choosing an image file #1335

Merged
merged 1 commit into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

- [SIL.Core] Added macOS support for `GlobalMutex`

### Fixed

- [SIL.Window.Forms] When choosing a file in the ImageToolbox.AcquireImageControl, a FileOk handler is simulated that verifies the selected file passes the given filter. Users can defeat the filter mechanism by pasting or typing the file name. While the returned filename does not pass the filter, the dialog is reopened until the user either chooses a proper filename or cancels the dialog. The native FileOk handler can prevent the dialog from closing: we can't achieve that. (See BL-13552.)

## [14.1.1] - 2024-05-23

### Fixed
Expand Down
72 changes: 72 additions & 0 deletions SIL.Windows.Forms.Tests/ImageToolbox/ImageToolboxTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -112,5 +112,77 @@ public void CheckMonoForSelectLargeIconView()
Assert.IsNotNull(viewOnClickMethod, "MWFFileView no longer has an OnClickViewMenuSubItem(object, EventArgs) method");
}
}

[Test]
public void DoubleCheckFileFilterWorks()
{
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(null, "foo.txt"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(null, "foo.doc"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(null, "foo"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(null, "foo."));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(null, "foo.bar"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(null, "foo.bar.baz"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter("", "foo.txt"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter("", "foo.doc"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter("", "foo"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter("", "foo."));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter("", "foo.bar"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter("", "foo.bar.baz"));

var filterAll = "All files|*.*";
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterAll, "foo.txt"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterAll, "foo.doc"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterAll, "foo"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterAll, "foo."));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterAll, "foo.bar"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterAll, "foo.bar.baz"));

var filterTxtOnly = "Text files|*.txt";
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterTxtOnly, "foo.txt"));
Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterTxtOnly, "foo.doc"));
Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterTxtOnly, "foo"));
Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterTxtOnly, "foo."));
Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterTxtOnly, "foo.bar"));
Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterTxtOnly, "foo.txt.baz"));

var filterTxtAndDoc = "Text files|*.txt|Word files|*.doc";
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDoc, "foo.txt"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDoc, "foo.doc"));
Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDoc, "foo"));
Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDoc, "foo."));
Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDoc, "foo.bar"));
Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDoc, "foo.txt.baz"));

var filterTxtAndDocWithAll = "Text files|*.txt|Word files|*.doc|All files|*.*";
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDocWithAll, "foo.txt"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDocWithAll, "foo.doc"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDocWithAll, "foo"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDocWithAll, "foo."));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDocWithAll, "foo.bar"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterTxtAndDocWithAll, "foo.txt.baz"));

var filterCodeSourceFiles = "Image files|*.png;*.jpg;*.jpeg;*.tiff";
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterCodeSourceFiles, "foo.png"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterCodeSourceFiles, "foo.jpg"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterCodeSourceFiles, "foo.jpeg"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterCodeSourceFiles, "foo.tiff"));
Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterCodeSourceFiles, "foo"));
Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterCodeSourceFiles, "foo."));
Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterCodeSourceFiles, "foo.bar"));
Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterCodeSourceFiles, "foo.png.baz"));

var filterCsCppSourceFiles = "Bitmap files|*.bmp;*.png;*.tiff|JPEG files|*.jpg;*.jpeg";
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterCsCppSourceFiles, "foo.bmp"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterCsCppSourceFiles, "foo.png"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterCsCppSourceFiles, "foo.tiff"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterCsCppSourceFiles, "foo.jpg"));
Assert.IsTrue(AcquireImageControl.DoubleCheckFileFilter(filterCsCppSourceFiles, "foo.jpeg"));
Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterCsCppSourceFiles, "foo.h"));
Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterCsCppSourceFiles, "foo.cs"));
Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterCsCppSourceFiles, "foo"));
Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterCsCppSourceFiles, "foo."));
Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterCsCppSourceFiles, "foo.bar"));
Assert.IsFalse(AcquireImageControl.DoubleCheckFileFilter(filterCsCppSourceFiles, "foo.png.baz"));
}
}
}
135 changes: 100 additions & 35 deletions SIL.Windows.Forms/ImageToolbox/AcquireImageControl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -81,61 +81,126 @@ private void GetImageFileFromSystem()
{
try
{
// Used to simulate having a FileOk event handler that can cancel the OK button click.
bool invalidFileChosen;
SetMode(Modes.SingleImage);
// The primary thing that OpenFileDialogWithViews buys us is the ability to default to large icons.
// OpenFileDialogWithViews still doesn't let us read (and thus remember) the selected view.
using (var dlg = new OpenFileDialogWithViews(OpenFileDialogWithViews.DialogViewTypes.Large_Icons))
do
{
if (string.IsNullOrEmpty(ImageToolboxSettings.Default.LastImageFolder))
invalidFileChosen = false;
// The primary thing that OpenFileDialogWithViews buys us is the ability to default to large icons.
// OpenFileDialogWithViews still doesn't let us read (and thus remember) the selected view.
using (var dlg = new OpenFileDialogWithViews(OpenFileDialogWithViews.DialogViewTypes.Large_Icons))
{
dlg.InitialDirectory = Environment.GetFolderPath(Environment.SpecialFolder.MyPictures);
}
else
{
dlg.InitialDirectory = ImageToolboxSettings.Default.LastImageFolder; ;
}

dlg.Title = "Choose Picture File".Localize("ImageToolbox.FileChooserTitle", "Title shown for a file chooser dialog brought up by the ImageToolbox");
if (string.IsNullOrEmpty(ImageToolboxSettings.Default.LastImageFolder))
{
dlg.InitialDirectory = Environment.GetFolderPath(Environment.SpecialFolder.MyPictures);
}
else
{
dlg.InitialDirectory = ImageToolboxSettings.Default.LastImageFolder; ;
}

//NB: disallowed gif because of a .net crash: http://jira.palaso.org/issues/browse/BL-85
dlg.Filter = "picture files".Localize("ImageToolbox.PictureFiles", "Shown in the file-picking dialog to describe what kind of files the dialog is filtering for") + "(*.png;*.tif;*.tiff;*.jpg;*.jpeg;*.bmp)|*.png;*.tif;*.tiff;*.jpg;*.jpeg;*.bmp;";
dlg.Title = "Choose Picture File".Localize("ImageToolbox.FileChooserTitle", "Title shown for a file chooser dialog brought up by the ImageToolbox");

if (DialogResult.OK == dlg.ShowDialog(this.ParentForm))
{
ImageToolboxSettings.Default.LastImageFolder = Path.GetDirectoryName(dlg.FileName);
ImageToolboxSettings.Default.Save();
//NB: disallowed gif because of a .net crash: http://jira.palaso.org/issues/browse/BL-85
dlg.Filter = "picture files".Localize("ImageToolbox.PictureFiles", "Shown in the file-picking dialog to describe what kind of files the dialog is filtering for") + "(*.png;*.tif;*.tiff;*.jpg;*.jpeg;*.bmp)|*.png;*.tif;*.tiff;*.jpg;*.jpeg;*.bmp;";

try
{
_currentImage = PalasoImage.FromFile(dlg.FileName);
}
catch (Exception err) //for example, http://jira.palaso.org/issues/browse/BL-199
if (DialogResult.OK == dlg.ShowDialog(this.ParentForm))
{
var msg = "Sorry, there was a problem loading that image.".Localize(
"ImageToolbox.ProblemLoadingImage");
if (ImageLoadingExceptionReporter!= null)
// Simulate having a FileOk event handler that can cancel the OK button click.
// We would prefer to actually use the event handler since that can prevent the
// dialog from closing, but we can't do that because of using DialogAdapters for
// cross-platform compatibility. It's not worth messing with DialogAdapters at
// this point just to get this feature, which might not be cross-platform.
if (!DoubleCheckFileFilter(dlg.Filter, dlg.FileName))
Comment on lines +111 to +115
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need DialogAdapters given that we are no longer shipping Bloom (or other software) on Mono Linux? It would seem like a good idea to just remove the "cross-platform" code since it's not actually used and go with the simpler native implementation of FileOK for Windows.Forms. Am I missing something?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We thought Flex was still using the mono code in libpalaso. Is that incorrect?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you fact-check the following for me, @jasonleenaylor ?

FieldWorks won't be shipping a new version on mono linux, so we're safe to remove mono cross-platform code from libpalaso master and reduce complexity for everyone. I gather this PR would have been simpler without the mono requirement. We don't even have build infrastructure anymore to build FW mono on Linux.

WinForms in libpalaso should be assumed to be Windows-only runtime.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes @megahirt you are correct. We have stopped packaging for linux and are focusing on rearchitecting with the hope handle future cross-platform versions without mono. Sorry for any extra effort that went in because that wasn't widely known. When I saw this comment I assumed Bloom was still shipping for linux with mono.

{
invalidFileChosen = true;
continue;
}
ImageToolboxSettings.Default.LastImageFolder = Path.GetDirectoryName(dlg.FileName);
ImageToolboxSettings.Default.Save();

try
{
ImageLoadingExceptionReporter(dlg.FileName, err, msg);
_currentImage = PalasoImage.FromFile(dlg.FileName);
}
else
catch (Exception err) //for example, http://jira.palaso.org/issues/browse/BL-199
{
ErrorReport.NotifyUserOfProblem(err, msg);
var msg = "Sorry, there was a problem loading that image.".Localize(
"ImageToolbox.ProblemLoadingImage");
if (ImageLoadingExceptionReporter!= null)
{
ImageLoadingExceptionReporter(dlg.FileName, err, msg);
}
else
{
ErrorReport.NotifyUserOfProblem(err, msg);
}

return;
}

return;
_pictureBox.Image = _currentImage.Image;
if (ImageChanged != null)
ImageChanged.Invoke(this, null);
}
_pictureBox.Image = _currentImage.Image;
if (ImageChanged != null)
ImageChanged.Invoke(this, null);
}
}
} while (invalidFileChosen);
}
finally
{
_actModal = false;
}
}

/// <summary>
/// Return true if the filePath truly passes the filtering of the filterString.
/// People can defeat the filtering by typing or pasting a file path into the file path box.
/// </summary>
/// <param name="filterString">filter string like those used in file dialogs</param>
/// <param name="filePath">file path returned by a file dialog</param>
internal static bool DoubleCheckFileFilter(string filterString, string filePath)
{
if (string.IsNullOrEmpty(filterString))
return true; // no filter, so everything passes
if (string.IsNullOrEmpty(filePath))
return false; // no file, so nothing passes
var filterSections = filterString.Split('|');
if (filterSections.Length < 2)
return true; // no filter, so everything passes
var fileName = Path.GetFileName(filePath);
for (int i = 1; i < filterSections.Length; i += 2)
{
if (PassesFilter(filterSections[i], fileName))
return true;
}
return false;
}
private static bool PassesFilter(string filterList, string fileName)
{
var parts = filterList.Split(';');
foreach (var part in parts)
{
if (part == "*.*" || part == "*")
return true;
var filter = part.Trim();
if (filter.StartsWith("*"))
{
filter = filter.Substring(1);
if (fileName.EndsWith(filter, StringComparison.InvariantCultureIgnoreCase))
return true;
}
else if (filter.EndsWith("*"))
{
filter = filter.Substring(0, filter.Length - 1);
if (fileName.StartsWith(filter, StringComparison.InvariantCultureIgnoreCase))
return true;
}
else if (fileName.Equals(filter, StringComparison.InvariantCultureIgnoreCase))
return true;
}
return false;
}

private void OpenFileFromDrag(string path)
{
SetMode(Modes.SingleImage);
Expand Down