383010 - Visual glitches while scrolling pages with position:fixed elements

Status: VERIFIED FIXED

(Core :: Web Painting, defect)

Description

[Stephen Moehle]

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

Comment 1

[Stephen Moehle]

Attachment: potential fix

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

Comment 2

[Chris Borrell]

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

Comment 3

[Adam Guthrie]

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

Comment 4

[Daniel.S]

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.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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?

Comment 6

[Stephen Moehle]

Attachment: review comments

Like this?

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 8

[Stephen Moehle]

Attachment: more review

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

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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

great!

Comment 10

[Martijn Wargers (dead)]

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

Comment 11

[Martijn Wargers (dead)]

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!

Comment 12

[Martijn Wargers (dead)]

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