fix import scoping issue that caused feeds not to be imported at times, set created folders to be opened by default

8 files changed, 69 insertions, 55 deletions

diff --git a/bl/folderbl.php b/bl/folderbl.php
index b542f3d34..59fdddf16 100644
--- a/bl/folderbl.php
+++ b/bl/folderbl.php
@@ -66,6 +66,7 @@ class FolderBl extends Bl {
 		$folder->setName($folderName);
 		$folder->setUserId($userId);
 		$folder->setParentId($parentId);
+		$folder->setOpened(true);
 		return $this->mapper->insert($folder);
 	}
 
diff --git a/css/feeds.css b/css/feeds.css
index 1985c9ebf..ba8b6a367 100644
--- a/css/feeds.css
+++ b/css/feeds.css
@@ -134,7 +134,7 @@ button.action:hover {
 }
 
 .progress-icon {
-	background-image: url('%webroot%/core/img/loading.gif');
+	background-image: url('%webroot%/core/img/loading.gif') !important; 
 }
 
 .problem-icon {
diff --git a/js/app/controllers/settingscontroller.coffee b/js/app/controllers/settingscontroller.coffee
index d573e1213..3c6a7771e 100644
--- a/js/app/controllers/settingscontroller.coffee
+++ b/js/app/controllers/settingscontroller.coffee
@@ -34,6 +34,7 @@ angular.module('News').controller 'SettingsController',
 		try
 			FolderBl.import(fileContent)
 		catch error
+			console.error error
 			$scope.error = true
 
 ]
\ No newline at end of file
diff --git a/js/app/services/bl/folderbl.coffee b/js/app/services/bl/folderbl.coffee
index abba97054..42c1d9752 100644
--- a/js/app/services/bl/folderbl.coffee
+++ b/js/app/services/bl/folderbl.coffee
@@ -132,18 +132,24 @@ ItemModel, ShowAll, _ExistsError, OPMLParser)->
 		_importElement: (opml, parentFolderId) ->
 			for item in opml.getItems()
 				if item.isFolder()
-					try
-						@create item.getName(), (data) =>
-							@_importElement(item, data.folders[0].id)
-					catch error
-						if error instanceof _ExistsError
-							folder = @_folderModel.getByName(item.getName())
-							@open(folder.id)
-							@_importElement(item, folder.id)
+					do (item) =>
+						try
+							@create item.getName(), (data) =>
+								@_importElement(item, data.folders[0].id)
+						catch error
+							if error instanceof _ExistsError
+								folder = @_folderModel.getByName(item.getName())
+								@open(folder.id)
+								@_importElement(item, folder.id)
+							else
+								console.info error
 				else
 					try
-						@_feedBl.create(item.getUrl(), parentFolderId)
+						do (item) =>
+							@_feedBl.create(item.getUrl(), parentFolderId)
 					catch error
+						if not error instanceof _ExistsError
+							console.info error
 
 
 	return new FolderBl(FolderModel, FeedBl, ShowAll, ActiveFeed, Persistence,
diff --git a/js/app/services/models/feedmodel.coffee b/js/app/services/models/feedmodel.coffee
index 405ea2651..e0006e8d2 100644
--- a/js/app/services/models/feedmodel.coffee
+++ b/js/app/services/models/feedmodel.coffee
@@ -41,6 +41,8 @@ angular.module('News').factory '_FeedModel',
 			if data.faviconLink == null
 				data.faviconLink = 'url(' +
 					@_utils.imagePath('news', 'rss.svg') + ')'
+			else
+				data.faviconLink = 'url(' + data.faviconLink + ')'
 			###
 			We want to add a feed on the client side before
 			we have an id from the server. Once the server returns
diff --git a/js/public/app.js b/js/public/app.js
index 358b1dacc..04dfaaf79 100644
--- a/js/public/app.js
+++ b/js/public/app.js
@@ -468,6 +468,7 @@ License along with this library.  If not, see <http://www.gnu.org/licenses/>.
           return FolderBl["import"](fileContent);
         } catch (_error) {
           error = _error;
+          console.error(error);
           return $scope.error = true;
         }
       };
@@ -993,7 +994,7 @@ License along with this library.  If not, see <http://www.gnu.org/licenses/>.
         };
 
         FolderBl.prototype._importElement = function(opml, parentFolderId) {
-          var error, folder, item, _i, _len, _ref, _results,
+          var error, item, _i, _len, _ref, _results,
             _this = this;
 
           _ref = opml.getItems();
@@ -1001,26 +1002,37 @@ License along with this library.  If not, see <http://www.gnu.org/licenses/>.
           for (_i = 0, _len = _ref.length; _i < _len; _i++) {
             item = _ref[_i];
             if (item.isFolder()) {
+              _results.push((function(item) {
+                var error, folder;
+
+                try {
+                  return _this.create(item.getName(), function(data) {
+                    return _this._importElement(item, data.folders[0].id);
+                  });
+                } catch (_error) {
+                  error = _error;
+                  if (error instanceof _ExistsError) {
+                    folder = _this._folderModel.getByName(item.getName());
+                    _this.open(folder.id);
+                    return _this._importElement(item, folder.id);
+                  } else {
+                    return console.info(error);
+                  }
+                }
+              })(item));
+            } else {
               try {
-                _results.push(this.create(item.getName(), function(data) {
-                  return _this._importElement(item, data.folders[0].id);
-                }));
+                _results.push((function(item) {
+                  return _this._feedBl.create(item.getUrl(), parentFolderId);
+                })(item));
               } catch (_error) {
                 error = _error;
-                if (error instanceof _ExistsError) {
-                  folder = this._folderModel.getByName(item.getName());
-                  this.open(folder.id);
-                  _results.push(this._importElement(item, folder.id));
+                if (!error instanceof _ExistsError) {
+                  _results.push(console.info(error));
                 } else {
                   _results.push(void 0);
                 }
               }
-            } else {
-              try {
-                _results.push(this._feedBl.create(item.getUrl(), parentFolderId));
-              } catch (_error) {
-                error = _error;
-              }
             }
           }
           return _results;
@@ -1455,6 +1467,8 @@ License along with this library.  If not, see <http://www.gnu.org/licenses/>.
           }
           if (data.faviconLink === null) {
             data.faviconLink = 'url(' + this._utils.imagePath('news', 'rss.svg') + ')';
+          } else {
+            data.faviconLink = 'url(' + data.faviconLink + ')';
           }
           /*
           			We want to add a feed on the client side before
@@ -1933,7 +1947,7 @@ License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 
 
 (function() {
-  angular.module('News').factory('_OPMLParser', function() {
+  angular.module('News').factory('OPMLParser', function() {
     var Feed, Folder, OPMLParser;
 
     Feed = (function() {
@@ -2018,7 +2032,7 @@ License along with this library.  If not, see <http://www.gnu.org/licenses/>.
       return OPMLParser;
 
     })();
-    return OPMLParser;
+    return new OPMLParser();
   });
 
 }).call(this);
@@ -2577,12 +2591,6 @@ License along with this library.  If not, see <http://www.gnu.org/licenses/>.
     }
   ]);
 
-  angular.module('News').factory('OPMLParser', [
-    '_OPMLParser', function(_OPMLParser) {
-      return new _OPMLParser();
-    }
-  ]);
-
 }).call(this);
 
 // Generated by CoffeeScript 1.6.2
diff --git a/js/tests/services/bl/folderblSpec.coffee b/js/tests/services/bl/folderblSpec.coffee
index 8bb40033d..49f68634f 100644
--- a/js/tests/services/bl/folderblSpec.coffee
+++ b/js/tests/services/bl/folderblSpec.coffee
@@ -273,12 +273,13 @@ describe 'FolderBl', ->
 			  </head>
 			  <body>
 			  		<outline text="Design" title="Design" />
+			  		<outline text="test" title="test"></outline>
 			  </body>
 			</opml>'
 
 		@FolderBl.import(xml)
 
-		expect(@persistence.createFolder).toHaveBeenCalledWith('Design', 0,
+		expect(@persistence.createFolder).toHaveBeenCalledWith('test', 0,
 			jasmine.any(Function))
 		expect(@persistence.createFeed).not.toHaveBeenCalled()
 
diff --git a/utility/feedfetcher.php b/utility/feedfetcher.php
index 27828336d..65d6b2683 100644
--- a/utility/feedfetcher.php
+++ b/utility/feedfetcher.php
@@ -62,10 +62,9 @@ class FeedFetcher implements IFeedFetcher {
 		// TODO: write unittests!
 		$simplePie = new \SimplePie_Core();
 		$simplePie->set_feed_url( $url );
-		$simplePie->enable_cache( false );
+		$simplePie->enable_cache(true);
 		$simplePie->set_cache_location($this->cacheDirectory);
 		$simplePie->set_cache_duration($this->cacheDuration);
-		$simplePie->enable_cache(true);
 		
 		if (!$simplePie->init()) {
 			throw new FetcherException('Could not initialize simple pie');
@@ -115,18 +114,21 @@ class FeedFetcher implements IFeedFetcher {
 			$feed->setUrlHash(md5($url));
 			$feed->setAdded(time());
 
+			// get the favicon from the feed
 			$favicon = $simplePie->get_image_url();
-
-			if ($favicon !== null && $this->checkFavicon($favicon)) {
+			if ($favicon) {
 				$feed->setFaviconLink($favicon);
 
+			// or the webpage
 			} else {
 				$webFavicon = $this->discoverFavicon($url);
 				if ($webFavicon !== null) {
 					$feed->setFaviconLink($webFavicon);
 				}
 			}
+
 			return array($feed, $items);
+
 		} catch(\Exception $ex){
 			throw new FetcherException($ex->getMessage());
 		}
@@ -134,21 +136,17 @@ class FeedFetcher implements IFeedFetcher {
 	}
 
 
-	private function checkFavicon($favicon) {
-		if ($favicon === null || $favicon == false)
+	private function isValidFavIcon($favicon) {
+		if (!$favicon){
 			return false;
+		}
 
 		$file = new \SimplePie_File($favicon);
-		// size in bytes
-		$filesize = strlen($file->body);
 
-		if($file->success && $filesize > 0 && $filesize < 50000) { //bigger files are not considered favicons
+		if($file->success) {
 			$sniffer = new \SimplePie_Content_Type_Sniffer($file);
 			if(substr($sniffer->get_type(), 0, 6) === 'image/') {
-				$imgsize = @getimagesize($favicon);
-				if ($imgsize && $imgsize['0'] <= 32 && $imgsize['1'] <= 32) { //bigger images are not considered favicons
-					return true;
-				}
+				return true;
 			}
 		}
 		return false;
@@ -156,16 +154,9 @@ class FeedFetcher implements IFeedFetcher {
 
 
 	private function discoverFavicon($url) {
-		//try webroot favicon
-		$favicon = \SimplePie_Misc::absolutize_url('/favicon.ico', $url);
-
-		if($this->checkFavicon($favicon))
-			return $favicon;
 
 		//try to extract favicon from web page
-		$absoluteUrl = \SimplePie_Misc::absolutize_url('/', $url);
-		$page = $this->api->getUrlContent($absoluteUrl);
-
+		$page = $this->api->getUrlContent($url);
 		if ( FALSE !== $page ) {
 			$doc = @\DOMDocument::loadHTML($page);
 
@@ -175,14 +166,18 @@ class FeedFetcher implements IFeedFetcher {
 
 				if ( $elements->length > 0 ) {
 					if ( $favicon = $elements->item(0)->getAttribute('href') ) {
+
 						// the specified uri might be an url, an absolute or a relative path
 						// we have to turn it into a url to be able to display it out of context
 						if ( !parse_url($favicon, PHP_URL_SCHEME) ) {
+							$absoluteUrl = \SimplePie_Misc::absolutize_url('/', $url);
 							$favicon = \SimplePie_Misc::absolutize_url($favicon, $absoluteUrl);
 						}
 
-						if($this->checkFavicon($favicon))
+						if($this->isValidFavIcon($favicon)){
 							return $favicon;
+						}
+							
 					}
 				}
 			}