328881 - Set correct offsets on scrolled frames

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

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

Prior to removing views, we need to set the correct position on scrolled frames. Currently scrolled frames are placed at (0,0) and the position is basically ignored because stuff like nsIFrame::GetOffsetTo uses view positions in this case.  I'll attach a patch that sets the "true" position on scrolled frames. I want to land this now just to ensure that any regressions from code mistakenly relying on the (0,0) position are easily caught.

Comment 1

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

Attachment: fix

Comment 2

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

checked in

Comment 3

[Boris Zbarsky [:bzbarsky]]

Looks like fixed-pos frames were depending on the (0,0) thing.  I'll fix them in bug 331729.

Comment 4

[Ria Klaassen (not reading all bugmail)]

Could this bug have caused Bug 330073 ?

Comment 5

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

Try backing it out to see... it's very small.

Comment 6

[Robert Strong (they/them - no direct email)]

I backed this out and I was no longer able to reproduce Bug 330073.

Comment 7

[Boris Zbarsky [:bzbarsky]]

So I just ran into the fact that updating frame coords in ScrollPositionDidChange is a little late.  In my case, nsScrollPortView::ScrollToImpl changes the view's position, then does various scroll analysis (e.g. CanScrollWithBitBlt and the like) _then_ calls ScrollPositionDidChange.  We need to update the frame's rect before the CanScrollWithBitBlt call if we want to wean nsIFrame::GetOffsetTo off of views.

Comment 8

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

My compositor patch does that. (In fact it eviscerates nsScrollPortView and puts most of the logic in nsGfxScrollFrame.)