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)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jruderman, Assigned: rbs)
Details
(Keywords: assertion, testcase)
Attachments
(4 files, 1 obsolete file)
(deleted),
text/xml
|
Details | |
(deleted),
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
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
Reporter | ||
Comment 1•19 years ago
|
||
Reporter | ||
Comment 2•19 years ago
|
||
Every time try to test MathML, I'm flooded with this assertion :( I guess I could comment it out locally.
Comment 3•19 years ago
|
||
rbs,
can you help on this. it is getting in the way of some test automation that we are working on.
thanks
-chofmann
Comment 4•19 years ago
|
||
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.
Reporter | ||
Comment 5•19 years ago
|
||
The assertion is staying on the trunk, though, right?
Comment 6•19 years ago
|
||
(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.
Reporter | ||
Comment 8•19 years ago
|
||
rbs, have you had a chance to look at this? Sounds like the assertion is staying on the trunk.
Assignee | ||
Comment 10•19 years ago
|
||
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.
Reporter | ||
Comment 11•19 years ago
|
||
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
Assignee | ||
Comment 12•19 years ago
|
||
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?
Assignee | ||
Comment 14•19 years ago
|
||
Checked in.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 15•19 years ago
|
||
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?
Assignee | ||
Comment 16•19 years ago
|
||
My tree had other cruft that masked that from me. No need to file another bug. Let's use this one.
Reporter | ||
Updated•19 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 17•19 years ago
|
||
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.
Assignee | ||
Comment 19•19 years ago
|
||
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 ∫ (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+
Assignee | ||
Comment 21•19 years ago
|
||
Checked in with an XXX comment.
Status: REOPENED → RESOLVED
Closed: 19 years ago → 19 years ago
Resolution: --- → FIXED
Attachment #220231 -
Attachment is obsolete: true
Attachment #220231 -
Flags: superreview?(roc)
Attachment #220231 -
Flags: review?(roc)
Reporter | ||
Updated•18 years ago
|
Attachment #220244 -
Flags: approval1.8.1.1?
Comment 22•18 years ago
|
||
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+
Assignee | ||
Comment 23•18 years ago
|
||
jesse, the assertion is not firing on the branches. So no need to bother there. Or are you seeing something else in your testing?
Reporter | ||
Comment 24•18 years ago
|
||
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.
Assignee | ||
Comment 25•18 years ago
|
||
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+
You need to log in
before you can comment on or make changes to this bug.
Description
•