Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uncaught TypeError: this.textEditor.displayBuffer.getMaxScrollTop is not a function #387

Closed
Alchiadus opened this issue Sep 27, 2015 · 21 comments

Comments

@Alchiadus
Copy link

[Enter steps to reproduce below:]

  1. Build and start the latest Atom/master. At least f17767a or newer.
  2. Open a file with minimap enabled.

DisplayBuffer::getMaxScrollTop is removed in this PR: atom/atom#8905


Atom Version: 1.2.0-dev-e9a8646
System: Microsoft Windows 10 Pro Insider Preview, Build 10547
Thrown From: minimap package, v4.13.4

Stack Trace

Uncaught TypeError: this.textEditor.displayBuffer.getMaxScrollTop is not a function

At /C:/Users/Philippe/.atom/packages/minimap/lib/minimap.coffee:169

TypeError: this.textEditor.displayBuffer.getMaxScrollTop is not a function
    at Minimap.module.exports.Minimap.getTextEditorMaxScrollTop (C:\Users\Philippe\.atom\packages\minimap\lib\minimap.coffee:200:46)
    at Minimap.module.exports.Minimap.getTextEditorScrollRatio (C:\Users\Philippe\.atom\packages\minimap\lib\minimap.coffee:216:36)
    at Minimap.module.exports.Minimap.getCapedTextEditorScrollRatio (C:\Users\Philippe\.atom\packages\minimap\lib\minimap.coffee:223:50)
    at Minimap.module.exports.Minimap.getScrollTop (C:\Users\Philippe\.atom\packages\minimap\lib\minimap.coffee:334:17)
    at Minimap.module.exports.Minimap.getLastVisibleScreenRow (C:\Users\Philippe\.atom\packages\minimap\lib\minimap.coffee:322:17)
    at Minimap.module.exports.DecorationManagement.emitRangeChanges (C:\Users\Philippe\.atom\packages\minimap\lib\mixins\decoration-management.coffee:261:31)
    at Minimap.module.exports.DecorationManagement.emitDecorationChanges (C:\Users\Philippe\.atom\packages\minimap\lib\mixins\decoration-management.coffee:253:6)
    at Minimap.module.exports.DecorationManagement.decorateMarker (C:\Users\Philippe\.atom\packages\minimap\lib\mixins\decoration-management.coffee:224:6)
    at MinimapGitDiffBinding.module.exports.MinimapGitDiffBinding.markRange (C:\Users\Philippe\.atom\packages\minimap-git-diff\lib\minimap-git-diff-binding.coffee:63:14)
    at MinimapGitDiffBinding.module.exports.MinimapGitDiffBinding.addDecorations (C:\Users\Philippe\.atom\packages\minimap-git-diff\lib\minimap-git-diff-binding.coffee:49:10)
    at MinimapGitDiffBinding.module.exports.MinimapGitDiffBinding.updateDiffs (C:\Users\Philippe\.atom\packages\minimap-git-diff\lib\minimap-git-diff-binding.coffee:42:8)
    at Immediate._onImmediate (C:\Users\Philippe\.atom\packages\minimap-git-diff\lib\minimap-git-diff-binding.coffee:1:1)
    at processImmediate [as _immediateCallback] (timers.js:371:17)

Commands

Config

{
  "core": {
    "themes": [
      "one-dark-ui",
      "base16-syntax"
    ],
    "disabledPackages": [
      "autoflow",
      "autosave",
      "background-tips",
      "bookmarks",
      "exception-reporting",
      "go-to-line",
      "incompatible-packages",
      "markdown-preview",
      "metrics",
      "package-generator",
      "symbols-view",
      "update-package-dependencies",
      "welcome"
    ],
    "ignoredNames": [
      "desktop.ini"
    ]
  },
  "minimap": {
    "displayPluginsControls": false,
    "plugins": {
      "codeglance": true,
      "find-and-replace": true,
      "git-diff": true,
      "pigments": true,
      "selection": true,
      "highlight-selected": true
    },
    "scrollAnimation": true
  }
}

Installed Packages

# User
atom-ternjs, v0.7.2
autoclose-html, v0.19.0
autocomplete-emojis, v2.2.2
autocomplete-paths, v1.0.2
base16-syntax, v1.5.0
color-picker, v2.0.12
column-select, v0.2.0
editorconfig, v1.2.0
file-icons, v1.6.9
highlight-selected, v0.10.1
language-latex, v0.6.1
language-lua, v0.9.4
language-powershell, v2.1.0
linter, v1.6.0
linter-coffeelint, v1.1.0
linter-csslint, v1.0.5
linter-eslint, v3.0.2
linter-less, v2.1.0
linter-lua, v1.0.0
markdown-preview-plus, v2.1.1
minimap, v4.13.4
minimap-find-and-replace, v4.3.0
minimap-git-diff, v4.1.8
minimap-highlight-selected, v4.3.1
minimap-selection, v4.3.0
pdf-view, v0.27.0
sublime-style-column-selection, v1.3.0
tab-control, v0.6.8

# Dev
base16-syntax, v1.5.0
latex-essentials, v0.0.0
@as-cii
Copy link
Contributor

as-cii commented Sep 28, 2015

Thanks for keeping an eye on this, @Alchiadus.

@abe33: the method in question was not part of the public API (and wasn't either present in TextEditor), therefore I went ahead and deprecated it. In the new version you should be able to replicate its behavior by using TextEditorElement::getScrollHeight().

I am not sure this ought to be included in the public API, but maybe it should. What do you think?

@abe33 abe33 closed this as completed in cf37f60 Sep 28, 2015
@abe33
Copy link
Contributor

abe33 commented Sep 28, 2015

That was a bold move...
I still got errors even after the change:

Uncaught TypeError: Cannot read property 'getScrollTop' of nullTextEditorElement.getScrollTop @ text-editor-element.coffee:248
module.exports.TextEditor.getScrollTop @ text-editor.coffee:3012
module.exports.Minimap.getTextEditorScrollRatio @ minimap.coffee:218
module.exports.Minimap.getCapedTextEditorScrollRatio @ minimap.coffee:225
module.exports.Minimap.getScrollTop @ minimap.coffee:336
module.exports.Minimap.getLastVisibleScreenRow @ minimap.coffee:324
module.exports.DecorationManagement.emitRangeChanges @ /Users/cedric/github/minimap/lib/mixins/decoration-management.coffee:232
module.exports.DecorationManagement.emitDecorationChanges @ /Users/cedric/github/minimap/lib/mixins/decoration-management.coffee:225
module.exports.DecorationManagement.decorateMarker @ /Users/cedric/github/minimap/lib/mixins/decoration-management.coffee:191
module.exports.MinimapGitDiffBinding.markRange @ /Users/cedric/github/minimap-git-diff/lib/minimap-git-diff-binding.coffee:103
module.exports.MinimapGitDiffBinding.addDecorations @ /Users/cedric/github/minimap-git-diff/lib/minimap-git-diff-binding.coffee:76
module.exports.MinimapGitDiffBinding.updateDiffs @ /Users/cedric/github/minimap-git-diff/lib/minimap-git-diff-binding.coffee:60
__bind @ /Users/cedric/github/minimap-git-diff/lib/minimap-git-diff-binding.coffee:3processImmediate @ timers.js:371

Raised when moving a file by drag and drop to another pane.

Another one is raised from the wrap guide package:

Uncaught TypeError: Cannot read property 'onDidChangeScrollLeft' of nullTextEditorElement.onDidChangeScrollLeft @ text-editor-element.coffee:225
module.exports.TextEditor.onDidChangeScrollLeft @ text-editor.coffee:437
WrapGuideElement.handleEvents @ wrap-guide-element.coffee:28
WrapGuideElement.initialize @ wrap-guide-element.coffee:7
(anonymous function) @ main.coffee:11
(anonymous function) @ workspace.coffee:198
(anonymous function) @ workspace.coffee:349
module.exports.Emitter.emit @ /Users/cedric/github/atom/node_modules/event-kit/lib/emitter.js:82
module.exports.PaneContainer.addedPaneItem @ pane-container.coffee:221
(anonymous function) @ pane-container.coffee:214
module.exports.Emitter.emit @ /Users/cedric/github/atom/node_modules/event-kit/lib/emitter.js:82
module.exports.Pane.addItem @ pane.coffee:363
module.exports.Pane.moveItemToPane @ pane.coffee:418
module.exports.TabBarView.moveItemBetweenPanes @ tab-bar-view.coffee:395
module.exports.TabBarView.onDrop @ tab-bar-view.coffee:342
__bind @ tab-bar-view.coffee:1
jQuery.event.dispatch @ /Users/cedric/github/atom/node_modules/jquery/dist/jquery.js:4435
jQuery.event.add.elemData.handle @ /Users/cedric/github/atom/node_modules/jquery/dist/jquery.js:4121

I'll make sure the patch won't be available for the current Atom version, cause it'll probably breaks with versions before the deprecations

@as-cii
Copy link
Contributor

as-cii commented Sep 28, 2015

In the new version you should be able to replicate its behavior by using TextEditorElement::getScrollHeight().

I should probably have emphasized this more but, if you want to keep compatibility for both versions, you can use methods on TextEditor (at least, for a couple of Atom versions), as we have shimmed them to call TextEditorElement. Sorry about that.

Please, note that they will be deprecated at some point, so you should probably switch to TextEditorElement methods anyways as soon as possible (maybe after a new Atom release lands).

Thanks, @abe33.

@abe33 abe33 reopened this Sep 28, 2015
@abe33
Copy link
Contributor

abe33 commented Sep 28, 2015

I'm trying to figure how to get rid of all the deprecations, but it has a major impact on the minimap behavior. Properties that were available even when the text editor was not in the DOM are no longer available, etc... I don't know how long it will take to fix all the broken specs...

@abe33
Copy link
Contributor

abe33 commented Oct 7, 2015

A quick update, the cn-fix-deprecations branch is now fully functional with Atom's master, I'm just waiting for the release of 1.0.20 to merge and publish it.

@steelbrain
Copy link

@abe33 You can just bump the target atom version instead of waiting for a release.

@abe33
Copy link
Contributor

abe33 commented Oct 14, 2015

@steelbrain sadly not.
I should bump it to 1.0.20, but it would still fail for people using the 1.1.0-beta1, as it doesn't have these changes.
I'll have to make the same changes than in Pigments, meaning supporting both API and then release a patch, and finally removing this support when 1.0.20 gets out.

@abe33
Copy link
Contributor

abe33 commented Oct 22, 2015

Ok, so I reverted many changes and published a new version with an adapter layer to handle the differences, let me know if it fixes things for you.

@abe33
Copy link
Contributor

abe33 commented Oct 23, 2015

Looks like most people with this issue confirmed the latest version fix these errors, I'm closing this issue then.

@abe33 abe33 closed this as completed Oct 23, 2015
@jchannon
Copy link

1.1.0 is now out and this error now appears 😄

@M4GNV5
Copy link

M4GNV5 commented Oct 29, 2015

yep 😒

error information: https://gist.github.com/M4GNV5/b15b87d02c29f3658d12

@yupswing
Copy link

reopen please.
in atom 1.1.0 this error is back.

EDIT:
confirm no error, I just needed to restart atom and everything is fine.
sorry :)
[atom v1.1.1, minimap v4.15.0]

@abe33
Copy link
Contributor

abe33 commented Oct 30, 2015

@jchannon, @M4GNV5, @yupswing Are you sure you have the latest version installed?

As far as I can tell, @M4GNV5 is using v4.12.1, the version that handles both pre-1.1 and 1.1 is v4.15.0.
And I'm using 1.1.0 actually and haven't noticed any error.

@Alchiadus
Copy link
Author

I just opened Atom v1.1.0 and I can confirm that I have no errors (using v4.15.0).

I typically use the latest master of Atom, so that is how I got the minimap update, but the engines field hasn't changed so everyone should be receiving the update?

@abe33
Copy link
Contributor

abe33 commented Oct 30, 2015

v4.15 engine field is no longer restricted to > 1.0.19 so yes they should get the update (but they should also have been notified of v4.14 as well as its engine field should match 1.1)

@jchannon
Copy link

Just checked and I had an old version of the plugin, sorry

On 30 October 2015 at 10:05, Cédric Néhémie [email protected]
wrote:

v4.15 engine field is no longer restricted to > 1.0.19 so yes they should
get the update (but they should also have been notified of v4.14 as well as
its engine field should match 1.1)


Reply to this email directly or view it on GitHub
#387 (comment)
.

@M4GNV5
Copy link

M4GNV5 commented Oct 30, 2015

same for me, updating to 4.15.0fixed it

@peterfraedrich
Copy link

Please reopen:
Seeing this issue on Minimap v4.15.1 with Atom 1.1.0

EDIT: apparently restarting everything fixed it (despite having done so already). strange.

@abe33
Copy link
Contributor

abe33 commented Oct 31, 2015

@peterfraedrich Yeah, it's a recurring problem with custom elements, as you can't unregister them, if a change is made in an update, the updated version won't be able to register (as one is already registered), leading to this kind of nasty behaviour. I think the Minimap will have to move on a new version without custom elements to definitely get rid of these issues.

@abe33
Copy link
Contributor

abe33 commented Nov 1, 2015

@bencasalino Not sure to understand what you mean? What is your problem?

@bencasalino
Copy link

bencasalino commented Nov 1, 2015 via email

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

No branches or pull requests

9 participants