(Previous discussion in bug 886646 comment 9 and following) CSS relative positioning is applied separately in several places: nsBlockReflowContext, nsCanvasFrame, nsFlexContainerFrame, and nsLineLayout. This should be factored out to a separate method, and sticky positioning can later be added there. Implementing this in a non-static method on nsHTMLReflowState works well except for the case of nsLineLayout. Using a static method and carrying along necessary bits of state works, but seems a bit messy.

Comment on attachment 775841 [details] [diff] [review] Refactor the application of relative positioning (v2, static method) >- nsContainerFrame::FinishReflowChild(mFrame, mPresContext, &aReflowState, mMetrics, x, y, 0); >+ nsContainerFrame::FinishReflowChild(mFrame, mPresContext, &aReflowState, mMetrics, position.x, position.y, 0); This line should get wrapped at slightly under 80 characters. What I realized I should have suggested is that you could have two methods, a static method and a non-static method, where the non-static method is just an inline function that calls the static one with the right parameters. That would be slightly preferable, if you don't mind -- it means changing a bit of stuff back to the way it was in the old patch. Sorry I wasn't clear about that the last time around. Also, in nsLineLayout, don't add an mStyleDisplay; instead, just use pfd->mFrame->StyleDisplay().

Yeah, that looks nicer. try push: https://tbpl.mozilla.org/?tree=Try&rev=11b2cf5448fd

Comment on attachment 776014 [details] [diff] [review] Refactor the application of relative positioning. >+ nsContainerFrame::FinishReflowChild(mFrame, mPresContext, &aReflowState, >+ mMetrics, position.x, position.y, 0); Line up mMetrics with mFrame on the previous line. >+ // If a relatively positioned element, adjust the position appropriately. >+ static void ApplyRelativePositioning(nsPoint *aPosition, >+ const nsStyleDisplay* aDisplay, >+ const nsMargin& aComputedOffsets); I might slightly prefer putting aPosition as the last parameter rather than the first, just because we have a tendency towards putting things that are output parameters (or in this case, in-out params) later in the parameter list (as a result of the XPCOM->C++ convention of having the return value, and thus out-param, as the last C++ argument). It's ok to leave that as-is, though. (Neither change should require a new try run; just check that it still compiles.) r=dbaron

Done.

Comment on attachment 776061 [details] [diff] [review] Refactor the application of relative positioning. Fixed some pointer stars.

Tests are all green on try.

Pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/556a557c7276

Hmm, we also make various use of the computed offsets via nsIFrame::GetRelativeOffset after storing them in ComputedOffsetProperty.