Created attachment 220498 [details] [review]
add navigation for inner collections: 

Added a stack to keep track of navigation in inner collections.
Back button goes back to previous collection, instead of going to main view directly.

Created attachment 220541 [details] [review]
fixed problem of skipping some collections

fixed problem of skipping some collections while navigating back from other collections.

Review of attachment 220498 [details] [review]:

::: src/documents.js
@@ +628,3 @@
+    
+    isCollection: function(){
+        return this.collection;

This appears to be useless...you can just use the value of "collection", since it's a public field of Document.

::: src/manager.js
@@ +42,3 @@
         if (title)
             this._title = title;
+        this.stack = new Array(); //keeps track of navigation within collections

I think the BaseManager class is not the best place for this, since the history stack is only useful for documents, (but not for all the other things we use BaseManager for).
I would probably put it into the DocumentsManager object instead.

@@ +70,3 @@
+            this.stack.pop(); //removes the current one
+            item = this.stack.pop(); //get the next one in the stack
+        }

I'd rather explicitly have a method to go back in the history list rather than relying on setting NULL meaning going back.

Review of attachment 220541 [details] [review]:

Should be squashed with the previous patch, but has the problems mentioned in the previous review.

Created attachment 224108 [details] [review]
DocumentManager: Use a stack to navigate collections

This patch only applies on top of the patch I attached to bug #680001.

I don't know If I got it all right. Questions:

+    activatePreviousCollection: function() {
+        Global.collectionManager.setActiveItem(this._collectionPath.pop());
+    },
+
Should this method also call this._clearActiveDocModel();?


Another implementation of activatePreviousCollection() that I thought of was:

activatePreviousCollection: function() {
    this._collectionPath.pop()
    this.setActiveItem(this._collectionPath.pop())
},

where this._collectionPath array would also include the active collection (like in the previous patches). Would this approach be better?

(In reply to comment #5)

> Another implementation of activatePreviousCollection() that I thought of was:
> 
> activatePreviousCollection: function() {
>     this._collectionPath.pop()
>     this.setActiveItem(this._collectionPath.pop())
> },

Oh, sorry, it's not that simple, there must be added a check for null to set the active collection to null, since setActiveItem does just return when its argument is null.

Attachment 224108 [details] pushed as 9420a42 - DocumentManager: Use a stack to navigate collections

Thanks, works great here! I pushed this to git master now. I think calling _clearActiveDocModel() is indeed a good idea, so I modified the patch to do so.
I think this bug can be closed now.