Steps: 1. Scroll down in view of week 2. Click to create appointment Expected results: Appointment time is the time that was clicked Actual results: Appointment time offset by some amount which depends on where you are scrolled. The bug is in nsGenericHTMLElement::GetOffsetRect; bug 328881 changed the way adding up frame offsets works. Testcase coming up.

*** Bug 330619 has been marked as a duplicate of this bug. ***

1) Ignore scroll position while compute offset rects for boxobjects and HTML elements 2) Move code to union continuation rects into a shared function

FWIW, I can confirm that this does indeed fix the google calendar regression.

erm. why isn't this in cvs? or is it?

(In reply to comment #5) > erm. why isn't this in cvs? or is it? Because it's still waiting for r/sr?

roc, once this lands the fixed-pos code in nsHTMLReflowState::CalculateHypotheticalBox should also switch to nsLayoutUtils::GetOffsetIgnoringScrolling, right?

Yes.

Comment on attachment 220044 [details] [diff] [review] fix Tossing the review to Boris.

Comment on attachment 220044 [details] [diff] [review] fix >Index: layout/base/nsLayoutUtils.cpp >+nsLayoutUtils::GetUnionOfAllRects(nsIFrame* aFrame) Should this guard against aFrame->GetParent() being null? It'll crash at the moment if that happens. >Index: layout/base/nsLayoutUtils.h >+ * @return the offset of aFrame from its parent, as if it was scrolled to s/was/were/ r+sr=bzbarsky with those nits; let's do the reflow state change in a followup.

(In reply to comment #10) > (From update of attachment 220044 [details] [diff] [review] [edit]) > >Index: layout/base/nsLayoutUtils.cpp > >+nsLayoutUtils::GetUnionOfAllRects(nsIFrame* aFrame) > > Should this guard against aFrame->GetParent() being null? It'll crash at the > moment if that happens. Yes. I'll just return aFrame->GetRect() in that case (there can't be continuations).

checked in

This may have caused a Tp regression on btek :-(

I wonder if this is also responsible for Thunderbird crazyhorse orange.

(In reply to comment #14) > I wonder if this is also responsible for Thunderbird crazyhorse orange. > Evidently not.

Backing out seemed to show that this is indeed the cause of the btek regression :-(. But do these GetOffsetRect functions actually get called during Tp? Through XUL maybe? Tomorrow I'll try relanding just the HTML part of the fix.

I wouldn't be surprised if the Tp pages call those functions... you could breakpoint and run Tp and see?

I added printfs to nsBoxObject::GetOffsetRect and nsGenericHTMLElement::GetOffsetRect. They don't get hit during Tp :-|.

Alright, here's a new approach. It should be faster than the previous try, just in case that matters, and it also changes only nsBoxObject for now. Basically I've added a custom method to nsIFrame to avoid the QueryInterface call. If this lands, and it doesn't perturb btek or anyone else, then I'll make another patch for nsGenericHTMLElement offsetX/Y.

Comment on attachment 225540 [details] [diff] [review] new patch >Index: layout/generic/nsGfxScrollFrame.h >+ if (aChild == mInner.GetScrolledFrame()) pt += GetScrollPosition(); ... >+ if (aChild == mInner.GetScrolledFrame()) pt -= GetScrollPosition(); Those can't both be right. The += should be what you want, right? r+sr=bzbarsky with that fixed.

Yes, += is what I want, and what I tested :-|

checked in. Waiting for performance results...

No btek regression this time. This regression is fixed.

Verified FIXED for me using build 2006-06-19-10 of SeaMonkey trunk under Windows XP with the Google Calendar application at http://www.google.com/calendar/render?pli=1. Dates can now be correctly chosen.