Last Comment Bug 697215 - Cannot scroll panel on OS X
: Cannot scroll panel on OS X
Status: RESOLVED FIXED
[qa+][qa!:11]
: verified-beta
Product: Core
Classification: Components
Component: Layout (show other bugs)
: unspecified
: All All
: P2 normal (vote)
: mozilla12
Assigned To: Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
:
Mentors:
: 726501 (view as bug list)
Depends on: 714320
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-25 12:18 PDT by Jeff Griffiths (:canuckistani) (:⚡︎)
Modified: 2012-02-13 09:28 PST (History)
12 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
verified


Attachments
Screenshot showing the problem (132.59 KB, image/png)
2011-11-24 14:18 PST, Will Bamberg [:wbamberg]
no flags Details
test.xpi to reproduce the problem (150.17 KB, application/x-xpinstal)
2011-12-12 08:50 PST, Eddy Bruel [:ejpbruel]
no flags Details
fix (3.49 KB, patch)
2011-12-14 20:04 PST, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
mats: review+
christian: approval‑mozilla‑aurora+
christian: approval‑mozilla‑beta+
Details | Diff | Review

Description Jeff Griffiths (:canuckistani) (:⚡︎) 2011-10-25 12:18:02 PDT
To repro:

1. Open https://builder.addons.mozilla.org/addon/1022414/revision/3/

2. try to scroll vertically via either the mouse wheel or by grabbing the scroll bar.

Result: 

 - display corruption / content gets 'stuck' and does not display correctly.

Specifically, I am running 10.7 on an i7 MBAir.
Comment 1 Wes Kocher (:KWierso) 2011-10-27 11:37:15 PDT
Eddy, can you see if there's a platform issue here?
Comment 2 Eddy Bruel [:ejpbruel] 2011-11-11 14:56:17 PST
I cannot reproduce this bug. Using OSX 10.6.7 on my MacBook Pro, and running Firefox 7.0.1, I don't see any vertical scrollbars. If I make the window much smaller the vertical scrollbar appears, but when I use it I don't see anything that indicates display corruption.

With respect to content getting 'stuck': I cannot add or change text in the editor window. It looks like it's totally frozen.
Comment 3 Will Bamberg [:wbamberg] 2011-11-24 14:18:08 PST
Created attachment 576825 [details]
Screenshot showing the problem

It's reproducible for me too. See the attachment.

(In reply to Eddy Bruel [:ejpbruel] from comment #2)
> I cannot reproduce this bug. Using OSX 10.6.7 on my MacBook Pro, and running
> Firefox 7.0.1, I don't see any vertical scrollbars. If I make the window
> much smaller the vertical scrollbar appears, but when I use it I don't see
> anything that indicates display corruption.
> 
> With respect to content getting 'stuck': I cannot add or change text in the
> editor window. It looks like it's totally frozen.

I think there's a misunderstanding, or a couple of implicit steps between Jeff's 1 and 2. The link he's attached is a link to a Builder add-on that can be used to repro it, not a direct repro. You need to run the add-on, then click the Firefox widget to get it to show the panel, then scroll in the panel.
Comment 4 Jeff Griffiths (:canuckistani) (:⚡︎) 2011-11-24 15:02:53 PST
Eddy contacted me on email about this issue, and I produced this quick screencast of it:

http://www.youtube.com/watch?v=KIM1mfuI72w
Comment 5 Eddy Bruel [:ejpbruel] 2011-11-24 15:46:03 PST
Disregard my last comment. I can definitely reproduce it now. I was indeed looking in the wrong place, but Jeff's screencast quickly pointed out my mistake. Thanks, Jeff.

From the looks of it, this is an issue with the renderer. Sadly, I don't know anything about that part of the platform, so I have no idea what could cause this behavior. Next step will be to ask around on irc until I find somebody who can point me in the right direction.
Comment 6 Jeff Griffiths (:canuckistani) (:⚡︎) 2011-11-24 16:20:22 PST
Well, it's definitely a mac problem, and definitely not a problem on other platforms. Hopefully that and the video narrow the problem scope down enough.
Comment 7 Paul Sawaya 2011-11-25 13:58:32 PST
Did a git-bisect and found these changes responsible: https://github.com/mozilla/addon-sdk/commit/97b79af12284454986b4b87590860ae6911c8564

Uncommenting those style changes fixes the issue on my system.
Comment 8 Eddy Bruel [:ejpbruel] 2011-12-12 08:50:00 PST
Created attachment 580927 [details]
test.xpi to reproduce the problem

I've attached a test.xpi to make it easy for the graphics team to reproduce
this issue. I've also made a couple of observations that might help in 
pinpointing the source of the problem:

1. Install the add-on by dragging the test.xpi to the browser window. This 
   should cause a small Firefox icon to appear in the add-on bar (usually in the 
   lower-right corner of the window).
2. Open a panel by clicking on the Firefox icon. The panel's content should be
   http://news.google.com.
3. Slowly scroll down all the way to the bottom by dragging the vertical
   scrollbar. Notice how the panel's content isn't updated until you've almost
   reached the bottom.
4. Slowly scroll up all the way to the top again. Notice how the panel's content
   is initially updated until you're some distance from the bottom of the page.
5. Move your mouse over the panel at random. Notice how this causes the panel's
   content to be updated in areas where the cursor touched it.
6. Make sure the panel is scrolled up all the way to the top. Scroll right by 
   dragging the horizontal scrollbar. Notice how this time, the panel's
   content is updated properly.
7. Make sure the panel is scrolled up all the way to the top *and* all the way
   to the right. Scroll down again. Notice how this time, the panel's content
   is updated properly.

Summarizing: it looks like the panel's content is not redrawn properly when 
scrolling vertically, unless you've also scrolled completely to the right. The 
way the panel is updated when moving over it with your mouse suggests that the 
renderer is at least aware of the position of the scrollbars, so the problem is 
not that the renderer thinks we didn't move. Rather, the problem seems to be 
that some part of the repaint mechanism is not triggered in response to a scroll 
event.

My next step will be to do some stack tracing in order to figure out what part 
of the repaint mechanism is not triggered.

Oh, one final remark. Making the panel bigger so that horizontal scrolling is not possible does not fix the problem. The only thing that does by making it
impossible to scroll all the way to the right, it's become impossible to get vertical scrolling to work in the way I just described.
Comment 9 Eddy Bruel [:ejpbruel] 2011-12-12 17:12:50 PST
I've managed to pinpoint the problem to nsDisplayList::ComputeVisibilityForSublist. For some reason, after this function is called, the visible region of the panel is reduced to 0, which causes it not to be redrawn.
Comment 10 Jeff Griffiths (:canuckistani) (:⚡︎) 2011-12-12 23:32:55 PST
Man this bug is getting epic. Thanks Eddy for following up!
Comment 11 Eddy Bruel [:ejpbruel] 2011-12-13 17:53:21 PST
Disregard my latest comment; it looks like it might have been premature.

I've worked with tn from the gfx team tonight and he gave me a rundown on how buffers and layers actually work. Based on that information, I now doubt that the problem is with the visible region.

What we've discovered though is that FrameLayerBuilder::DrawThebesLayer is called 3 times for each paint event when scrolling is working properly, but only 2 times when it is not. In other words, one layer is not being rendered. I have no idea why this is the case, but tn seems to have a hunch. From what I know, the layer that is not being rendered is a dependent layer, which is something that we do not expect. According to tn, the only way that layer could end up being dependent is by using border-radius, which is exactly the style property that caused the problem in the first place! This looks promising.

tn needs some time to think on the problem. I'm having a day off tomorrow, so my plan is to contact him again this thursday and see what else we can come up with.
Comment 12 Jeff Griffiths (:canuckistani) (:⚡︎) 2011-12-14 17:09:22 PST
I've debugged the css on the reddit .compact page and reduced it to a specific set of 3 css rules that, if they are all commented out, allow scrolling to work:

https://github.com/canuckistani/bug_697215/blob/master/data/compact.xrmhC5EAhEY.css#L27-38

( they are the ones that are currently commented out in that block )
Comment 13 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-14 19:18:48 PST
If I open up a plain Firefox build, run DOM Inspector and set border-radius:50px on xul:browser in the chrome document, then I get rounded corners on my content document and I see the bug --- the window doesn't repaint when I scroll.
Comment 14 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-14 20:04:10 PST
The bug is that in nsGfxScrollFrame.cpp, CanScrollWithBitBlt is returning true even though the scrollframe is inside a frame that's clipping to a border-radius. It should return false in that situation.

It returns true because its logic assumes that we clip to border-radius only for scrollframes. But in fact replaced elements with overflow not "visible" don't induce scrollframes and we also clip to border-radius for them. Most such elements can't contain scrollframes, but <iframe>s can.
Comment 15 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-14 20:04:59 PST
Created attachment 581863 [details] [diff] [review]
fix
Comment 16 Mats Palmgren (:mats) 2011-12-14 22:18:20 PST
Comment on attachment 581863 [details] [diff] [review]
fix

> ... replaced elements with overflow not "visible"
> don't induce scrollframes and we also clip to border-radius for them. Most
> such elements can't contain scrollframes, but <iframe>s can.

Worth documenting with a code comment to explain the eReplaced test there, IMO.
Comment 17 Jeff Griffiths (:canuckistani) (:⚡︎) 2011-12-14 23:17:44 PST
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #14)
That's awesome! Really appreciate you taking a look at this issue.
Comment 18 Eddy Bruel [:ejpbruel] 2011-12-15 04:55:39 PST
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #14)
A big thanks from me too for helping us out on this issue!
Comment 19 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-18 00:55:52 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/026ec6345ae3
Comment 20 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-18 00:56:33 PST
Note that scrolling a panel that's clipped to a border-radius will not be fast.
Comment 21 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-18 13:56:25 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/4a63669b8aa4

Backed out because the test failed on Linux.
Comment 22 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-18 14:09:59 PST
On Linux we didn't scroll down far enough?
Maybe the IFRAME content in the test isn't quite tall enough to be scrolled? I dunno. Did a try push with taller content.

https://tbpl.mozilla.org/?tree=Try&rev=e3839c0ccdaf
Comment 23 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-18 17:41:13 PST
The problem is that the test always removed the reftest-wait in the first doTest because I omitted an early return. Pushed a try build with that fixed.
https://tbpl.mozilla.org/?tree=Try&rev=fc1b7ac48457
Comment 24 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-19 17:32:49 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/4a81c89bb466
Comment 25 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-19 21:20:28 PST
Backed out again, another (different) test failure. Just a couple of pixels this time. Trying disabling scrollbars to make that go away.

http://hg.mozilla.org/integration/mozilla-inbound/rev/a9293d72f97a

https://tbpl.mozilla.org/?tree=Try&rev=b7361d5b7d4b
Comment 26 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-20 00:57:16 PST
https://tbpl.mozilla.org/?tree=Try&rev=f64df0a61b31
Comment 27 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-20 11:45:30 PST
That try push is green.

https://hg.mozilla.org/integration/mozilla-inbound/rev/8b83a0ecb986
Comment 28 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-20 14:19:47 PST
If you need this on FF11 (or 10) please nominate it.
Comment 29 Jeff Griffiths (:canuckistani) (:⚡︎) 2011-12-20 15:04:30 PST
roc: thanks. I'm going to nominate it for FF 10, the issue affects basic functionality in the SDK in a way that renders SDK component behaviour inconsistent on different platforms with a pretty common use case.
Comment 30 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-20 15:06:35 PST
Comment on attachment 581863 [details] [diff] [review]
fix

You'll want this on Aurora as well.
Comment 31 Jeff Griffiths (:canuckistani) (:⚡︎) 2011-12-20 15:28:06 PST
roc: It occurs to me that this patch hasn't landed yet - is it too early to add the nomination flag? I suppose I should wait for it to land and sit on nightly for a few days and, you know, do some testing.
Comment 32 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-20 15:31:25 PST
I think we can leave the noms in place
Comment 33 Ed Morley [:emorley] 2011-12-21 04:31:41 PST
https://hg.mozilla.org/mozilla-central/rev/8b83a0ecb986
Comment 34 christian 2011-12-21 15:49:33 PST
Comment on attachment 581863 [details] [diff] [review]
fix

[triage comment]
Approved for aurora and beta
Comment 35 christian 2011-12-21 15:50:27 PST
[triage comment]
I should say why: Fixes and issue on reddit, fixes display corruption on  https://builder.addons.mozilla.org
Comment 37 Jeff Griffiths (:canuckistani) (:⚡︎) 2011-12-28 17:09:00 PST
Works for me in the latest Aurora build, using the builder link in the original report.
Comment 38 christian 2012-01-03 22:55:59 PST
We backed this out in http://hg.mozilla.org/releases/mozilla-beta/rev/01ef9195f79b to see if it caused bug 714320. If not, we'll put it back in.
Comment 39 Paul Silaghi, QA [:pauly] 2012-02-07 05:54:18 PST
This is verified fixed on Firefox 11b1:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0) Gecko/20100101 Firefox/11.0
Comment 40 Jeff Griffiths (:canuckistani) (:⚡︎) 2012-02-13 09:28:52 PST
*** Bug 726501 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.