Skip to content
This repository has been archived by the owner on Mar 3, 2023. It is now read-only.

🐛 Prevent call to getCursorScreenRow if there's no cursor #2401

Closed
wants to merge 1 commit into from

Conversation

abe33
Copy link
Contributor

@abe33 abe33 commented May 26, 2014

I got this error randomly when closing tabs. The bracket-matcher package still try to update after the tab destruction, calling isFoldedAtCursorRow on an editor that no longer have a cursor.

Uncaught TypeError: Cannot call method 'getScreenRow' of undefined /Applications/Atom.app/Contents/Resources/app/src/editor.js:1311
module.exports.Editor.getCursorScreenRow /Applications/Atom.app/Contents/Resources/app/src/editor.js:1311
module.exports.Editor.isFoldedAtCursorRow /Applications/Atom.app/Contents/Resources/app/src/editor.js:819
module.exports.BracketMatcherView.updateMatch /Applications/Atom.app/Contents/Resources/app/node_modules/bracket-matcher/lib/bracket-matcher-view…:138
(anonymous function) /Applications/Atom.app/Contents/Resources/app/node_modules/bracket-matcher/lib/bracket-matcher-view…:123
(anonymous function) /Applications/Atom.app/Contents/Resources/app/node_modules/emissary/lib/emitter.js:133
module.exports.Emitter.emit /Applications/Atom.app/Contents/Resources/app/node_modules/emissary/lib/emitter.js:132
(anonymous function) /Applications/Atom.app/Contents/Resources/app/src/cursor.js:62
(anonymous function) /Applications/Atom.app/Contents/Resources/app/node_modules/emissary/lib/emitter.js:133
module.exports.Emitter.emit /Applications/Atom.app/Contents/Resources/app/node_modules/emissary/lib/emitter.js:132
module.exports.DisplayBufferMarker.destroyed /Applications/Atom.app/Contents/Resources/app/src/display-buffer-marker.js:170
(anonymous function) /Applications/Atom.app/Contents/Resources/app/src/display-buffer-marker.js:37
(anonymous function) /Applications/Atom.app/Contents/Resources/app/node_modules/emissary/lib/emitter.js:133
module.exports.Emitter.emit /Applications/Atom.app/Contents/Resources/app/node_modules/emissary/lib/emitter.js:132
module.exports.Marker.destroy /Applications/Atom.app/Contents/Resources/app/node_modules/text-buffer/lib/marker.js:291
module.exports.DisplayBufferMarker.destroy /Applications/Atom.app/Contents/Resources/app/src/display-buffer-marker.js:149
module.exports.Selection.destroy /Applications/Atom.app/Contents/Resources/app/src/selection.js:49
module.exports.Editor.destroyed /Applications/Atom.app/Contents/Resources/app/src/editor.js:205
module.exports.Model.destroy /Applications/Atom.app/Contents/Resources/app/node_modules/theorist/lib/model.js:218
module.exports.EditorView.beforeRemove /Applications/Atom.app/Contents/Resources/app/src/editor-view.js:1422
jQuery.cleanData /Applications/Atom.app/Contents/Resources/app/node_modules/space-pen/lib/space-pen.js:395
jQuery.cleanData /Applications/Atom.app/Contents/Resources/app/src/space-pen-extensions.js:24
jQuery.fn.extend.remove /Applications/Atom.app/Contents/Resources/app/node_modules/space-pen/vendor/jquery.js:5494
module.exports.EditorView.remove /Applications/Atom.app/Contents/Resources/app/src/editor-view.js:1413
module.exports.PaneView.onItemRemoved /Applications/Atom.app/Contents/Resources/app/src/pane-view.js:321
(anonymous function) /Applications/Atom.app/Contents/Resources/app/src/pane-view.js:3
(anonymous function) /Applications/Atom.app/Contents/Resources/app/node_modules/emissary/lib/emitter.js:133
module.exports.Emitter.emit /Applications/Atom.app/Contents/Resources/app/node_modules/emissary/lib/emitter.js:132
module.exports.Pane.removeItem /Applications/Atom.app/Contents/Resources/app/src/pane.js:227
module.exports.Pane.destroyItem /Applications/Atom.app/Contents/Resources/app/src/pane.js:260
module.exports.Pane.destroyActiveItem /Applications/Atom.app/Contents/Resources/app/src/pane.js:252
module.exports.Workspace.destroyActivePaneItem /Applications/Atom.app/Contents/Resources/app/src/workspace.js:296
_results.push._this.(anonymous function) /Applications/Atom.app/Contents/Resources/app/node_modules/delegato/lib/delegator.js:67
(anonymous function) /Applications/Atom.app/Contents/Resources/app/src/workspace-view.js:299
jQuery.event.dispatch /Applications/Atom.app/Contents/Resources/app/node_modules/space-pen/vendor/jquery.js:4676
elemData.handle /Applications/Atom.app/Contents/Resources/app/node_modules/space-pen/vendor/jquery.js:4360
module.exports.KeymapManager.dispatchCommandEvent /Applications/Atom.app/Contents/Resources/app/node_modules/atom-keymap/lib/keymap-manager.js:395
module.exports.KeymapManager.handleKeyboardEvent /Applications/Atom.app/Contents/Resources/app/node_modules/atom-keymap/lib/keymap-manager.js:176
(anonymous function) /Applications/Atom.app/Contents/Resources/app/src/window-event-handler.js:90
jQuery.event.dispatch /Applications/Atom.app/Contents/Resources/app/node_modules/space-pen/vendor/jquery.js:4676
elemData.handle

This patch forces the isFoldedAtCursorRow method to return false if there's no cursor available.

I got this error randomly when closing tabs. The `bracket-matcher` package still try to update after the tab destruction, calling `isFoldedAtCursorRow` on an editor that no longer have a cursor.

```
Uncaught TypeError: Cannot call method 'getScreenRow' of undefined /Applications/Atom.app/Contents/Resources/app/src/editor.js:1311
module.exports.Editor.getCursorScreenRow /Applications/Atom.app/Contents/Resources/app/src/editor.js:1311
module.exports.Editor.isFoldedAtCursorRow /Applications/Atom.app/Contents/Resources/app/src/editor.js:819
module.exports.BracketMatcherView.updateMatch /Applications/Atom.app/Contents/Resources/app/node_modules/bracket-matcher/lib/bracket-matcher-view…:138
(anonymous function) /Applications/Atom.app/Contents/Resources/app/node_modules/bracket-matcher/lib/bracket-matcher-view…:123
(anonymous function) /Applications/Atom.app/Contents/Resources/app/node_modules/emissary/lib/emitter.js:133
module.exports.Emitter.emit /Applications/Atom.app/Contents/Resources/app/node_modules/emissary/lib/emitter.js:132
(anonymous function) /Applications/Atom.app/Contents/Resources/app/src/cursor.js:62
(anonymous function) /Applications/Atom.app/Contents/Resources/app/node_modules/emissary/lib/emitter.js:133
module.exports.Emitter.emit /Applications/Atom.app/Contents/Resources/app/node_modules/emissary/lib/emitter.js:132
module.exports.DisplayBufferMarker.destroyed /Applications/Atom.app/Contents/Resources/app/src/display-buffer-marker.js:170
(anonymous function) /Applications/Atom.app/Contents/Resources/app/src/display-buffer-marker.js:37
(anonymous function) /Applications/Atom.app/Contents/Resources/app/node_modules/emissary/lib/emitter.js:133
module.exports.Emitter.emit /Applications/Atom.app/Contents/Resources/app/node_modules/emissary/lib/emitter.js:132
module.exports.Marker.destroy /Applications/Atom.app/Contents/Resources/app/node_modules/text-buffer/lib/marker.js:291
module.exports.DisplayBufferMarker.destroy /Applications/Atom.app/Contents/Resources/app/src/display-buffer-marker.js:149
module.exports.Selection.destroy /Applications/Atom.app/Contents/Resources/app/src/selection.js:49
module.exports.Editor.destroyed /Applications/Atom.app/Contents/Resources/app/src/editor.js:205
module.exports.Model.destroy /Applications/Atom.app/Contents/Resources/app/node_modules/theorist/lib/model.js:218
module.exports.EditorView.beforeRemove /Applications/Atom.app/Contents/Resources/app/src/editor-view.js:1422
jQuery.cleanData /Applications/Atom.app/Contents/Resources/app/node_modules/space-pen/lib/space-pen.js:395
jQuery.cleanData /Applications/Atom.app/Contents/Resources/app/src/space-pen-extensions.js:24
jQuery.fn.extend.remove /Applications/Atom.app/Contents/Resources/app/node_modules/space-pen/vendor/jquery.js:5494
module.exports.EditorView.remove /Applications/Atom.app/Contents/Resources/app/src/editor-view.js:1413
module.exports.PaneView.onItemRemoved /Applications/Atom.app/Contents/Resources/app/src/pane-view.js:321
(anonymous function) /Applications/Atom.app/Contents/Resources/app/src/pane-view.js:3
(anonymous function) /Applications/Atom.app/Contents/Resources/app/node_modules/emissary/lib/emitter.js:133
module.exports.Emitter.emit /Applications/Atom.app/Contents/Resources/app/node_modules/emissary/lib/emitter.js:132
module.exports.Pane.removeItem /Applications/Atom.app/Contents/Resources/app/src/pane.js:227
module.exports.Pane.destroyItem /Applications/Atom.app/Contents/Resources/app/src/pane.js:260
module.exports.Pane.destroyActiveItem /Applications/Atom.app/Contents/Resources/app/src/pane.js:252
module.exports.Workspace.destroyActivePaneItem /Applications/Atom.app/Contents/Resources/app/src/workspace.js:296
_results.push._this.(anonymous function) /Applications/Atom.app/Contents/Resources/app/node_modules/delegato/lib/delegator.js:67
(anonymous function) /Applications/Atom.app/Contents/Resources/app/src/workspace-view.js:299
jQuery.event.dispatch /Applications/Atom.app/Contents/Resources/app/node_modules/space-pen/vendor/jquery.js:4676
elemData.handle /Applications/Atom.app/Contents/Resources/app/node_modules/space-pen/vendor/jquery.js:4360
module.exports.KeymapManager.dispatchCommandEvent /Applications/Atom.app/Contents/Resources/app/node_modules/atom-keymap/lib/keymap-manager.js:395
module.exports.KeymapManager.handleKeyboardEvent /Applications/Atom.app/Contents/Resources/app/node_modules/atom-keymap/lib/keymap-manager.js:176
(anonymous function) /Applications/Atom.app/Contents/Resources/app/src/window-event-handler.js:90
jQuery.event.dispatch /Applications/Atom.app/Contents/Resources/app/node_modules/space-pen/vendor/jquery.js:4676
elemData.handle
```

This patch forces the `isFoldedAtCursorRow` method to return false if there's no cursor available.
@joaoafrmartins
Copy link

is this being triggered when you remove files?? +1

@joaoafrmartins
Copy link

@kevinsawicki @benogle

@abe33
Copy link
Contributor Author

abe33 commented May 26, 2014

@joaoafrmartins So far I didn't encounter that on file remove, but it's possible, if some code calls this API and a cursor can't be found it'll break as well.

@kevinsawicki
Copy link
Contributor

There are lots of API in Editor that always assume a cursor/selection is available so none of them have these guards since it is a really nice assumption that if you have an Editor that isn't destroyed it is guaranteed to have a cursor.

In this case though it looks like the Editor is destroyed but bracket matcher hasn't been notified yet since it relies on its View having beforeRemove called which will occur after EditorView.remove is called.

Adding something like this to bracket matcher should fix it in that case:

@subscribe @editor, 'destroyed', => @unsubscribe()

@abe33
Copy link
Contributor Author

abe33 commented May 27, 2014

I guess this PR can be closed. I reported the issue in bracket-matcher, I'll try to provide a fix if I can find some time.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants