There are a few places in Reflow of MathML frames where child frames are moved after calling FinishReflowChild().

<msqrt> is the most common case where the frames are moved a large amount, and the fix for <mpadded> is similar.

The next most common case is http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/mathml/base/src/nsMathMLContainerFrame.cpp&rev=1.173&mark=1442#1422 The distance moved could be significant for some <mo> operators that are direct children of a <math> element (with no enclosing <mrow>). We should be able to shift the work that FixInterFrameSpacing does to the Place() methods. They could use another method to calculate the offset for the child frames, probably only necessary when aPlaceOrigin is true. There are about 13 different implementations of Place(), though. I hope the same issue with spacing around embellished operators in nsMathMLContainerFrame::Stetch() can be solved in the same way. http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/mathml/base/src/nsMathMLContainerFrame.cpp&rev=1.173&mark=402,436-437#402

Comment on attachment 294807 [details] [diff] [review] FinishReflowChild() with the final position for <msqrt> and <mpadded> I've noticed a spacing issue that I need to look into.

Corrected != that should have been == nsGkAtoms::msqrt.

+ if (mThinSpace < 0) { + // cache away thinspace + mThinSpace = GetThinSpace(mParentFrame->GetStyleFont()); + } Why not just call GetThinSpace every time instead of caching in mThinSpace at all. The performance issues here are negiligible. + nscoord mX; Put this with the other fields. + if (child.Frame()) { + ascent = child.Ascent(); + descent = child.Descent(); + mBoundingMetrics = child.BoundingMetrics(); + mBoundingMetrics.leftBearing += child.mX; + mBoundingMetrics.rightBearing += child.mX; + ++child; + } I'd prefer to avoid this by judiciously initializing things to nscoord_MAX etc. Looks great otherwise.

(In reply to comment #6) > Why not just call GetThinSpace every time instead of caching in mThinSpace at > all. The performance issues here are negiligible. Dunno. It used to be cached but I've happily removed that. > + nscoord mX; > > Put this with the other fields. Done (and it didn't really need to be public). > > + if (child.Frame()) { > + ascent = child.Ascent(); > + descent = child.Descent(); > + mBoundingMetrics = child.BoundingMetrics(); > + mBoundingMetrics.leftBearing += child.mX; > + mBoundingMetrics.rightBearing += child.mX; > + ++child; > + } > > I'd prefer to avoid this by judiciously initializing things to nscoord_MAX etc. I've avoided this by modifying nsBoundingMetrics::operator+= . I think the frame rect should include the baseline, so haven't done anything special with the ascent and descent for the reflow metrics.

+ leftBearing = width - bm.leftBearing; That should be "width + bm.leftBearing".

Comment on attachment 295181 [details] [diff] [review] FinishReflowChild() with the final position for <msqrt> and <mpadded> and make nsBoundingMetrics::operator+= handle empty bounds [checked-in] 1.81 gfx/public/nsIRenderingContext.h 1.301 layout/generic/nsContainerFrame.cpp 3.81 layout/generic/nsContainerFrame.h 1.175 layout/mathml/base/src/nsMathMLContainerFrame.cpp 1.78 layout/mathml/base/src/nsMathMLContainerFrame.h 1.65 layout/mathml/base/src/nsMathMLTokenFrame.cpp 1.74 layout/mathml/base/src/nsMathMLmfencedFrame.cpp 1.47 layout/mathml/base/src/nsMathMLmpaddedFrame.cpp 1.17 layout/mathml/base/src/nsMathMLmpaddedFrame.h 1.50 layout/mathml/base/src/nsMathMLmsqrtFrame.cpp (Comment 2 is not yet addressed.)

Bug tidying. Filed bug 433064 on comment 2.