132.59 KB, image/png
150.17 KB, application/x-xpinstal
3.49 KB, patch
|Details | Diff | Splinter Review|
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.
Eddy, can you see if there's a platform issue here?
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.
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.
Eddy contacted me on email about this issue, and I produced this quick screencast of it: http://www.youtube.com/watch?v=KIM1mfuI72w
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.
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.
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.
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.
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.
Man this bug is getting epic. Thanks Eddy for following up!
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.
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 )
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.
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.
Created attachment 581863 [details] [diff] [review] fix
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.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #14) That's awesome! Really appreciate you taking a look at this issue.
(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!
Note that scrolling a panel that's clipped to a border-radius will not be fast.
https://hg.mozilla.org/integration/mozilla-inbound/rev/4a63669b8aa4 Backed out because the test failed on Linux.
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
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
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
That try push is green. https://hg.mozilla.org/integration/mozilla-inbound/rev/8b83a0ecb986
If you need this on FF11 (or 10) please nominate it.
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 on attachment 581863 [details] [diff] [review] fix You'll want this on Aurora as well.
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.
I think we can leave the noms in place
Comment on attachment 581863 [details] [diff] [review] fix [triage comment] Approved for aurora and beta
[triage comment] I should say why: Fixes and issue on reddit, fixes display corruption on https://builder.addons.mozilla.org
Works for me in the latest Aurora build, using the builder link in the original report.
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.
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