From 65a3bdc992e3c7dbda6e9988a94cdf54e49019b9 Mon Sep 17 00:00:00 2001
From: michael starke <michael.starke@hicknhack-software.com>
Date: Mon, 30 Oct 2017 14:03:33 +0100
Subject: [PATCH] fixed bugs in icon selection view

download icon button now is only enabled for nodes
delete icon call now has saveguards to not run into assertions
delete icon context menu action gets disabled if not applicable
feedback for errors when downloading a icon
better error handling when download for icons fails
---
 MacPass/Base.lproj/IconSelection.xib | 14 +++---
 MacPass/MPIconSelectViewController.m | 75 ++++++++++++++++++++++++++--
 2 files changed, 78 insertions(+), 11 deletions(-)

diff --git a/MacPass/Base.lproj/IconSelection.xib b/MacPass/Base.lproj/IconSelection.xib
index 4c6c8d75..f0856904 100644
--- a/MacPass/Base.lproj/IconSelection.xib
+++ b/MacPass/Base.lproj/IconSelection.xib
@@ -1,12 +1,14 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<document type="com.apple.InterfaceBuilder3.Cocoa.XIB" version="3.0" toolsVersion="12121" systemVersion="16G29" targetRuntime="MacOSX.Cocoa" propertyAccessControl="none" useAutolayout="YES">
+<document type="com.apple.InterfaceBuilder3.Cocoa.XIB" version="3.0" toolsVersion="13196" targetRuntime="MacOSX.Cocoa" propertyAccessControl="none" useAutolayout="YES">
     <dependencies>
-        <plugIn identifier="com.apple.InterfaceBuilder.CocoaPlugin" version="12121"/>
+        <deployment identifier="macosx"/>
+        <plugIn identifier="com.apple.InterfaceBuilder.CocoaPlugin" version="13196"/>
         <capability name="documents saved in the Xcode 8 format" minToolsVersion="8.0"/>
     </dependencies>
     <objects>
         <customObject id="-2" userLabel="File's Owner" customClass="MPIconSelectViewController">
             <connections>
+                <outlet property="downloadIconButton" destination="8kv-BJ-IEk" id="JH0-uV-F5k"/>
                 <outlet property="iconCollectionView" destination="58" id="77"/>
                 <outlet property="view" destination="1" id="56"/>
             </connections>
@@ -53,8 +55,8 @@
                     </connections>
                 </button>
                 <button verticalHuggingPriority="750" translatesAutoresizingMaskIntoConstraints="NO" id="8kv-BJ-IEk">
-                    <rect key="frame" x="153" y="18" width="104" height="25"/>
-                    <buttonCell key="cell" type="roundTextured" title="Download Icon" bezelStyle="texturedRounded" alignment="center" lineBreakMode="truncatingTail" state="on" borderStyle="border" imageScaling="proportionallyDown" inset="2" id="iaf-XW-XUo">
+                    <rect key="frame" x="149" y="18" width="108" height="25"/>
+                    <buttonCell key="cell" type="roundTextured" title="Download Icon" bezelStyle="texturedRounded" imagePosition="left" alignment="center" lineBreakMode="truncatingTail" state="on" borderStyle="border" imageScaling="proportionallyDown" inset="2" id="iaf-XW-XUo">
                         <behavior key="behavior" pushIn="YES" lightByBackground="YES" lightByGray="YES"/>
                         <font key="font" metaFont="system"/>
                     </buttonCell>
@@ -63,7 +65,7 @@
                     </connections>
                 </button>
                 <button verticalHuggingPriority="750" translatesAutoresizingMaskIntoConstraints="NO" id="His-4A-hcY">
-                    <rect key="frame" x="89" y="18" width="56" height="25"/>
+                    <rect key="frame" x="85" y="18" width="56" height="25"/>
                     <buttonCell key="cell" type="roundTextured" title="Cancel" bezelStyle="texturedRounded" alignment="center" state="on" borderStyle="border" imageScaling="proportionallyDown" inset="2" id="1kM-cI-P1o">
                         <behavior key="behavior" pushIn="YES" lightByBackground="YES" lightByGray="YES"/>
                         <font key="font" metaFont="system"/>
@@ -86,7 +88,7 @@
                 <constraint firstItem="57" firstAttribute="top" secondItem="1" secondAttribute="top" constant="20" symbolic="YES" id="iuz-pK-yXC"/>
                 <constraint firstItem="101" firstAttribute="leading" secondItem="8kv-BJ-IEk" secondAttribute="trailing" constant="8" symbolic="YES" id="nvi-CZ-a6L"/>
             </constraints>
-            <point key="canvasLocation" x="-143" y="15"/>
+            <point key="canvasLocation" x="-447" y="15"/>
         </customView>
         <collectionViewItem id="61" customClass="MPCollectionViewItem">
             <connections>
diff --git a/MacPass/MPIconSelectViewController.m b/MacPass/MPIconSelectViewController.m
index 84b3c027..0910a289 100644
--- a/MacPass/MPIconSelectViewController.m
+++ b/MacPass/MPIconSelectViewController.m
@@ -26,21 +26,33 @@
 #import "MPCollectionView.h"
 #import "MPCollectionViewItem.h"
 
+typedef NS_ENUM(NSInteger, MPIconDownloadStatus) {
+  MPIconDownloadStatusNone,
+  MPIconDownloadStatusProgress,
+  MPIconDownloadStatusError
+};
+
 @interface MPIconSelectViewController () <NSCollectionViewDelegate>
 
 /* UI properties */
 @property (weak) IBOutlet MPCollectionView *iconCollectionView;
 @property (weak) IBOutlet NSButton *imageButton;
+@property (weak) IBOutlet NSButton *downloadIconButton;
+@property (assign) MPIconDownloadStatus downloadStatus;
 
 @end
 
 @implementation MPIconSelectViewController
 
+@dynamic downloadStatus;
+
 - (NSString *)nibName {
   return @"IconSelection";
 }
 
 - (void)viewDidLoad {
+  KPKNode *node = self.representedObject;
+  self.downloadIconButton.enabled = (nil != node.asEntry);
   self.iconCollectionView.backgroundColors = @[NSColor.clearColor];
   self.iconCollectionView.selectable = YES;
   self.iconCollectionView.allowsMultipleSelection = NO;
@@ -64,35 +76,70 @@
 }
 
 - (IBAction)downloadIcon:(id)sender {
+  //self.downloadStatus = MPIconDownloadStatusProgress;
   KPKNode *node = self.representedObject;
   if(!node.asEntry) {
     return;
   }
-  NSString *rawURL = node.asEntry.url;
-  if([rawURL hasPrefix:@"http://"] || ![rawURL hasPrefix:@"https://"]) {
-    rawURL = [@"https://" stringByAppendingString:rawURL];
+  [self _downloadIconForURL:node.asEntry.url];
+}
+
+- (void)setDownloadStatus:(MPIconDownloadStatus)status {
+  switch (status) {
+    case MPIconDownloadStatusNone:
+      self.downloadIconButton.image = nil;
+      break;
+    case MPIconDownloadStatusError:
+      self.downloadIconButton.image = [NSImage imageNamed:NSImageNameCaution];
+      break;
+    case MPIconDownloadStatusProgress:
+      self.downloadIconButton.image = [NSImage imageNamed:NSImageNameRefreshTemplate];
+      break;
+  }
+}
+
+- (void)_downloadIconForURL:(NSString *)URLString {
+  if([URLString hasPrefix:@"http://"] || ![URLString hasPrefix:@"https://"]) {
+    URLString = [@"https://" stringByAppendingString:URLString];
   }
   
-  NSURL *url = [NSURL URLWithString:rawURL];
+  NSURL *url = [NSURL URLWithString:URLString];
   if(!url) {
+    self.downloadStatus = MPIconDownloadStatusError;
     return;
   }
   
   NSString *urlString = [NSString stringWithFormat:@"%@://%@/favicon.ico", url.scheme, url.host ? url.host : @""];
   NSURL *favIconURL = [NSURL URLWithString:urlString];
   if(!favIconURL) {
+    self.downloadStatus = MPIconDownloadStatusError;
     return;
   }
   
   KPKMetaData *metaData = ((MPDocument *)[NSDocumentController sharedDocumentController].currentDocument).tree.metaData;
   NSURLSessionTask *task = [[NSURLSession sharedSession] dataTaskWithURL:favIconURL completionHandler:^(NSData * _Nullable data, NSURLResponse * _Nullable response, NSError * _Nullable error) {
-    if(data) {
+    if(error) {
+      dispatch_async(dispatch_get_main_queue(), ^{
+        self.downloadStatus = MPIconDownloadStatusError;
+        [NSApp presentError:error];
+      });
+    }
+    if(data.length > 0) {
       dispatch_async(dispatch_get_main_queue(), ^{
         KPKIcon *newIcon = [[KPKIcon alloc] initWithImageData:data];
         if(newIcon) {
+          self.downloadStatus = MPIconDownloadStatusNone;
           [metaData addCustomIcon:newIcon];
           [self _updateCollectionViewContent];
         }
+        else {
+          self.downloadStatus = MPIconDownloadStatusError;
+        }
+      });
+    }
+    else {
+      dispatch_async(dispatch_get_main_queue(), ^{
+        self.downloadStatus = MPIconDownloadStatusError;
       });
     }
   }];
@@ -105,6 +152,9 @@
   if(index < firstCustomIndex) {
     return;
   }
+  if(index >= self.iconCollectionView.content.count) {
+    return;
+  }
   MPDocument *document = [NSDocumentController sharedDocumentController].currentDocument;
   KPKIcon *icon = self.iconCollectionView.content[index];
   [document.tree.metaData removeCustomIcon:icon];
@@ -127,6 +177,21 @@
   [self.view.window performClose:sender];
 }
 
+- (BOOL)validateMenuItem:(NSMenuItem *)menuItem {
+  if(menuItem.action == @selector(deleteIcon:)) {
+    NSUInteger index = self.iconCollectionView.contextMenuIndex;
+    NSUInteger firstCustomIndex = [MPIconHelper databaseIcons].count;
+    if(index < firstCustomIndex) {
+      return NO;
+    }
+    if(index >= self.iconCollectionView.content.count) {
+      return NO;
+    }
+    return YES;
+  }
+  return NO;
+}
+
 - (void)didSelectCollectionViewItem:(id)sender {
   if(![sender isKindOfClass:[NSCollectionViewItem class]]) {
     return;