Closed Bug 353876 Opened 18 years ago Closed 18 years ago

assertions about chars without style contexts

Categories

(Core :: MathML, defect)

x86
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: rbs, Assigned: rbs)

References

Details

Attachments

(2 files)

329 bytes, application/xhtml+xml
Details
10.61 KB, patch
roc
: review+
Details | Diff | Splinter Review
Start with an empty <mi/>:

<math xmlns="http://www.w3.org/1998/Math/MathML">
 <mi id="mi"/>
</math>

Append a child text to it:

  var mi = document.getElementById("mi");
  var t = document.createTextNode("x");
  mi.appendChild(t);

The resulting <mi>x</mi> should be styled the same as if the text was static.
But it isn't.
Attached file testcase
I can't reproduce with a 1.8 branch build. It now seems to me that the bug was due this change (from attachment 238624 [details] [diff] [review]). I had this change in my tree to remove the assertions about chars without style contexts, which are triggered by StirDOM when it twists the DOM in strange/invalid ways in bug 322625.

Index: layout/mathml/base/src/nsMathMLTokenFrame.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/mathml/base/src/nsMathMLTokenFrame.cpp,v
retrieving revision 1.49
diff -u -p -r1.49 nsMathMLTokenFrame.cpp
--- layout/mathml/base/src/nsMathMLTokenFrame.cpp	15 Aug 2006 04:49:43 -0000	1.49
+++ layout/mathml/base/src/nsMathMLTokenFrame.cpp	15 Sep 2006 13:28:25 -0000
@@ -314,6 +314,9 @@ nsMathMLTokenFrame::SetTextStyle(nsPresC
   // set the -moz-math-font-style attribute without notifying that we want a reflow
   mContent->SetAttr(kNameSpaceID_None, nsMathMLAtoms::MOZfontstyle, fontstyle, PR_FALSE);
 
+  //XXX commented out because it doesn't have to be done straightaway.
+  //It is going to be picked up when building the automatica style data for scriptsizes
+#if 0
   // then, re-resolve the style contexts in our subtree
   nsFrameManager *fm = aPresContext->FrameManager();
   nsStyleChangeList changeList;
@@ -323,6 +326,7 @@ nsMathMLTokenFrame::SetTextStyle(nsPresC
   nsIFrame* parentFrame = GetParent();
   fm->DebugVerifyStyleTree(parentFrame ? parentFrame : this);
 #endif
+#endif

=================
For the rationale, I am quoting here what I said in bug 322625 comment 6:

<quote>
As for the assertion in nsMathMLChar.cpp:
   NS_ASSERTION(mStyleContext, "chars shoud always have style context");

it turned out to be triggered by this debugging call in nsMathMLTokenFrame:
   fm->DebugVerifyStyleTree(parentFrame ? parentFrame : this);

What happens is that StirDOM puts <mi> as a child of <mo>
  <mo>
     <mi>

and when DebugVerifyStyleTree() is called in the parent, i.e. <mo>, this <mo>
has not yet received its SetInitialChildList() to setup its own nsMathMLChar.
Fortunately, I could just comment the whole thing out because there is another
re-resolve sweep through the <math> subtree anyway -- to setup all the
potential style data needed for scripting elements. So the style change needed
by <mi> will happen as a side-effect.
</quote>

Well, apparently then, commenting this out caused this bug... I will attach a patch to avoid these assertions (without causing this bug).
Summary: dynamic text appended to <mi> is not italicized → assertions about chars without style contexts
Attached patch patchSplinter Review
The patch looks a little bit messier than it really is. What it does is the commenting out that I mentioned above, but with the proviso that it is _conditionally_ using the added parameter aComputeStyleChange. This change of declaration unwinds a cascade.

I also moved ChildListChanged() from <mo> to the base token class shared by <mo> and <mi>.
Attachment #239726 - Flags: superreview?(roc)
Attachment #239726 - Flags: review?(roc)
*** Bug 336065 has been marked as a duplicate of this bug. ***
Attachment #239726 - Flags: superreview?(roc)
Attachment #239726 - Flags: superreview+
Attachment #239726 - Flags: review?(roc)
Attachment #239726 - Flags: review+
Checked in.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: