Cannot scroll panel on OS X

RESOLVED FIXED in Firefox 11

Status

()

Core
Layout
P2
normal
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: canuckistani, Assigned: roc)

Tracking

(Depends on: 1 bug, {verified-beta})

unspecified
mozilla12
verified-beta
Points:
---

Firefox Tracking Flags

(firefox10 affected, firefox11 verified)

Details

(Whiteboard: [qa+][qa!:11])

Attachments

(3 attachments)

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?
Assignee: nobody → ejpbruel
Priority: -- → P2
Hardware: x86 → All
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.

Comment 7

6 years ago
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 )

Updated

6 years ago
Attachment #580927 - Attachment mime type: text/plain → application/x-xpinstal
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
Assignee: ejpbruel → roc
Attachment #581863 - Flags: review?(matspal)
Assignee: roc → nobody
Component: General → Layout
Product: Add-on SDK → Core
QA Contact: general → layout
OS: Mac OS X → All
Assignee: nobody → roc
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.
Attachment #581863 - Flags: review?(matspal) → review+
(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!
https://hg.mozilla.org/integration/mozilla-inbound/rev/026ec6345ae3
Whiteboard: [inbound]
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.
Whiteboard: [inbound]
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
https://hg.mozilla.org/integration/mozilla-inbound/rev/4a81c89bb466
Whiteboard: [inbound]
Whiteboard: [inbound]
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
https://tbpl.mozilla.org/?tree=Try&rev=f64df0a61b31
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.
Attachment #581863 - Flags: approval-mozilla-beta?
Comment on attachment 581863 [details] [diff] [review]
fix

You'll want this on Aurora as well.
Attachment #581863 - Flags: approval-mozilla-aurora?
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
https://hg.mozilla.org/mozilla-central/rev/8b83a0ecb986
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12

Comment 34

6 years ago
Comment on attachment 581863 [details] [diff] [review]
fix

[triage comment]
Approved for aurora and beta
Attachment #581863 - Flags: approval-mozilla-beta?
Attachment #581863 - Flags: approval-mozilla-beta+
Attachment #581863 - Flags: approval-mozilla-aurora?
Attachment #581863 - Flags: approval-mozilla-aurora+

Comment 35

6 years ago
[triage comment]
I should say why: Fixes and issue on reddit, fixes display corruption on  https://builder.addons.mozilla.org

Comment 36

6 years ago
http://hg.mozilla.org/releases/mozilla-aurora/rev/b64a8da1413a
http://hg.mozilla.org/releases/mozilla-beta/rev/7a0d36baf5be
status-firefox10: --- → fixed
status-firefox11: --- → fixed
Whiteboard: [qa+]
Works for me in the latest Aurora build, using the builder link in the original report.

Comment 38

6 years ago
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.
status-firefox11: fixed → affected

Updated

6 years ago
Depends on: 714320
status-firefox10: fixed → affected
status-firefox11: affected → fixed
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
status-firefox11: fixed → verified
Keywords: verified-beta
Whiteboard: [qa+] → [qa+][qa!:11]
Duplicate of this bug: 726501
You need to log in before you can comment on or make changes to this bug.