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

Scrolling with mouse fails past 5.9.1 when CSS alters inner div width of scrollbar. #3677

Closed
johnjbarton opened this issue Dec 2, 2015 · 5 comments

Comments

@johnjbarton
Copy link

With CodeMirror past 5.9.1 we lost the ability to scroll with the mouse in our use of CodeMirror.

We trace this issue to
https://google.com/url?sa=D&q=https%3A%2F%2Fgithub.com%2Fcodemirror%2FCodeMirror%2Fcommit%2Fa11ccbefa281a0e728b7431ba52f550854831ea6
where these lines:

          var box = bar.getBoundingClientRect();
          var elt = document.elementFromPoint(box.left + 1, box.bottom - 1); 
          if (elt != bar) bar.style.pointerEvents = "none";

seem to assume that the scrollbar has an inner div only one pixel wide (the 1 in the elt computation).
When our scroll bar renders, the div inside of .CodeMirror-vscrollbar comes out 6px wide:

<div class="CodeMirror-vscrollbar" cm-not-content="true" style="bottom: 0px; width: 18px; pointer-events: none; display: block;">
  <div style="min-width: 1px; height: 1640px;"></div>
</div> 

Thus elt points to the inner div and elt != bar, disabling the pointerEvents.

At least part of the width was caused by a CSS rule like

::-webkit-scrollbar {
  width: 12px;
}

but, this being CSS, removing the 12px width reduced the inner div from 6px to 3px, not 12px to 1px.

We worked around the issue by adding a CSS rule to set the maxWidth: 1px on the inner div. We'd love to not have to do that!

@johnjbarton
Copy link
Author

Well our analysis was all wrong.

If we get lucky, then

measure.clientHeight 827 sWidth 12

in NativeScrollbars update(), and we never call the zeroWidthHack().

If we get unlucky then these values start as zero, and we set pointerEvents="none" then don't recover.

Every time we switch files in our tool we are unlucky :-(

@johnjbarton
Copy link
Author

I dug in to this further. Note that our results vary if we stop on Chrome Devtools breakpoints.

We seem to go through a state where measure.clientHeight > 0 but sWidth == 0 so the zeroWidthHack() is triggered. Once it triggers we end up taking the true branch in

if (elt != bar) bar.style.pointerEvents = "none";

So if we could avoid triggering the zeroWidthHack() or if we could have elt == bar, we would not have the bug. Somehow we are using CodeMirror in way it's not quite expecting, both in terms of order of operations (clientHeight and nativeScrollbar width changes) and in terms of how elements are arranged (elt vs bar).

I'd love to have a simple reproduction, but it seems hard.

For now we set

.CodeMirror-vscrollbar {
  pointer-events: auto !important;
}

@marijnh
Copy link
Member

marijnh commented Dec 7, 2015

I'd like to get to the bottom of this, but without a test case that reproduces the problem, I really don't know where to look. I double-checked that if the editor starts off invisible, it still does the right thing. You seem to somehow end up with a situation where your editor does have dimensions, but your scrollbar width still comes out as zero. I couldn't find a way to produce such a situation.

@johnjbarton
Copy link
Author

With measure.clientHeight and sWidth non-zero we change codemirror on a resize event:

codeMirrorWrapper.on('resize', function() {
    this.codeMirror.refresh();
...

That results in clientHeight 0, sWidth 12.
Next we change the content:

 this.codeMirror.getDoc().setValue(contents);

That results in clientHeight 0 sWidth 0.
Then we call CodeMirror.setOptions(), which later calls

function wrappingChanged(cm) {
            if (cm.options.lineWrapping) {
                addClass(cm.display.wrapper, "CodeMirror-wrap");
                cm.display.sizer.style.minWidth = "";
                cm.display.sizerWidth = null 
            } else {
                rmClass(cm.display.wrapper, "CodeMirror-wrap");
                findMaxLine(cm)
            }
            estimateLineHeights(cm);
            regChange(cm);
            clearCaches(cm);
            setTimeout(function() {
                updateScrollbars(cm)
            }, 100)
        }

And the updateScrollbars call here changes clientHeight to 594 while sWidth remains 0.

Does this help?

@marijnh
Copy link
Member

marijnh commented Dec 15, 2015

The way setValue could cause sWidth to become zero is still somewhat mysterious. Can you set up a minimal HTML page that triggers this?

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

2 participants