Empty mfenced elements should display fences

RESOLVED FIXED in mozilla8

Status

()

Core
MathML
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: fscholz, Assigned: fscholz)

Tracking

(Blocks: 1 bug)

Trunk
mozilla8
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(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+
Blocks: 553918
Keywords: checkin-needed
Filed bug 670334 to follow-up the height issue.
http://hg.mozilla.org/integration/mozilla-inbound/rev/c262b7cdfd2f
Keywords: checkin-needed
Whiteboard: [inbound]
Merged:
http://hg.mozilla.org/mozilla-central/rev/c262b7cdfd2f
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Whiteboard: [inbound]
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.