There are a number of bugs that I have observed in <mpadded> 1. Attributes doesn't work at all. This is a regression. I saw it a long while ago, but was mystified as to its cause. It is only recently that I had a look in bonsai to see where it came from. If you look closely at this diff: http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsMathMLmpaddedFrame.cpp&root=/cvsroot&subdir=mozilla/layout/mathml/base/src&command=DIFF_FRAMESET&rev1=1.9&rev2=1.10 It appears that the following line is removed: aString.Right(value, stringLength - i); This was an inadvertent removal when trying to re-indent the line tha followed. Other bugs that have always been there, and which I hope to fix in this round: 2. There is an error in the code that parses attributes -- whitespace before the unit (e.g., "1 em") leads to troubles. 3. When attribute-values cause children to stick outiside (e.g., width="0"), the frame and thus its children aren't painted. Proposed fixes: =============== For 1. the fix is simply to re-instate the deleted line. For 3. the fix is to make sure to set the NS_FRAME_OUTSIDE_CHILDREN bit when children stick outside. For 2. well, the fix is to get the parsing right...

The patch is essentially a rewrite of the parsing code. In its premature optimization, the previous code was overzealous and had some troubles. This one takes a more secure and comprehensive route. Basically, the problem is to parse an atribute with a formal value as follows: [+|-] unsigned-number (% [pseudo-unit] | pseudo-unit | css-unit | namedspace) where * "+/-", if there, means the value is an incremental value * "unsigned-number" can be a fixed-point number (e.g., 1.25) * "pseudo-unit" is something related to the current frame, e.g., one can do width="1 height" to force the frame to end-up as a square frame or witdh="200=% height" to force the frame to be twice as wide as its height (it is because of this that children can actually stick outside) * "css-unit" is one of the usual 'ex', 'em', etc... (there is an existing function to handle them) * "namedspace" is a string such as 'thinspace', 'thickspace', etc., for which the actual numeric value depends on the current style (there is an existing function to handle them) * ...and there can be space between the tokens * ...and errors must be handled gracefully (i.e., proceed as if the attribute wasn't there) The patch does: 1. simply compress the input (hence there is at most one space between tokens) 2. tokenize while taking care of edge cases and other technicalities. I have tested the patch thoroughly and it now appears to address the reported problems fully.

Comment on attachment 86168 [details] [diff] [review] patch to fix the bug roc/waterson, r/sr? (there are particular edge cases such as height="200%" to magnify the current height by 200%).

Re-reading comment #4, I had the impression that one could read it that the edge case being mentioned was a remaining edge case that wasn't handled. In fact, I was merely drawing attention to an example of edge case. The code is handling all the edge cases to my knowledge.

Rather than using "if (oldValue != newValue)" to detect if an attribute is there, use a more encompassing test the "if (sign in [+- ])" because the sole presence of an attribute should disable some other generous inter-frame spacing that the MathML egine does by default, even if an oldValue coincides with a newValue. Also added more explantatory comments.

-> targeting m1.1b

Comment on attachment 86521 [details] [diff] [review] patch - take3 sr=waterson

I think you need to se the FRAME_OUTSIDE_CHILDREN bit if (dx < 0 || dy < 0 || mOverflowArea.XMost() > aDesiredSize.XMost() || mOverflowArea.YMost() > aDesiredSize.YMost()) ... know what I mean?

yep, I see what you mean. It had crossed my mind but I didn't gave it the due time & attention that it deserved. Attached is a patch which needed a bit a quietness to get the details right with the less amount of the code.

Comment on attachment 87639 [details] [diff] [review] patch - take 4 r=roc+moz

Fix checked in.