nsHTMLReflowState::ApplyRelativePositioning, before changing the position given, should store it in a frame property. There are already a number of calculations that need this "normal" position and get it by using |GetPosition() - GetRelativeOffset()|. Storing it explicitly is cleaner and will work when sticky positioning changes the frame's position. This stored position will also be used in sticky positioning calculations.

Is this instead of the offset or in addition to the offset?

It looks like ComputedOffsetProperty will then be unnecessary, yes. I only notice one use that doesn't directly follow the pattern I've noted, but of course that can be reworked using GetPosition and GetNormalPosition. http://mxr.mozilla.org/mozilla-central/source/layout/base/nsCSSRendering.cpp#3952

ok, sounds good. I'm not sure if GetNormalPosition() is the best name, though I don't have a better one off the top of my head.

Maybe GetStaticPosition(), though I'm not crazy about that, either?

ReflowPosition (or FlowPosition) might also work, though I still favor NormalPosition.

That worked out well enough. Most of the changes here are of the form |GetPosition() - GetRelativeOffset()| => |GetNormalPosition()|, but a few are slightly more involved, and ApplyRelativePositioning had to be changed to take the whole frame as a parameter. A try push with this and the patches from bug 898797: https://tbpl.mozilla.org/?tree=Try&rev=49ea49b2d4f2

Oh, plus this similar bit. (Still not sure I could safely store computed sticky offsets in mComputedOffsets, but those I might want to only store in a frame property anyway.)

(I'll fold the patches together before checking in.)

Folded patches and rebased onto m-c (with the nsTextFrameThebes move).

Comment on attachment 785820 [details] [diff] [review] Store normal frame position before applying relative positioning. nsFlexContainerFrame.cpp: Looks ok to me, but maybe run it by dholbert? nsFloatManager.cpp: Probably best to initialize as: nsRect region(aFloat->GetNormalPosition(), aFloat->GetSize()); nsFrame.cpp: Given the code that was there before, I suspect it might be faster for GetNormalPosition to check IsRelativelyPositioned() before getting the frame property, although maybe frame property access is faster now. Maybe at least leave a comment suggesting that it might be faster? r=dbaron with that (though I'm still wondering about the name, but ok with it)

Comment on attachment 785820 [details] [diff] [review] Store normal frame position before applying relative positioning. >+++ b/layout/generic/nsFlexContainerFrame.cpp >@@ -2408,20 +2408,20 @@ nsFlexContainerFrame::Reflow(nsPresConte >- // (We subtract mComputedOffsets.top because we don't want relative >- // positioning on the child to affect the baseline that we read from it). >- flexContainerAscent = physicalPosn.y + childDesiredSize.ascent - >- childReflowState.mComputedOffsets.top; >+ // (We don't want relative positioning on the child to affect the >+ // baseline that we read from it). >+ flexContainerAscent = curItem.Frame()->GetNormalPosition().y + >+ childDesiredSize.ascent; Just one nit -- could you tweak the comment to say: // We use GetNormalPosition() instead of GetRect() because we don't want instead of just "We don't want"? (or something to that effect) Otherwise it's not really clear what part of the code the comment is talking about, unless the reader knows what GetNormalPosition() is. And most readers won't recognize that at first, since GetNormalPosition is all-new :)

Done. (In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from comment #10) > nsFrame.cpp: > > Given the code that was there before, I suspect it might be faster > for GetNormalPosition to check IsRelativelyPositioned() before getting > the frame property, although maybe frame property access is faster now. > Maybe at least leave a comment suggesting that it might be faster? Given that most of the current callers of GetRelativeOffset didn't bother passing aDisplay, I'd guess the performance impact would be pretty minimal. But I added a comment, and see also bug 404215 comment 42.