1265807 - Cannot scroll with scrollbar in CodeMirror instances

Status: VERIFIED FIXED

(DevTools :: Source Editor, defect, P1)

Description

[arni2033]

>>>   My Info:   Win7_64, Nightly 48, 32bit, ID 20160416030220
STR:
0. Set dark devtools theme, close devtools.
1. Open https://www.mozilla.org/firefox/nightly/firstrun/?oldversion=45.0.1
2. Open any scrollable script in debugger
3. Hover mouse over scrollbar thumb, try to scroll the script vertically

AR:  Text is selected
ER:  The script should be scrolled

Note:  I can reproduce on light devtools theme, will provide STR if necessary

This is regression from bug 1156450. Regression range:
> https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ca78a5af2576347a318789f34060e0abfd5c9640&tochange=637e6dce9a3f1210163cfa513292146a7367acd8

Comment 1

[arni2033]

Note, this is not necessarily something wrong with codemirror - it may be an issue with floating scrollbars (because UA style has changed in bug 1156450)

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

Joe is this on your radar?

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

Can you still repro this?

Comment 4

[Joe Walker [:jwalker]]

Dark themes and scrollbars...
gl: I have a feeling this is a dupe?
Could you take a look and prioritize please?

Comment 5

[arni2033]

>>>   My Info:   Win7_64, Nightly 50, 32bit, ID 20160715063552 (2016-07-15)
Yes

Comment 6

[arni2033]

>>>   My Info:   Win7_64, Nightly 52, 32bit, ID 20160920030429 (2016-09-20)
Reproducible.

Comment 7

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Gabriel, this is tagged as a P1 bug. Is there a fix in the works? I'd be happy to uplift to Beta50 if deemed low risk.

Comment 8

[fremea]

More about it:
With dark theme, scroll bar can be handled in most part of UI (devtools-Inspector) except in some part:
===
1.Open devtools-Inspector
2.Click (ev) button next when available
3.Click on a script to unfold it and see see see Event Listener code
4.Except the fact the scroll bars are way too transparent and hard to see most of the time, you can NOT grab them to scroll [press left mouse button on scrollbar, then move mouse]
5.But if there is list of lots of event listener script, you can grab the scroll bars to browse through that list. It works in that case.
===
1.Open devtools-Inspector
2.Go to the CSS editor tab
3.You can NOT grab them to scroll [press left mouse button on scrollbar, then move mouse]
===
1.Open devtools-Inspector
2.Go to the Debugger tab
3.In the code panel, you can NOT grab them to scroll [press left mouse button on scrollbar, then move mouse]
===
Every other Code mirror editor like stylish code editor are affected by this BUG.

===
Please fix it and make the scroll bar for dark theme larger and more visible (more opacity)
===
setup:
Windows 10x64
Firefox 49.0 x64

related bugs:
Bug 1100622 - Scroll bar is hard to find in devtools
Bug 1197229 - CodeMirror editor inside event popup can't be scrolled with left mouse button (with dark theme active)

Comment 9

[Gabriel Luong [:gl] (ΦωΦ)]

(In reply to Ritu Kothari (:ritu) from comment #7)
> Hi Gabriel, this is tagged as a P1 bug. Is there a fix in the works? I'd be
> happy to uplift to Beta50 if deemed low risk.

Don't have a fix yet, but will be working on this after I fix another P1, but this is second from the top of my queue.

Comment 12

[infoplus007]

Scrollbar don't work in JS Fiddle when you try to grab them, only the "down" button and the scroll mouse button work.

URL I've used: http://jsfiddle.net/u26mK/8/

Screencast here:
http://screencast-o-matic.com/watch/cDXjr6j80r

Tested with FF49 under Win7/64 with and without E10
Work fine in Chrome.

Comment 13

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

:gl, are you able to look at this again?

Comment 16

[Ryan VanderMeulen [:RyanVM]]

This bug appears to be stuck. Any chance you can help get it moving again, Patrick?

Comment 17

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: 1265807.patch

Comment 18

[Julian Descottes [:jdescottes]]

Comment on attachment 8822565 [details] [diff] [review]
1265807.patch

Review of attachment 8822565 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Tested on Win7.
This fixes the scrollbar issues for
- Style Inspector 
- Debugger
- edit as HTML in markup view

I should mention there is still an issue in the event popup (cf comment 8).
For the event popup:
- fixed: clicking on the thumb no longer selects text
- fixed: clicking below or above the thumb scrolls as expected
- not fixed: dragging the thumb doesn't work

It is probably because we have overlapping scrollbars, one for the popup content and one for code mirror. 
Let's land this and either leave-open or open a follow-up.

Comment 21

[Patrick Brosset <:pbro>]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #16)
> This bug appears to be stuck. Any chance you can help get it moving again,
> Patrick?
Well, looks like it got moving again before I could reply.
Seems like only a try push is needed and then we could land this simple R+'d patch.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b12ad3ab9baa
Fix scrolling with the CodeMirror scroll r=jdescottes

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b12ad3ab9baa

Comment 24

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

[Tracking Requested - why for this release]: Seems important to uplift to repair scrolling in DevTools

Comment 25

[Julien Cristau [:jcristau]]

Tracking for 51 and 52.  Can we get the fix verified in the latest nightly?

Comment 26

[Sebastian Zartner [:sebo]]

I can confirm the bug for 52.0a2 2017-01-11 and that it's fixed in 53.0a1 2017-01-11 (using the new debugger UI). For what it's worth, I've tested the light and dark theme on Window 10.

Sebastian

Comment 27

[Timothy Nikkel (:tnikkel)]

Is this a bug that would be visible in regular web content on platforms that have floating scrollbars?

Comment 28

[Gabriel Luong [:gl] (ΦωΦ)]

(In reply to Timothy Nikkel (:tnikkel) from comment #27)
> Is this a bug that would be visible in regular web content on platforms that
> have floating scrollbars?

This could be the case.

Comment 29

[Timothy Nikkel (:tnikkel)]

(In reply to Gabriel Luong [:gl][1 biz day review guarantee] (ΦωΦ) from comment #28)
> (In reply to Timothy Nikkel (:tnikkel) from comment #27)
> > Is this a bug that would be visible in regular web content on platforms that
> > have floating scrollbars?
> 
> This could be the case.

Would we be able to make a testcase for that? And a bug on file so we can fix it?

Comment 30

[Ryan VanderMeulen [:RyanVM]]

We're spinning the RC build for Fx51 today. If you want this included in the 51 release, it needs approval requests ASAP. And even then, it's not guaranteed at this point.

Comment 31

[Gabriel Luong [:gl] (ΦωΦ)]

(In reply to Timothy Nikkel (:tnikkel) from comment #29)
> (In reply to Gabriel Luong [:gl][1 biz day review guarantee] (ΦωΦ) from
> comment #28)
> > (In reply to Timothy Nikkel (:tnikkel) from comment #27)
> > > Is this a bug that would be visible in regular web content on platforms that
> > > have floating scrollbars?
> > 
> > This could be the case.
> 
> Would we be able to make a testcase for that? And a bug on file so we can
> fix it?

I assumed this might be happening if we had a CM instance inside an iframe, but that doesn't seem to the be the case. Also, I was lead to that previous conclusion because of Bug 1329785, but it seems we have filed a different platform issue related to that in regards of the scrollbar not working.

Comment 32

[Gerry Chang [:gchang]]

We've built 51 RC. This is too late for 51. Mark 51 won't fix.

Comment 33

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 8822565 [details] [diff] [review]
1265807.patch

Approval Request Comment
[Feature/Bug causing the regression]: 1156450
[User impact if declined]: Can't scroll in any of the codemirror instances of the devtools
[Is this code covered by automated tests?]: Nope
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: Yes.
1.Open devtools-Inspector
2.Go to the CSS editor tab
3.You can NOT grab them to scroll [press left mouse button on scrollbar, then move mouse]
===
1.Open devtools-Inspector
2.Go to the Debugger tab
3.In the code panel, you can NOT grab them to scroll [press left mouse button on scrollbar, then move mouse]
[List of other uplifts needed for the feature/fix]: 
[Is the change risky?]: No
[Why is the change risky/not risky?]: CSS Change only
[String changes made/needed]: None

Comment 34

[Julien Cristau [:jcristau]]

Comment on attachment 8822565 [details] [diff] [review]
1265807.patch

scrollbar fix in devtools, aurora52+

Comment 35

[Julien Cristau [:jcristau]]

Flagging for verification in aurora, see STR in comment 33.  Sebastian since you did the verification in nightly maybe you can help here?

Comment 36

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b8ec36898a7d

Comment 37

[Sebastian Zartner [:sebo]]

Confirmed that it's working in Aurora 52.0a2 2017-01-21.

Sebastian