Closed Bug 701724 Opened 10 years ago Closed 8 years ago

"ASSERTION: mEndOffset is beyond the end of this node with MathML, part 2

Categories

(Core :: MathML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: jruderman, Unassigned)

References

(Blocks 2 open bugs)

Details

(Keywords: assertion, testcase)

Attachments

(3 files)

Attached file testcase
Slightly different testcase from bug 700090, still triggers the assertion.
Attached file stack trace
Stack trace looks the same, fwiw.
Attached file Testcase #2
Great, so this assertion caught an existing bug!

The problem is nsMathMLTokenFrame::Init calls CompressWhitespace which
changes the text node data without notifying, thus making the Range
invalid.

http://hg.mozilla.org/mozilla-central/annotate/b51eb4007926/layout/mathml/nsMathMLTokenFrame.cpp#l118

FWIW, this code is the same all the way back to CVS rev. 1.1
So, if I change the 'false' to 'true' to make it notify that fixes the
reported assertion.  But instead I get:
###!!! ASSERTION: recurring into frame construction: 'mPresContext->mLayoutPhaseCount[eLayoutPhase_FrameC] == 0', file layout/base/nsAutoLayoutPhase.h, line 86

which doesn't seem like a good trade.

Does anyone have suggestions how to fix the MathML code so it doesn't need to
modify the text node (like the XXX comment on line 139 suggests?)
http://hg.mozilla.org/mozilla-central/annotate/b51eb4007926/layout/mathml/nsMathMLTokenFrame.cpp#l118
Component: DOM: Traversal-Range → MathML
OS: Mac OS X → All
QA Contact: traversal-range → mathml
Hardware: PowerPC → All
A good first question is why we need to do this at all.  What happens if you take that call out?
The comment mentions bug 15402.  I'm guessing it's needed for the whitespace
handling for token elements the spec asks for:
http://www.w3.org/TR/MathML3/chapter2.html#fund.collapse

Anyway, I can't test it in my local debug build for some reason, for example
the test layout/mathml/tests/simple.xml looks wrong both with and without the
CompressWhitespace call.  It looks fine in a Nightly build on the same machine
though.
I doubt the authors of the spec intended for layout to modify the DOM. And I don't think they intended to modify the DOM at all, given language like

"In the second example, a single space character is to be rendered before 'Theorem'"

Instead, I think the extra whitespace is just supposed to be ignored for purposes of rendering.
Shouldn't modifying the DOM from layout cause an assertion failure on its own?
I guess that's bug 335054.
Blocks: 335054
I don't see the assertion. This has probably been fixed by 
https://hg.mozilla.org/releases/mozilla-beta/rev/5a0a98d11dbe

BTW, with the recent code refactoring (especially bug 114365 and bug 731667), I expect that we removed all the places where MathML code modified the content tree during layout.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.