Last Comment Bug 337700 - js changes to MathML are not drawn
: js changes to MathML are not drawn
Status: RESOLVED FIXED
[not present on the trunk]
: fixed1.8.0.7, fixed1.8.1, regression
Product: Core
Classification: Components
Component: MathML (show other bugs)
: 1.8 Branch
: x86 Linux
: -- normal with 1 vote (vote)
: ---
Assigned To: rbs
: Hixie (not reading bugmail)
Mentors:
http://page.mi.fu-berlin.de/~planz/si...
Depends on:
Blocks: 344281
  Show dependency treegraph
 
Reported: 2006-05-12 06:44 PDT by Ingo Planz
Modified: 2008-01-16 03:07 PST (History)
7 users (show)
dbaron: blocking1.9-
reed: wanted1.9+
dveditz: blocking1.9a1+
dveditz: blocking1.8.0.5-
dveditz: blocking1.8.0.7-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (876 bytes, patch)
2006-08-05 15:44 PDT, rbs
no flags Details | Diff | Review
patch (1.52 KB, patch)
2006-08-15 21:08 PDT, rbs
bzbarsky: review+
bzbarsky: superreview+
dveditz: approval1.8.0.7+
mbeltzner: approval1.8.1+
Details | Diff | Review

Description Ingo Planz 2006-05-12 06:44:31 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; de; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; de; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3

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.
Comment 1 Jesse Ruderman 2006-05-12 13:43:18 PDT
WFM on trunk, Mac OS X.

Btw, awesome use of dynamic MathML and SVG.
Comment 2 Andrew Schultz 2006-05-12 19:33:17 PDT
worksforme with trunk and SeaMonkey 1.0 (Gecko 1.8.0.1).  But SeaMonkey 1.0.1 (Gecko 1.8.0.2) exhibits the bug.
Comment 3 :Gavin Sharp [email: gavin@gavinsharp.com] 2006-05-12 19:44:20 PDT
Works for me with Windows 1.8.0 branch builds from 2006-03-21-08, 2006-04-25-05, and 2006-05-12-05.
Comment 4 Thilo Planz 2006-05-12 21:10:58 PDT
I can reproduce the problem with Firefox 1.5.0.3 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:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3

Works on Camino 1.0 (Version 2006021400), broken in Camino 1.0.1 (Version 2006042704)
Comment 5 Andrew Schultz 2006-05-13 07:26:02 PDT
This is a regression from bug 309120
Comment 6 rbs 2006-05-14 23:13:01 PDT
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.


Comment 7 :Gavin Sharp [email: gavin@gavinsharp.com] 2006-05-14 23:17:23 PDT
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.
Comment 8 :Gavin Sharp [email: gavin@gavinsharp.com] 2006-05-14 23:36:55 PDT
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.
Comment 9 Daniel Veditz [:dveditz] 2006-06-07 15:47:31 PDT
rbs: is there any chance of a fix for this regression in a 1.8.0.x release?
Comment 10 rbs 2006-06-10 02:21:59 PDT
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.
Comment 11 Daniel Veditz [:dveditz] 2006-06-19 17:39:30 PDT
Not going to make 1.8.0.5, please nominate for a later 1.8.0.x release if you get it into 1.8.1
Comment 12 Darin Fisher 2006-06-28 19:59:06 PDT
not going to block ff2 for this, but we'll happily consider a safe patch.
Comment 13 rbs 2006-08-05 15:44:14 PDT
Created attachment 232369 [details] [diff] [review]
patch

Curiously, this patch (on the 1.8 branch) makes the bug go away.
Comment 14 rbs 2006-08-05 16:20:17 PDT
reminder: this bug is not present on the trunk.
Comment 15 rbs 2006-08-15 13:39:49 PDT
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.)
Comment 16 Daniel Veditz [:dveditz] 2006-08-15 14:44:00 PDT
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.
Comment 17 rbs 2006-08-15 21:08:32 PDT
Created attachment 233937 [details] [diff] [review]
patch

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):

nsMathMLContainerFrame::ReflowDirtyChild()
{
  // 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;
      mState |= NS_FRAME_HAS_DIRTY_CHILDREN;

      // 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;
}
Comment 18 Boris Zbarsky [:bz] 2006-08-15 21:27:30 PDT
Comment on attachment 233937 [details] [diff] [review]
patch

r+sr=bzbarsky, but you might want to talk to dbaron about how this should work on reflow branch....
Comment 19 rbs 2006-08-15 22:33:47 PDT
Checked in.

Okay if the reflow-branch will involve changing this. I don't mind revisiting it.
Comment 20 Mike Beltzner [:beltzner, not reading bugmail] 2006-08-16 10:28:26 PDT
Comment on attachment 233937 [details] [diff] [review]
patch

a=beltzner on behalf of drivers for the mozilla_1_8_branch
Comment 21 rbs 2006-08-16 13:53:26 PDT
fixed1.8.1: checked in the 1.8 branch.
Comment 22 Daniel Veditz [:dveditz] 2006-08-18 11:35:34 PDT
Comment on attachment 233937 [details] [diff] [review]
patch

approved for 1.8.0 branch, a=dveditz for drivers
Comment 23 rbs 2006-08-18 14:53:02 PDT
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 cvs.mozilla.org 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.
Comment 24 :Gavin Sharp [email: gavin@gavinsharp.com] 2006-08-18 15:02:11 PDT
I checked this patch in on the 1.8.0 branch.
mozilla/layout/mathml/base/src/nsMathMLContainerFrame.cpp 	1.113.14.3.2.4

Note You need to log in before you can comment on or make changes to this bug.