See testcase. Changing writing mode of the inner (yellow) <div>s should only affect the display of their content; the overall layout should not change. However, what actually happens is that when the writing mode of the inner <div>s is vertical, one of the *side* margins (right in the case of vertical-lr, left in the case of vertical-rl) appears *below* the inner block. (This is kinda like a display:block version of bug 1094914, which was about similar margin confusion on display:inline elements.)

One thing that I think is a problem here is the line at http://hg.mozilla.org/mozilla-central/annotate/d380166816dd/layout/generic/nsBlockReflowContext.cpp#l355 which combines a margin value from aReflowState (in the placed block's writing mode) with one from aMetrics (in the parent's writing mode). However, adding what looks like the appropriate conversion here, with something like - aBEndMarginResult.Include(aReflowState.ComputedLogicalMargin().BEnd(wm)); + aBEndMarginResult.Include(aReflowState.ComputedLogicalMargin(). + ConvertTo(parentWM, wm).BEnd(parentWM)); is not sufficient (the resulting BEnd margin is close to "unconstrained").

The main problem here, I think, is that CalculateBlockSideMargins should be computing side margins for the containing block's writing mode.

We also need to fix the mixture of writing modes in PlaceBlock, as suggested in comment 1 above.

And here's a simple reftest based on the testcase above.

Comment on attachment 8520059 [details] [diff] [review] part 1 - CalculateBlockSideMargins should be working in the containing block's writing mode. Review of attachment 8520059 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/generic/nsHTMLReflowState.cpp @@ +2346,5 @@ > + ComputedLogicalMargin().ConvertTo(cbWM, mWritingMode); > + LogicalMargin borderPadding = > + ComputedLogicalBorderPadding().ConvertTo(cbWM, mWritingMode); > + nscoord sum = margin.IStartEnd(cbWM) + > + borderPadding.IStartEnd(cbWM) + aComputedISize; Is this right? It looks as if the caller is passing in aComputedISize and aAvailISize from ComputedISize() and AvailISize(), which will be in its own writing mode.

I think you're right, that doesn't make sense. Actually, there's no need for the caller to pass these values at all; as CalculateBlockSideMargins is a method of the same nsHTMLReflowState that's calling it, it can decide internally (based on the writing mode of the containing block) what to use.

Comment on attachment 8528301 [details] [diff] [review] part 1 - CalculateBlockSideMargins should be working in the containing block's writing mode. Review of attachment 8528301 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/generic/nsHTMLReflowState.cpp @@ +2356,5 @@ > + availISizeCBWM = AvailableBSize(); > + } else { > + computedISizeCBWM = ComputedISize(); > + availISizeCBWM = AvailableISize(); > + } I'm in two minds whether it would be preferable to just do nscoord computedISizeCBWM = ComputedSize(cbWM).ISize(cbWM); nscoord availISizeCBWM = AvailableSize(cbWM).ISize(cbWM); On the one hand it makes the code here cleaner. On the other hand it's a bit less efficient (but not in the commonest case IIANM). @@ +2369,5 @@ > + ComputedLogicalMargin().ConvertTo(cbWM, mWritingMode); > + LogicalMargin borderPadding = > + ComputedLogicalBorderPadding().ConvertTo(cbWM, mWritingMode); > + nscoord sum = margin.IStartEnd(cbWM) + > + borderPadding.IStartEnd(cbWM) + computedISizeCBWM; Here too a possible alternative with the same caveats is nscoord sum = ComputedSizeWithMarginBorderPadding(cbWM).ISize(cbWM); r=me either way.

https://hg.mozilla.org/integration/mozilla-inbound/rev/6379adced839 https://hg.mozilla.org/integration/mozilla-inbound/rev/0688bf59271b https://hg.mozilla.org/integration/mozilla-inbound/rev/ae7b4d6cf9b4

https://hg.mozilla.org/mozilla-central/rev/6379adced839 https://hg.mozilla.org/mozilla-central/rev/0688bf59271b https://hg.mozilla.org/mozilla-central/rev/ae7b4d6cf9b4