Fix: Fixed an issue where changing file permission for multiple files simultaneously would crash the app (#10886)

hecksmosis · web-flow · commit 9d9b0d59e6bc · 2023-01-06T14:40:13.000-05:00
diff --git a/src/Files.App/ViewModels/Properties/PropertiesTab.cs b/src/Files.App/ViewModels/Properties/PropertiesTab.cs
@@ -74,7 +74,7 @@ protected override void OnNavigatedFrom(NavigationEventArgs e)
 		/// Tries to save changed properties to file.
 		/// </summary>
 		/// <returns>Returns true if properties have been saved successfully.</returns>
-		public abstract Task<bool> SaveChangesAsync(ListedItem item);
+		public abstract Task<bool> SaveChangesAsync();
 
 		public abstract void Dispose();
 	}
diff --git a/src/Files.App/Views/Pages/Properties.xaml b/src/Files.App/Views/Pages/Properties.xaml
@@ -29,8 +29,8 @@
 
 		<Border
 			x:Name="TitlebarArea"
-			HorizontalAlignment="Stretch"
 			Grid.Row="0"
+			HorizontalAlignment="Stretch"
 			Background="Transparent">
 			<NavigationView
 				x:Name="NavigationView"
@@ -57,73 +57,74 @@
 				<!--  Tabs  -->
 				<NavigationView.MenuItems>
 					<NavigationViewItem
-					x:Name="TabGeneral"
-					AccessKey="G"
-					Content="{helpers:ResourceString Name=General}"
-					CornerRadius="0"
-					Tag="General">
+						x:Name="TabGeneral"
+						AccessKey="G"
+						Content="{helpers:ResourceString Name=General}"
+						CornerRadius="0"
+						Tag="General">
 						<NavigationViewItem.Icon>
 							<FontIcon Glyph="&#xE7C3;" />
 						</NavigationViewItem.Icon>
 					</NavigationViewItem>
 					<NavigationViewItem
-					x:Name="TabSecurity"
-					Content="{helpers:ResourceString Name=Security}"
-					CornerRadius="0"
-					Tag="Security">
+						x:Name="TabSecurity"
+						Content="{helpers:ResourceString Name=Security}"
+						CornerRadius="0"
+						Tag="Security"
+						Visibility="Collapsed">
 						<NavigationViewItem.Icon>
 							<FontIcon Glyph="&#xE730;" />
 						</NavigationViewItem.Icon>
 					</NavigationViewItem>
 					<NavigationViewItem
-					x:Name="TabShorcut"
-					AccessKey="S"
-					Content="{helpers:ResourceString Name=Shortcut}"
-					CornerRadius="0"
-					Tag="Shortcut"
-					Visibility="Collapsed">
+						x:Name="TabShorcut"
+						AccessKey="S"
+						Content="{helpers:ResourceString Name=Shortcut}"
+						CornerRadius="0"
+						Tag="Shortcut"
+						Visibility="Collapsed">
 						<NavigationViewItem.Icon>
 							<FontIcon FontFamily="{StaticResource CustomGlyph}" Glyph="&#xF10A;" />
 						</NavigationViewItem.Icon>
 					</NavigationViewItem>
 					<NavigationViewItem
-					x:Name="TabLibrary"
-					AccessKey="L"
-					Content="{helpers:ResourceString Name=Library}"
-					CornerRadius="0"
-					Tag="Library"
-					Visibility="Collapsed">
+						x:Name="TabLibrary"
+						AccessKey="L"
+						Content="{helpers:ResourceString Name=Library}"
+						CornerRadius="0"
+						Tag="Library"
+						Visibility="Collapsed">
 						<NavigationViewItem.Icon>
 							<FontIcon Glyph="&#xE1D3;" />
 						</NavigationViewItem.Icon>
 					</NavigationViewItem>
 					<NavigationViewItem
-					x:Name="TabDetails"
-					AccessKey="D"
-					Content="{helpers:ResourceString Name=Details}"
-					CornerRadius="0"
-					Tag="Details"
-					Visibility="Collapsed">
+						x:Name="TabDetails"
+						AccessKey="D"
+						Content="{helpers:ResourceString Name=Details}"
+						CornerRadius="0"
+						Tag="Details"
+						Visibility="Collapsed">
 						<NavigationViewItem.Icon>
 							<FontIcon Glyph="&#xE946;" />
 						</NavigationViewItem.Icon>
 					</NavigationViewItem>
 					<NavigationViewItem
-					x:Name="TabCustomization"
-					Content="{helpers:ResourceString Name=Customization}"
-					CornerRadius="0"
-					Tag="Customization"
-					Visibility="Collapsed">
+						x:Name="TabCustomization"
+						Content="{helpers:ResourceString Name=Customization}"
+						CornerRadius="0"
+						Tag="Customization"
+						Visibility="Collapsed">
 						<NavigationViewItem.Icon>
 							<FontIcon Glyph="&#xE771;" />
 						</NavigationViewItem.Icon>
 					</NavigationViewItem>
 					<NavigationViewItem
-					x:Name="TabCompatibility"
-					Content="{helpers:ResourceString Name=Compatibility}"
-					CornerRadius="0"
-					Tag="Compatibility"
-					Visibility="Collapsed">
+						x:Name="TabCompatibility"
+						Content="{helpers:ResourceString Name=Compatibility}"
+						CornerRadius="0"
+						Tag="Compatibility"
+						Visibility="Collapsed">
 						<NavigationViewItem.Icon>
 							<FontIcon Glyph="&#xECAA;" />
 						</NavigationViewItem.Icon>
diff --git a/src/Files.App/Views/Pages/Properties.xaml.cs b/src/Files.App/Views/Pages/Properties.xaml.cs
@@ -30,8 +30,6 @@ public sealed partial class Properties : Page
 		private object navParameterItem;
 		private IShellPage AppInstance;
 
-		private ListedItem listedItem;
-
 		public SettingsViewModel AppSettings => App.AppSettings;
 
 		public AppWindow appWindow;
@@ -58,18 +56,26 @@ protected override void OnNavigatedTo(NavigationEventArgs e)
 			var args = e.Parameter as PropertiesPageNavigationArguments;
 			AppInstance = args.AppInstanceArgument;
 			navParameterItem = args.Item;
-			listedItem = args.Item as ListedItem;
-			TabShorcut.Visibility = listedItem is not null && listedItem.IsShortcut ? Visibility.Visible : Visibility.Collapsed;
-			TabLibrary.Visibility = listedItem is not null && listedItem.IsLibrary ? Visibility.Visible : Visibility.Collapsed;
-			TabDetails.Visibility = listedItem is not null && listedItem.FileExtension is not null && !listedItem.IsShortcut && !listedItem.IsLibrary ? Visibility.Visible : Visibility.Collapsed;
-			TabSecurity.Visibility = args.Item is DriveItem ||
-				(listedItem is not null && !listedItem.IsLibrary && !listedItem.IsRecycleBinItem) ? Visibility.Visible : Visibility.Collapsed;
-			TabCustomization.Visibility = listedItem is not null && !listedItem.IsLibrary && (
-				(listedItem.PrimaryItemAttribute == Windows.Storage.StorageItemTypes.Folder && !listedItem.IsArchive) ||
-				(listedItem.IsShortcut && !listedItem.IsLinkItem)) ? Visibility.Visible : Visibility.Collapsed;
-			TabCompatibility.Visibility = listedItem is not null && (
-					FileExtensionHelpers.IsExecutableFile(listedItem is ShortcutItem sht ? sht.TargetPath : listedItem.FileExtension, true)
-				) ? Visibility.Visible : Visibility.Collapsed;
+			if (args.Item is ListedItem listedItem)
+			{
+				var isShortcut = listedItem.IsShortcut;
+				var isLibrary = listedItem.IsLibrary;
+				var fileExt = listedItem.FileExtension;
+				TabShorcut.Visibility = isShortcut ? Visibility.Visible : Visibility.Collapsed;
+				TabLibrary.Visibility = isLibrary ? Visibility.Visible : Visibility.Collapsed;
+				TabDetails.Visibility = fileExt is not null && !isShortcut && !isLibrary ? Visibility.Visible : Visibility.Collapsed;
+				TabCustomization.Visibility = !isLibrary && (
+					(listedItem.PrimaryItemAttribute == Windows.Storage.StorageItemTypes.Folder && !listedItem.IsArchive) ||
+					(isShortcut && !listedItem.IsLinkItem)) ? Visibility.Visible : Visibility.Collapsed;
+				TabCompatibility.Visibility = (
+						FileExtensionHelpers.IsExecutableFile(listedItem is ShortcutItem sht ? sht.TargetPath : fileExt, true)
+					) ? Visibility.Visible : Visibility.Collapsed;
+				TabSecurity.Visibility = !isLibrary && !listedItem.IsRecycleBinItem ? Visibility.Visible : Visibility.Collapsed;
+			}
+			else if (args.Item is DriveItem)
+			{
+				TabSecurity.Visibility = Visibility.Visible;
+			}
 			base.OnNavigatedTo(e);
 		}
 
@@ -194,11 +200,11 @@ private async void OKButton_Click(object sender, RoutedEventArgs e)
 		{
 			if (contentFrame.Content is PropertiesGeneral propertiesGeneral)
 			{
-				await propertiesGeneral.SaveChangesAsync(listedItem);
+				await propertiesGeneral.SaveChangesAsync();
 			}
 			else
 			{
-				if (!await (contentFrame.Content as PropertiesTab).SaveChangesAsync(listedItem))
+				if (!await (contentFrame.Content as PropertiesTab).SaveChangesAsync())
 					return;
 			}
 
diff --git a/src/Files.App/Views/Pages/PropertiesCompatibility.xaml.cs b/src/Files.App/Views/Pages/PropertiesCompatibility.xaml.cs
@@ -37,7 +37,7 @@ protected override void Properties_Loaded(object sender, RoutedEventArgs e)
 			}
 		}
 
-		public override Task<bool> SaveChangesAsync(ListedItem item)
+		public override Task<bool> SaveChangesAsync()
 		{
 			if (CompatibilityProperties is not null)
 				return Task.FromResult(CompatibilityProperties.SetCompatibilityOptions());
diff --git a/src/Files.App/Views/Pages/PropertiesCustomization.xaml.cs b/src/Files.App/Views/Pages/PropertiesCustomization.xaml.cs
@@ -29,7 +29,7 @@ private void CustomIconsSelectorFrame_Loaded(object sender, RoutedEventArgs e)
 			}, new SuppressNavigationTransitionInfo());
 		}
 
-		public override Task<bool> SaveChangesAsync(ListedItem item)
+		public override Task<bool> SaveChangesAsync()
 		{
 			return Task.FromResult(true);
 		}
diff --git a/src/Files.App/Views/Pages/PropertiesDetails.xaml.cs b/src/Files.App/Views/Pages/PropertiesDetails.xaml.cs
@@ -40,7 +40,7 @@ private static ContentDialog SetContentDialogRoot(ContentDialog contentDialog)
 			return contentDialog;
 		}
 
-		public override async Task<bool> SaveChangesAsync(ListedItem item)
+		public override async Task<bool> SaveChangesAsync()
 		{
 			while (true)
 			{
diff --git a/src/Files.App/Views/Pages/PropertiesGeneral.xaml.cs b/src/Files.App/Views/Pages/PropertiesGeneral.xaml.cs
@@ -22,26 +22,34 @@ public sealed partial class PropertiesGeneral : PropertiesTab
 
 		public PropertiesGeneral() => InitializeComponent();
 
-		public override async Task<bool> SaveChangesAsync(ListedItem item)
+		public override async Task<bool> SaveChangesAsync()
 		{
-			ViewModel.ItemName = ItemFileName.Text; // Make sure Name is updated
-
-			string newName = ViewModel.ItemName;
-			string oldName = ViewModel.OriginalItemName;
-			bool hasNewName = !string.IsNullOrWhiteSpace(newName) && newName != oldName;
-
 			return BaseProperties switch
 			{
 				DriveProperties properties => SaveDrive(properties.Drive),
 				LibraryProperties properties => await SaveLibraryAsync(properties.Library),
 				CombinedProperties properties => await SaveCombinedAsync(properties.List),
-				_ => await SaveBaseAsync(),
+				FileProperties properties => await SaveBaseAsync(properties.Item),
+				FolderProperties properties => await SaveBaseAsync(properties.Item),
 			};
 
+			bool GetNewName(out string newName)
+			{
+				if (ItemFileName is not null)
+				{
+					ViewModel.ItemName = ItemFileName.Text; // Make sure Name is updated
+					newName = ViewModel.ItemName;
+					string oldName = ViewModel.OriginalItemName;
+					return !string.IsNullOrWhiteSpace(newName) && newName != oldName;
+				}
+				newName = "";
+				return false;
+			}
+
 			bool SaveDrive(DriveItem drive)
 			{
 				var fsVM = AppInstance.FilesystemViewModel;
-				if (!hasNewName || fsVM is null)
+				if (!GetNewName(out var newName) || fsVM is null)
 					return false;
 
 				newName = letterRegex.Replace(newName, string.Empty); // Remove "(C:)" from the new label
@@ -58,7 +66,7 @@ bool SaveDrive(DriveItem drive)
 			async Task<bool> SaveLibraryAsync(LibraryItem library)
 			{
 				var fsVM = AppInstance.FilesystemViewModel;
-				if (!hasNewName || fsVM is null || !App.LibraryManager.CanCreateLibrary(newName).result)
+				if (!GetNewName(out var newName) || fsVM is null || !App.LibraryManager.CanCreateLibrary(newName).result)
 					return false;
 
 				newName = $"{newName}{ShellLibraryItem.EXTENSION}";
@@ -94,7 +102,7 @@ await App.Window.DispatcherQueue.EnqueueAsync(() =>
 				return true;
 			}
 
-			async Task<bool> SaveBaseAsync()
+			async Task<bool> SaveBaseAsync(ListedItem item)
 			{
 				// Handle the visibility attribute for a single file
 				var itemMM = AppInstance?.SlimContentPage?.ItemManipulationModel;
@@ -105,7 +113,7 @@ await App.Window.DispatcherQueue.EnqueueAsync(() =>
 					);
 				}
 
-				if (!hasNewName)
+				if (!GetNewName(out var newName))
 					return true;
 
 				return await App.Window.DispatcherQueue.EnqueueAsync(() =>
diff --git a/src/Files.App/Views/Pages/PropertiesLibrary.xaml.cs b/src/Files.App/Views/Pages/PropertiesLibrary.xaml.cs
@@ -190,7 +190,7 @@ private static ContentDialog SetContentDialogRoot(ContentDialog contentDialog)
 			return contentDialog;
 		}
 
-		public override async Task<bool> SaveChangesAsync(ListedItem item)
+		public override async Task<bool> SaveChangesAsync()
 		{
 			if (BaseProperties is LibraryProperties props)
 			{
diff --git a/src/Files.App/Views/Pages/PropertiesSecurity.xaml.cs b/src/Files.App/Views/Pages/PropertiesSecurity.xaml.cs
@@ -49,7 +49,7 @@ protected override void OnNavigatedTo(NavigationEventArgs e)
 			base.OnNavigatedTo(e);
 		}
 
-		public async override Task<bool> SaveChangesAsync(ListedItem item)
+		public async override Task<bool> SaveChangesAsync()
 		{
 			return SecurityProperties is null || SecurityProperties.SetFilePermissions();
 		}
diff --git a/src/Files.App/Views/Pages/PropertiesShortcut.xaml.cs b/src/Files.App/Views/Pages/PropertiesShortcut.xaml.cs
@@ -11,7 +11,7 @@ public PropertiesShortcut()
 			InitializeComponent();
 		}
 
-		public override Task<bool> SaveChangesAsync(ListedItem item)
+		public override Task<bool> SaveChangesAsync()
 		{
 			return Task.FromResult(true);
 		}

Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,7 @@ protected override void OnNavigatedFrom(NavigationEventArgs e)`
`74`	`74`	`/// Tries to save changed properties to file.`
`75`	`75`	`/// </summary>`
`76`	`76`	`/// <returns>Returns true if properties have been saved successfully.</returns>`
`77`		`- public abstract Task<bool> SaveChangesAsync(ListedItem item);`
	`77`	`+ public abstract Task<bool> SaveChangesAsync();`
`78`	`78`
`79`	`79`	`public abstract void Dispose();`
`80`	`80`	`}`
Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,7 @@ protected override void Properties_Loaded(object sender, RoutedEventArgs e)`
`37`	`37`	`}`
`38`	`38`	`}`
`39`	`39`
`40`		`- public override Task<bool> SaveChangesAsync(ListedItem item)`
	`40`	`+ public override Task<bool> SaveChangesAsync()`
`41`	`41`	`{`
`42`	`42`	`if (CompatibilityProperties is not null)`
`43`	`43`	`return Task.FromResult(CompatibilityProperties.SetCompatibilityOptions());`
Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ private void CustomIconsSelectorFrame_Loaded(object sender, RoutedEventArgs e)`
`29`	`29`	`}, new SuppressNavigationTransitionInfo());`
`30`	`30`	`}`
`31`	`31`
`32`		`- public override Task<bool> SaveChangesAsync(ListedItem item)`
	`32`	`+ public override Task<bool> SaveChangesAsync()`
`33`	`33`	`{`
`34`	`34`	`return Task.FromResult(true);`
`35`	`35`	`}`
Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,7 @@ private static ContentDialog SetContentDialogRoot(ContentDialog contentDialog)`
`40`	`40`	`return contentDialog;`
`41`	`41`	`}`
`42`	`42`
`43`		`- public override async Task<bool> SaveChangesAsync(ListedItem item)`
	`43`	`+ public override async Task<bool> SaveChangesAsync()`
`44`	`44`	`{`
`45`	`45`	`while (true)`
`46`	`46`	`{`
Original file line number	Diff line number	Diff line change
`@@ -190,7 +190,7 @@ private static ContentDialog SetContentDialogRoot(ContentDialog contentDialog)`
`190`	`190`	`return contentDialog;`
`191`	`191`	`}`
`192`	`192`
`193`		`- public override async Task<bool> SaveChangesAsync(ListedItem item)`
	`193`	`+ public override async Task<bool> SaveChangesAsync()`
`194`	`194`	`{`
`195`	`195`	`if (BaseProperties is LibraryProperties props)`
`196`	`196`	`{`
Original file line number	Diff line number	Diff line change
`@@ -49,7 +49,7 @@ protected override void OnNavigatedTo(NavigationEventArgs e)`
`49`	`49`	`base.OnNavigatedTo(e);`
`50`	`50`	`}`
`51`	`51`
`52`		`- public async override Task<bool> SaveChangesAsync(ListedItem item)`
	`52`	`+ public async override Task<bool> SaveChangesAsync()`
`53`	`53`	`{`
`54`	`54`	`return SecurityProperties is null \|\| SecurityProperties.SetFilePermissions();`
`55`	`55`	`}`
Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@ public PropertiesShortcut()`
`11`	`11`	`InitializeComponent();`
`12`	`12`	`}`
`13`	`13`
`14`		`- public override Task<bool> SaveChangesAsync(ListedItem item)`
	`14`	`+ public override Task<bool> SaveChangesAsync()`
`15`	`15`	`{`
`16`	`16`	`return Task.FromResult(true);`
`17`	`17`	`}`