Browse files

Don't try to scroll if editor isn't visible

  • Loading branch information...
1 parent f936a70 commit 01e72d7c6b1c64f7809569d25368551e1be0614c @njx njx committed Mar 13, 2013
Showing with 7 additions and 5 deletions.
  1. +7 −5 lib/codemirror.js
@@ -1432,15 +1432,17 @@ window.CodeMirror = (function() {
if (!captureMiddleClick) on(d.scroller, "contextmenu", function(e) {onContextMenu(cm, e);});
on(d.scroller, "scroll", function() {
- setScrollTop(cm, d.scroller.scrollTop);
- setScrollLeft(cm, d.scroller.scrollLeft, true);
- signal(cm, "scroll", cm);
+ if (d.scroller.clientHeight) {
+ setScrollTop(cm, d.scroller.scrollTop);
+ setScrollLeft(cm, d.scroller.scrollLeft, true);
+ signal(cm, "scroll", cm);
+ }
on(d.scrollbarV, "scroll", function() {
- setScrollTop(cm, d.scrollbarV.scrollTop);
+ if (d.scroller.clientHeight) setScrollTop(cm, d.scrollbarV.scrollTop);
on(d.scrollbarH, "scroll", function() {
- setScrollLeft(cm, d.scrollbarH.scrollLeft);
+ if (d.scroller.clientHeight) setScrollLeft(cm, d.scrollbarH.scrollLeft);
on(d.scroller, "mousewheel", function(e){onScrollWheel(cm, e);});

2 comments on commit 01e72d7

How about moving that test into setScrollTop and setScrollLeft? Do you see any problem with that? (It seems that all calls to these functions suffer from this problem.)

njx replied Mar 13, 2013

I'm not sure if that's necessary. I just took a look in the debugger, and it seems that in the original bug case, the primary problem is that the inputs to setScrollTop and setScrollLeft from the scroll event handlers are bogus (because they're trying to read the scroll position from the hidden scroller). It's also possible that other stuff downstream would also be bogus if setScrollTop/setScrollLeft were called in other cases while the editor is hidden, but conversely, I was worried that it might be dangerous to skip other things those functions do (like setting cm.doc.scrollTop/Left) while the editor is hidden.

I'll have to defer to your judgment on this one.

Please sign in to comment.