Closed Bug 323733 Opened 19 years ago Closed 19 years ago

<math:ms> causes ASSERTION: Frame abuses NS_FRAME_OUTSIDE_CHILDREN flag

Categories

(Core :: MathML, defect)

PowerPC
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jruderman, Assigned: rbs)

Details

(Keywords: assertion, testcase)

Attachments

(4 files, 1 obsolete file)

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060115 Firefox/1.6a1 ###!!! ASSERTION: Frame abuses NS_FRAME_OUTSIDE_CHILDREN flag: 'Not Reached', file /Users/admin/trunk/mozilla/layout/generic/nsFrame.cpp, line 4420
Attached file testcase (deleted) —
Every time try to test MathML, I'm flooded with this assertion :( I guess I could comment it out locally.
rbs, can you help on this. it is getting in the way of some test automation that we are working on. thanks -chofmann
Note that this assertion was added in bug 315210, and that bug has a patch removing the assertion, waiting for approval for 1.8.0 branch. It's already been backed out on the 1.8 branch.
The assertion is staying on the trunk, though, right?
(In reply to comment #5) > The assertion is staying on the trunk, though, right? From the comments in that bug, it seems like it is meant to stay on the trunk, but my comment meant to convey that if it is being removed on the branches it may be a candidate for removal on the trunk, if the bugs that trigger it can't be fixed in a reasonable timeframe.
I commented out this thing a long time ago in my tree :-) (Per the other bug, have Firefox/Thunderbox abandonned the trunk? If they are on the trunk, how come they are not suffering from this nasty assertion anymore?) I am presently blocked by the switch to VC8 and have not been able to get my build again. I have filed bug 329209 to that effect. Any helpful clues there will be appreciated. Once I get my build again, I will be able to look at this bug.
rbs, have you had a chance to look at this? Sounds like the assertion is staying on the trunk.
With the change of the painting code to a display list, the artefact that MathML was using to get around the overflow has become irrelevant. The new painting code only relies on nsIFrame::GetOverflowRect(). Hence, the current trunk (with or without the previous patch) exhibits the clippings that you see in this screenshot anyway.
Is there a bug on the overflow issue? I don't see one in the dependency tree for bug 317375 (where frame display lists were added).
Attachment #219724 - Attachment description: patch v1 - don't set bit that causes the assert on the mathml side → patch v1 - don't set the bit that causes the assert on the mathml side
There is bug 161155 as well. The display list isn't the root cause of the problem. I have filed bug 335405 for other thoughts I had with the NS_FRAME_OUTSIDE_CHILDREN bit.
Attachment #219724 - Flags: superreview?(roc)
Attachment #219724 - Flags: review?(roc)
Attachment #219724 - Flags: superreview?(roc)
Attachment #219724 - Flags: superreview+
Attachment #219724 - Flags: review?(roc)
Attachment #219724 - Flags: review+
rbs, can you describe in bug 161155 or some new bug what the underlying problem is, based on the comments you're removing here? Is it a bug in the font system, in specific fonts, or our code?
Checked in.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Now this testcase gives me ###!!! ASSERTION: Computed overflow area must contain frame bounds: 'aNewSize.width == 0 || aNewSize.height == 0 || aOverflowArea->Contains(nsRect(nsPoint(0, 0), aNewSize))', file /Users/admin/trunk/mozilla/layout/generic/nsFrame.cpp, line 4586 :( Should I file a new bug?
My tree had other cruft that masked that from me. No need to file another bug. Let's use this one.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch extra patch (obsolete) (deleted) — Splinter Review
roc, care to r+sr? I am still on the hook here. I should have left this line out as it triggers another assertion, although I am not done yet. But no luck nailing down the clipping problem so far.
Attachment #220231 - Flags: superreview?(roc)
Attachment #220231 - Flags: review?(roc)
Instead of dropping the call to FinishStoreOverflow, which is really needed, just set aDesiredSize.mOverflowArea to nsRect(0, 0, aDesiredSize.width, aDesiredSize.height) before the call.
Attached patch alternative extra (deleted) — Splinter Review
I considered this but hesitated because it gives the impression that it is done already, whereas the size is still tentative at that point. What happens is that, at the beginning, the &Integral; (to pick an example), has a normal size (small) glyph. Then, much later (could be way much later), when its surrounding context is known, an ancestor somewhere determines that the integral has to mutate and stretch to cover things properly. And it is its new stretched size that is the real thing. It is that one that has to go in FinishAndStoreOverflow. So I am okay with either alternative because I am not done yet either way. I also wonder if the clipping bug surfaces somewhere because of this convoluted wiring, which is what I am trying to nail down.
Attachment #220244 - Flags: superreview?(roc)
Attachment #220244 - Flags: review?(roc)
Comment on attachment 220244 [details] [diff] [review] alternative extra Add an XXX comment to that effect. I prefer this patch because it's a little closer to being correct.
Attachment #220244 - Flags: superreview?(roc)
Attachment #220244 - Flags: superreview+
Attachment #220244 - Flags: review?(roc)
Attachment #220244 - Flags: review+
Checked in with an XXX comment.
Status: REOPENED → RESOLVED
Closed: 19 years ago19 years ago
Resolution: --- → FIXED
Attachment #220231 - Attachment is obsolete: true
Attachment #220231 - Flags: superreview?(roc)
Attachment #220231 - Flags: review?(roc)
Attachment #220244 - Flags: approval1.8.1.1?
Comment on attachment 220244 [details] [diff] [review] alternative extra approved for 1.8 branch, a=dveditz for drivers
Attachment #220244 - Flags: approval1.8.1.1? → approval1.8.1.1+
jesse, the assertion is not firing on the branches. So no need to bother there. Or are you seeing something else in your testing?
The reason the assertion doesn't fire on branch is that the assertion was removed from the branch (last patch in bug 315210). I don't know whether this bug causes any problems other than an assertion firing, but I nominated it because it looked like a simple patch.
Comment on attachment 220244 [details] [diff] [review] alternative extra The patch was a continuatio on of the other bigger patch. So, pulling back the approval to get it off the radar.
Attachment #220244 - Flags: approval1.8.1.1+
Crashtest checked in.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: