NavigationProvider - _removeXFramesForFile missing a null check

[petetnt]

  /**
    * Removes all frames from backward navigation stack for the given file only if the file is changed on disk.
    * @private
    */
    function _removeBackwardFramesForFile(file) {
        jumpBackwardStack = jumpBackwardStack.filter(function (frame) {
            return frame.filePath !== file._path && frame.stat !== file._stat;
        });
    }
    
    /**
    * Removes all frames from forward navigation stack for the given file only if the file is changed on disk.
    * @private
    */
    function _removeForwardFramesForFile(file) {
        jumpForwardStack = jumpForwardStack.filter(function (frame) {
            return frame.filePath !== file._path && frame.stat !== file._stat;
        });
    }

For some reason file can be null (for example when switching projects), raising errors in the stack filter calls.

@swmitra

[swmitra]

@petetnt If we look at the usage of this API

function _handleExternalChange(evt, doc) {
        if (doc) {
            _removeBackwardFramesForFile(doc.file);
            _removeForwardFramesForFile(doc.file);
            _validateNavigationCmds();
        }
    }

We have already checked for the null doc and in Brackets Document object can't exist without a file handle. Please let me know if you still think we need a null check on the 'file'.

[petetnt]

I am not sure why, but when I change projects I do have documents that don't have a file attached and run into this error:

ErrorNotification.js:122 Exception in 'change' listener on FileSystem
TypeError: Cannot read property '_path' of null TypeError: Cannot read property '_path' of null
    at file:///Applications/Brackets.app/Contents/dev/src/extensions/default/NavigationAndHistory/NavigationProvider.js:464:43
    at Array.filter (native)
    at _removeBackwardFramesForFile 

(Sorry, should have added the error in the original message too)

[petetnt]

It must have something to do with some sort of corrupted state but I am unable to find what exactly (MRUlist? Working set?)

[swmitra]

This looks like an external modification to me as FileSystem 'change' is getting called.

[swmitra]

@petetnt Can you please do this change at line 537

FileSystem.on("change", function (event, entry) {
            if (entry) {
                _handleExternalChange(event, {file: entry});
            }
        });

[swmitra]

function _initHandlers() {
        EditorManager.on("activeEditorChange", _handleActiveEditorChange);
        ProjectManager.on("projectOpen", _handleProjectOpen);
        EditorManager.on("_fullEditorCreatedForDocument", function (event, document, editor) {
            _handleExternalChange(event, document);
            _reinstateMarkers(editor, jumpBackwardStack);
            _reinstateMarkers(editor, jumpForwardStack);
        });
        FileSystem.on("change", function (event, entry) {
            if (entry) {
                _handleExternalChange(event, {file: entry});
            }
        });
        Document.on("_documentRefreshed", function (event, doc) {
            _handleExternalChange(event, doc);
        });
    }

You can use this code 😄

[swmitra]

The problem is the file watcher change notification where the entry parameter is null.

[petetnt]

👍 #13492