User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a6pre) Gecko/20070602 Firefox/3.0a6pre Build Identifier: Mozilla/5.0 (Windows XP; en-US; rv:1.9a6pre) Gecko/20070602 Firefox/3.0a6pre The fix for bug 343430 gives rise to some visual glitches while scrolling pages with position:fixed elements. This is most obvious on: http://www.csszengarden.com/?cssfile=/202/202.css&page=0 Scrolling up at a medium to fast speed will cause random black horizontal lines that disappear as soon as scrolling stops. See also: http://www.seisomapaikka.com/esiintyjat/ Scrolling up here at a fast clip causes yellow horizontal lines. I think the problem is that the use of nsRect::ScaleRoundOut() in nsScrollPortView::Scroll() is making the clipping rect for scrolling too big and is causing parts of the fixed elements to get scrolled. Replacing ScaleRoundOut() with ScaleRoundIn(), fixes the glitches and does not seem to regress any of the test cases in bug 343430, bug 324819, and their various duplicates. I am assuming that ScaleRoundOut() will eventually be called on updateRegion so ScaleRoundOut() on the clipping rect leads to overlap and that ScaleRoundIn() on the clipping rect will not leave any areas that have not been scrolled or invalidated. Reproducible: Always

Use ScaleRoundIn() instead of ScaleRoundOut() when creating the scrolling clipping rect.

Bug re-created at http://www.csszengarden.com/?cssfile=/202/202.css&page=0 using both Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070601 Minefield/3.0a5pre ID:2007060104 [cairo] and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a6pre) Gecko/20070602 Minefield/3.0a6pre

You should request review on the patch from Robert O'Callahan (roc@oc).

Testcase-o-rama: http://www.w3.org/Style/CSS/ Standard style "Gold": Already described problem (white lines when scrolling up). Alternate style "Blue shadows": While scrolling down the bottom-border of the fixed positioned menu flickers.

I think this is the right idea, but what you should do here is ScaleRoundIn on biggestRect to compute toScroll and then scale it out again before subtracting biggestRect from regionToScroll, to ensure that we're actually removing exactly the rect we're going to scroll. Does that make sense?

Like this?

Yes. Does that fix the bug? + toScroll = biggestRect.ScaleRoundIn(1.0/aP2A); I'd prefer this as + biggestRect.ScaleRoundIn(1.0/aP2A); + toScroll = biggestRect; to make it clear that ScaleRoundIn is mutating biggestRect.

Updated for comments. Yes, it does fix the problem and does not regress any of the test cases I have been using.

Comment on attachment 267234 [details] [diff] [review] more review great!

I think I also see this bug with this testcase: https://bugzilla.mozilla.org/attachment.cgi?id=267052

Checking in nsScrollPortView.cpp; /cvsroot/mozilla/view/src/nsScrollPortView.cpp,v <-- nsScrollPortView.cpp new revision: 3.86; previous revision: 3.85 done Checked into trunk. Thanks for the patch, Stephen!

Verified fixed on the various pages/testcases, using: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a6pre) Gecko/20070605 Minefield/3.0a6pre