Closed Bug 701724 Opened 10 years ago Closed 8 years ago
End Offset is beyond the end of this node with Math ML, part 2
Slightly different testcase from bug 700090, still triggers the assertion.
Stack trace looks the same, fwiw.
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 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.