(Follow-up of bug 553918 comment1) Testsuite test: http://www.w3.org/Math/testsuite/build/main/Presentation/GeneralLayout/mfenced/mfenced1-full.xhtml I already provided a reftest which currently fails (mfenced-9): https://bug553918.bugzilla.mozilla.org/attachment.cgi?id=543487 Perhaps we can just remove the if statement here? http://mxr.mozilla.org/mozilla-central/source/layout/mathml/nsMathMLmfencedFrame.cpp#354
> Perhaps we can just remove the if statement here? > http://mxr.mozilla.org/mozilla-central/source/layout/mathml/ > nsMathMLmfencedFrame.cpp#354 Yes, I think I will work. We just have to verified that we won't use null Frame pointer or similar.
Created attachment 543603 [details] [diff] [review] Fix: Removes the if condition
Assignee: nobody → elchi3
Status: NEW → ASSIGNED
Attachment #543603 - Flags: review?(karlt)
(In reply to comment #2) > Created attachment 543603 [details] [diff] [review] [review] > Fix: Removes the if condition Did you test it? I think I read it too quickly this morning, and thought this if statement was to leave the function when the mfenced is empty.
Comment on attachment 543603 [details] [diff] [review] Fix: Removes the if condition Yes, I agree that the empty mfenced should not be handled specially here. The centering logic happens to provide additional size so that the fences become visible. This is still not quite exactly how they should be, but is definitely better. The height of the fences should be considered in containerSize and I don't see that happening. This might be a regression from bug 414277, because, before then, characters were always stretched to at least their base size. This patch is at least part of the solution and good enough for this bug.
Attachment #543603 - Flags: review?(karlt) → review+
Filed bug 670334 to follow-up the height issue.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
Version: unspecified → Trunk
Is this tested with tests in bug 553918?
(In reply to comment #8) > Is this tested with tests in bug 553918? Yes, it is.
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.