419270 - Elements with position:fixed jump while scrolling

Status: RESOLVED FIXED

(Core :: Web Painting, defect)

Description

[Peter Weilbacher]

Page elements that are set to be fixed using CSS position:fixed should really stay fixed while scrolling. On Linux trunk builds and OS/2 branch builds they do, on OS/2 trunk builds they don't. 

Pages that show the problem:
- http://ninjakitten.us
  The whole header jumps around when scrolling
- http://weilbacher.org/Mozilla/builds.html
  While the fixed content (the yellow menu on the left in the default style)
  does stay fixed, the surrounding black border jumps around when scrolling.

I suspect that both of these are related and that this has something to do with cairo GFX or a widget change that we missed on OS/2 but I don't really know where to start debugging this...

Comment 1

[Walter Meinl]

(In reply to comment #0)
> I suspect that both of these are related and that this has something to do with
> cairo GFX or a widget change that we missed on OS/2 but I don't really know
> where to start debugging this...
> 
Its not cairo, because minefield-alpha2 has the same problem and IIRC this was still "normal" gfx. BTW, minefield-alpha2 is terribly slow on ninjakitten.us

Comment 2

[Walter Meinl]

Attachment: this works

searching through widget\src I found bug343430 that sounds very similar. A fix was done for linux and windows. Actually, the patch here tries to do the same for OS/2 what was done for windows (see https://bugzilla.mozilla.org/attachment.cgi?id=265542). So far it works.

Comment 3

[Peter Weilbacher]

Here it works to fix the bug in question but causes heavy repaint problems in the other parts of the pages that contain position:fixed elements. Not sure if that is because I have other stuff in my tree, because the patch itself looks fine to me.

Comment 4

[Walter Meinl]

Comment on attachment 305403 [details]
this works

clearing request, sigh, you're right this works only with one element. Here is a better testcase with two fixed elements
http://www.howtocreate.co.uk/fixedPosition.html
I tried also the gtk2 solution in the attachment from bug343430 https://bugzilla.mozilla.org/attachment.cgi?id=265542
This would work with the testcases mentioned, also with two fixed elements.... but then URLs started from the menus don't get called with a single left button click only with the right button using the context menu. Other menu functions like bookmark this page do work with the left mouse button.

Comment 5

[Walter Meinl]

Attachment: alternative

Tried the gtk2 solution again when I realized that the menu problem was not caused by the patch but was probably related to - Bug 419549 - click bookmark item does not work. I'm not going for review now, but maybe Peter you have some time to test it, maybe with even a better testcase. Scrolling of other elements seems to be affected by this solution (to be seen best with the test page posted in my last comment), the monospace textboxes scroll delayed.

Comment 6

[Peter Weilbacher]

Comment on attachment 305832 [details] [diff] [review]
alternative

Yes, from quick testing with a Firefox debug build this seems to work. Will put it in the SeaMonkey that I use daily to see if it causes anything bad.

Comment 7

[Peter Weilbacher]

Comment on attachment 305832 [details] [diff] [review]
alternative

OK, went to lots of position:fixed files and it seems to work for all of them. Although I really don't understand why this time the GTK2 solution would work and not the Windows one that we use in most other cases...

Comment 8

[Peter Weilbacher]

Walter, when you do the work and upload a patch you should in most cases tick the "take bug" box, so that everyone can see how much time you devote to all this. :-)

Comment 9

[Peter Weilbacher]

Comment on attachment 305832 [details] [diff] [review]
alternative

OS/2 only change in a cross-platform file.

Comment 10

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 305832 [details] [diff] [review]
alternative

OS2-only fix, a1.9b4=beltzner

Comment 11

[Walter Meinl]

(In reply to comment #7)
> (From update of attachment 305832 [details] [diff] [review])
> Although I really don't understand why this time the GTK2 solution would work
> and not the Windows one that we use in most other cases...
> 
Maybe I've should have posted the comment of the checkin for this bug:

Bug 343430. Reduce the area we scroll on Windows to reduce flicker by excluding areas that shouldn't be moving. On Linux, since we can't control the area we scroll, just disable accelerated scrolling in that case and repaint everything

I guess the gtk2 version works for OS/2 since it's a generic solution. Could it be that the windows solution didn't work or worked only partially, because there is an interference with the drag'n'drop in the os2 function? Here's the diff of that respective part of nsWindow.cpp

--- widget/src/windows/nsWindow.cpp      2008-02-22 08:13:31.000000000 +0100
+++ widget/src/os2/nsWindow.cpp  2008-02-09 23:26:06.000000000 +0100
@@ -2885,49 +2261,77 @@
 //XXX Scroll is obsolete and should go away soon
 NS_METHOD nsWindow::Scroll(PRInt32 aDx, PRInt32 aDy, nsRect *aClipRect)
 {
-  RECT  trect;
+  RECTL rcl;
 
   if (nsnull != aClipRect)
   {
-    trect.left = aClipRect->x;
-    trect.top = aClipRect->y;
-    trect.right = aClipRect->XMost();
-    trect.bottom = aClipRect->YMost();
-  }
+    rcl.xLeft = aClipRect->x;
+    rcl.yBottom = aClipRect->y + aClipRect->height;
+    rcl.xRight = rcl.xLeft + aClipRect->width;
+    rcl.yTop = rcl.yBottom + aClipRect->height;
+    NS2PM( rcl);
+    // this rect is inex
+  }
+
+    // this prevents screen corruption while scrolling during a
+    // Moz-originated drag - during a native drag, the screen
+    // isn't updated until the drag ends. so there's no corruption
+  HPS hps = 0;
+  CheckDragStatus(ACTION_SCROLL, &hps);
+
+  WinScrollWindow( mWnd, aDx, -aDy, aClipRect ? &rcl : 0, 0, 0,
+                   0, SW_SCROLLCHILDREN | SW_INVALIDATERGN);
+  Update();
+
+  if (hps)
+    ReleaseIfDragHPS(hps);
 
-  ::ScrollWindowEx(mWnd, aDx, aDy, NULL, (nsnull != aClipRect) ? &trect : NULL,
-                   NULL, NULL, SW_INVALIDATE | SW_SCROLLCHILDREN);
-  // Invalidate all child windows that aren't ours; we're moving them, and we
-  // expect them to be painted at the new location even if they're outside the
-  // region we're bit-blit scrolling. See bug 387701.
-  ::EnumChildWindows(GetWindowHandle(), nsWindow::InvalidateForeignChildWindows, NULL);
-  ::UpdateWindow(mWnd);
   return NS_OK;
 }
 
 NS_IMETHODIMP nsWindow::ScrollWidgets(PRInt32 aDx, PRInt32 aDy)

Comment 12

[Peter Weilbacher]

Walter, no, I don't think so. I looked at CheckDragStatus and that really only does something if you drag while you scroll. Unless gDragStatus would get set by mistake at some point (but then lots of other stuff would be messed up).

So, I've checked in fix from attachment 305832 [details] [diff] [review] now. In case you keep experimenting and find a better solution, please reopen.