Closed Bug 337700 Opened 18 years ago Closed 18 years ago

js changes to MathML are not drawn


(Core :: MathML, defect)

1.8 Branch
Not set





(Reporter: planzilla, Assigned: rbs)




(Keywords: fixed1.8.0.7, fixed1.8.1, regression, Whiteboard: [not present on the trunk])


(2 files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; de; rv: Gecko/20060426 Firefox/
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; de; rv: Gecko/20060426 Firefox/

When manipulating a MathML formula in an XHTML page, the modified parts are not redrawn. Forcing a redraw by covering and uncovering the window leads to blank areas where changes should be. The remaining part of the formula redraws as expected, though. However, changing the font size helps.

On the example URL (sorry, german) you'll find an integral and four buttons to change the boundaries. Clicking triggers some javascript that replaces the relevant parts of the formula through the DOM. These changes do appear in the DOM Inspector, yet  are not shown, as described above.

Reproducible: Always

Steps to Reproduce:
1. go to URL
2. click on any of the buttons
3. cover and uncover the browser window

Actual Results:  
white gaps in the formula

Expected Results:  
fractions resp. numbers

It did work on Firefox 1.0.x.

According to the warning dialog, I have only the "Symbol" missing.
WFM on trunk, Mac OS X.

Btw, awesome use of dynamic MathML and SVG.
worksforme with trunk and SeaMonkey 1.0 (Gecko  But SeaMonkey 1.0.1 (Gecko exhibits the bug.
Ever confirmed: true
Flags: blocking1.8.0.5?
Keywords: regression
Version: Trunk → 1.8 Branch
Works for me with Windows 1.8.0 branch builds from 2006-03-21-08, 2006-04-25-05, and 2006-05-12-05.
I can reproduce the problem with Firefox on Mac OS.
Resizing the window forces the ML to be redrawn correctly.
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv: Gecko/20060426 Firefox/

Works on Camino 1.0 (Version 2006021400), broken in Camino 1.0.1 (Version 2006042704)
This is a regression from bug 309120
Guys, are you sure it is a regression from bug bug 309120? 

From bug 309120 comment 34, that patch was checked in the branch on 2006-02-26, and the WFM period reported here in comment 3 is past that date:

> Works for me with Windows 1.8.0 branch builds from 2006-03-21-08,
> 2006-04-25-05, and 2006-05-12-05.

Maybe I did something wrong when testing those builds, or maybe this isn't a problem on Windows. I'll test again to be sure.
Indeed, I was misinterpreting the instructions. I do see this bug in all of the builds I mentioned above, and in 2006-02-27-06, but not in 2006-02-26-06, which points to the changes from bug 309120 as the cause, as mentioned earlier.
rbs: is there any chance of a fix for this regression in a 1.8.0.x release?
Flags: blocking1.9a1+
Flags: blocking1.8.1?
Flags: blocking1.8.0.5?
Flags: blocking1.8.0.5+
I hope so (or 1.8.1.x). I have been unfortunately unable to investigate so far due to other balls in the air, and I am travelling tomorrow. Hopefully, I will have a chance to dig a bit earlier next month.
Not going to make, please nominate for a later 1.8.0.x release if you get it into 1.8.1
Flags: blocking1.8.0.5+ → blocking1.8.0.5-
Flags: blocking1.8.1? → blocking1.8.1+
not going to block ff2 for this, but we'll happily consider a safe patch.
Flags: blocking1.8.1+ → blocking1.8.1-
Flags: blocking1.9-
Whiteboard: [wanted-1.9]
Blocks: 344281
Attached patch patchSplinter Review
Curiously, this patch (on the 1.8 branch) makes the bug go away.
reminder: this bug is not present on the trunk.
Whiteboard: [wanted-1.9] → [not present on the trunk]][wanted-1.9]
The current patch in attachment 232369 [details] [diff] [review] does the job. I am investigating what is going on. (If I don't get to understand fully what is going on it by today, I will request r/sr/a on this fallback patch.)
Flags: blocking1.8.1?
Flags: blocking1.8.1-
Flags: blocking1.8.0.7?
When you get a reviewed patch you're happy with nominate for 1.8.0.x and we'll take a look, but we're not going to block on this.
Flags: blocking1.8.0.7? → blocking1.8.0.7-
Attached patch patchSplinter Review
OK, I now understand the problem, and this patch fixes it. I am proposing it for both the trunk and the 1.8.0 and 1.8 branches.

The bug is happening because the bit NS_FRAME_HAS_DIRTY_CHILDREN is set too early [causing ReflowDirtyChild() not to be called]. The sequence is like this:

1- Content is changed, this causes new frames to be created, and we eventually get to nsMathMLContainerFrame::ChildListChanged(), in the patch.

2- There, we walk up to an appropriate parent (e.g., to handle the &integral, an ancestor higher up tells that &integral how tall it ought to be to properly cover the formula). And it is here that the bit is set too early. As we climb up (c.f. the patch), the bit must be set on the current level, not the parent, a level up. Otherwise when we get in

3 - ReLayoutChildren(frame), which eventually calls ReflowDirtyChild(), this turns out to be a no-op because it (purposely) optimizes things out when a frame (repeatedly) calls it with its NS_FRAME_HAS_DIRTY_CHILDREN bit set (so that changes in different children coalesce into posting a single reflow event):

  // The inline container frame does not handle the reflow
  // request.  It passes it up to its parent container.

  // If you don't already have dirty children,
  if (!(mState & NS_FRAME_HAS_DIRTY_CHILDREN)) {
    if (mParent) {
      // Record that you are dirty and have dirty children
      mState |= NS_FRAME_IS_DIRTY;

      // Pass the reflow request up to the parent
      mParent->ReflowDirtyChild(aPresShell, (nsIFrame*) this);
    else {
      NS_ASSERTION(0, "No parent to pass the reflow request up to.");

  return NS_OK;
Attachment #233937 - Flags: superreview?(bzbarsky)
Attachment #233937 - Flags: review?(bzbarsky)
Comment on attachment 233937 [details] [diff] [review]

r+sr=bzbarsky, but you might want to talk to dbaron about how this should work on reflow branch....
Attachment #233937 - Flags: superreview?(bzbarsky)
Attachment #233937 - Flags: superreview+
Attachment #233937 - Flags: review?(bzbarsky)
Attachment #233937 - Flags: review+
Checked in.

Okay if the reflow-branch will involve changing this. I don't mind revisiting it.
Closed: 18 years ago
Resolution: --- → FIXED
Attachment #233937 - Flags: approval1.8.1?
Attachment #233937 - Flags: approval1.8.0.7?
Whiteboard: [not present on the trunk]][wanted-1.9] → [not present on the trunk] [wanted-1.9]
Comment on attachment 233937 [details] [diff] [review]

a=beltzner on behalf of drivers for the mozilla_1_8_branch
Attachment #233937 - Flags: approval1.8.1? → approval1.8.1+
fixed1.8.1: checked in the 1.8 branch.
Flags: blocking1.8.1?
Keywords: fixed1.8.1
Comment on attachment 233937 [details] [diff] [review]

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #233937 - Flags: approval1.8.0.7? → approval1.8.0.7+
I am behind an extremely slow connection at present and I am not getting cvs from my laptop:

D:\Mozilla\m1.8.0\mozilla\layout\mathml\base\src>cvs commit -m"js changes to MathML are not drawn, b=337700, r+sr=bzbarsky, a=dveditz" nsMathMLContainerFrame.cpp

ssh: connect to host port 22: Connection timed out
cvs [commit aborted]: end of file from server (consult above messages if any)

Please, do well to checkin for me. Thanks.
I checked this patch in on the 1.8.0 branch.
Keywords: fixed1.8.0.7
Flags: wanted1.9+
Whiteboard: [not present on the trunk] [wanted-1.9] → [not present on the trunk]
You need to log in before you can comment on or make changes to this bug.