Last Comment Bug 321994 - Firefox doesn't display pages containing MathML
: Firefox doesn't display pages containing MathML
Status: VERIFIED FIXED
: verified1.8.0.9, verified1.8.1.1
Product: Core
Classification: Components
Component: MathML (show other bugs)
: Trunk
: All All
: -- major with 1 vote (vote)
: ---
Assigned To: rbs
: Hixie (not reading bugmail)
Mentors:
http://www.mozilla.org/projects/mathm...
Depends on: 349904
Blocks: 307157
  Show dependency treegraph
 
Reported: 2005-12-31 05:52 PST by Vincent Lefevre
Modified: 2008-01-16 03:20 PST (History)
12 users (show)
caillon: blocking1.9?
reed: wanted1.9+
mbeltzner: blocking1.8.1-
dveditz: blocking1.8.0.8-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Screenshot of Firefox displaying MathML incorrectly (101.94 KB, image/png)
2006-04-18 23:38 PDT, Jitse Niesen
no flags Details
undo the fix of bug 307157 (1.55 KB, patch)
2006-09-26 11:34 PDT, rbs
roc: review+
roc: superreview+
dveditz: approval1.8.0.8+
mconnor: approval1.8.1+
Details | Diff | Review
proposed patch (1.86 KB, patch)
2006-10-04 19:31 PDT, Behdad Esfahbod
no flags Details | Diff | Review
patch to be checked in (4.03 KB, patch)
2006-10-06 08:28 PDT, rbs
rbs: review+
rbs: superreview+
dveditz: approval1.8.0.8-
dveditz: approval1.8.0.9+
mconnor: approval1.8.1-
dveditz: approval1.8.1.1+
Details | Diff | Review

Description Vincent Lefevre 2005-12-31 05:52:12 PST
User-Agent:       Mozilla/5.0 (X11; U; Linux ppc; en-US; rv:1.8) Gecko/20051224 Debian/1.5.dfsg-3 Firefox/1.5
Build Identifier: Mozilla/5.0 (X11; U; Linux ppc; en-US; rv:1.8) Gecko/20051224 Debian/1.5.dfsg-3 Firefox/1.5

When I open a page containing MathML such as the above URL, Firefox doesn't display anything (as with the above URL) or very little. Firefox displays parts of the page only when scrolling (but even in this case, some characters are not displayed) or when doing text selection.

Reproducible: Always

Steps to Reproduce:
1. Open http://www.mozilla.org/projects/mathml/start.xhtml
2. Scroll.
Actual Results:  
After 1, the URL appears in the address bar, but the previous page is still displayed. After 2, the part that is redrawn has the contents of the real page, though some characters of MathML formulas are randomly missing (sometimes they appear, sometimes they don't, and this can particularly be seen when selecting and deselecting text).

Expected Results:  
The page should have been displayed.
Comment 1 Ria Klaassen (not reading all bugmail) 2005-12-31 06:13:23 PST
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051230 Firefox/1.6a1 ID:2005123023

FYI: in windows I see this: http://img421.imageshack.us/img421/2107/fonts2my.png
Comment 2 Vincent Lefevre 2005-12-31 06:58:04 PST
You get the correct result under Windows (just install the fonts). BTW, there's no problem either under Mac OS X. This may be a Linux-only bug, and possibly Linux/PPC-only (like bug 321317).
Comment 3 Peter Weilbacher 2006-01-02 04:43:47 PST
It works mostly with
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8) Gecko/20051228 SeaMonkey/1.0b
I get formulae displayed and no warning about fonts (but _all_ chars are displayed as greek symbols but that's bug 149566 and probably my fault). So I agree that this is appears to be PPC specific.
Comment 4 Christian von Schultz 2006-03-05 09:41:22 PST
The same problem has appeared on both Windows and Linux using Firefox 1.5.0.1, but not for the URL indicated in this report, but rather for an entry in a forum for physics students at Chalmers in Gothenburg, Sweden: http://org-pc23.fy.chalmers.se/forum/viewtopic.php?t=28

Screenshots using different versions of Mozilla and Firefox are available at http://www.dd.chalmers.se/~von/mathmlproblems/

Most MathML pages seem to work fine. The bug appears to have been introduced in Firefox 1.5.
Comment 5 Vincent Lefevre 2006-03-05 10:09:53 PST
(In reply to comment #4)
> The same problem has appeared on both Windows and Linux using Firefox 1.5.0.1,
> but not for the URL indicated in this report, but rather for an entry in a
> forum for physics students at Chalmers in Gothenburg, Sweden:
> http://org-pc23.fy.chalmers.se/forum/viewtopic.php?t=28

Ditto with Firefox 1.5.0.1 under Mac OS X. I don't know if this related to the problem I have under Linux/PPC, but I'm setting Hardware/OS to All/All since this more general (and reproducible) problem must be dealt with first. Then I'll see if something changes under Linux/PPC.
Comment 6 rbs 2006-03-05 21:20:32 PST
>The bug appears to have been introduced in Firefox 1.5.

Guys, what has happen between 1.5 and 1.5.0.1?!?

I am still trying to get my tree back after the forcefull switch to VC8 that has become the norm (see bug 329209 which still has unresolved issues for me).
Comment 7 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-03-05 21:31:25 PST
Er... you mean between 1.0 and 1.5, since the bug claims to have been introduced in 1.5?  That's between Gecko 1.7 and 1.8.  What _hasn't_ changed?

I'm not seeing a bug on Linux/x86 either, as far as I can tell.

Vincent, would it be possible to get a somewhat smaller regression range?  I dunno whether we even have Linux-ppc nightlies... :(
Comment 8 Vincent Lefevre 2006-03-06 01:46:13 PST
After the latest upgrades under Linux/ppc, Mozilla's MathML start page now displays, but with problems similar to those observed on http://org-pc23.fy.chalmers.se/forum/viewtopic.php?t=28 on the other platforms.

Testing http://org-pc23.fy.chalmers.se/forum/viewtopic.php?t=28 under Mac OS X: some normal characters don't appear when the selection changes. And if I select a part of the document containing math characters and delect with a click, the whole selection often completely disappears and is not redrawn, and when I move the mouse pointer, another part of the document sometimes disappears too.

> Vincent, would it be possible to get a somewhat smaller regression range?  I
> dunno whether we even have Linux-ppc nightlies... :(

No, there aren't even official Linux/ppc releases, AFAIK. However this could probably be done under Mac OS X.
Comment 9 Jungshik Shin 2006-03-06 02:05:26 PST
I don't see a problem on Linux (1.5.0.1). I have yet to test MathML on Windows and Mac OS X.
Comment 10 Vincent Lefevre 2006-03-06 05:44:48 PST
I've just tried under Linux/x86 with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.1) Gecko/20060226 Debian/1.5.dfsg+1.5.0.1-3 Firefox/1.5.0.1 and I have the same problem as under Linux/ppc (also Debian) with Mozilla's MathML start page: weird display. On http://org-pc23.fy.chalmers.se/forum/viewtopic.php?t=28 it is worse: see what I get just after loading the page:
  http://www.vinc17.org/download/ff-chalmers.png
Comment 11 Christian von Schultz 2006-03-06 06:13:20 PST
(In reply to comment #7)
> Er... you mean between 1.0 and 1.5, since the bug claims to have been
> introduced in 1.5?  That's between Gecko 1.7 and 1.8.  What _hasn't_ changed?
> 
> I'm not seeing a bug on Linux/x86 either, as far as I can tell.

I did some more testing, and on Linux i686 (or at least Debian GNU/Linux 3.1) the problem was introduced on 2005-09-13. I have updated http://www.dd.chalmers.se/~von/mathmlproblems/ to include new screenshots and links to the 2005-09-12 and 2005-09-13 tarballs that I tested.
Comment 12 Christian von Schultz 2006-03-06 07:23:07 PST
A MacOS user (Simon Finne) at our forum has now confirmed that the problem with http://org-pc23.fy.chalmers.se/forum/viewtopic.php?t=28 appears to have been introduced 2006-09-13. According to him the following version works: "Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b4) Gecko/20050912 Firefox/1.4"

While the next day's build doesn't: "Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b4) Gecko/20050913 Firefox/1.4"

And neither does the current version: "Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1"
Comment 13 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-03-06 08:18:34 PST
> the problem was introduced on 2005-09-13.

Looking at the checkin range there (which I hope is for the right branch; in general, testing with trunk builds works better, since we have multiple "1.8" branches):

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=MOZILLA_1_8_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-09-12-03&maxdate=2005-09-13-08&cvsroot=%2Fcvsroot

it looks like bug 307157 landed in that range.  Could that be the problem?
Comment 14 Christian von Schultz 2006-03-07 04:20:32 PST
(In reply to comment #13)
> it looks like bug 307157 landed in that range.  Could that be the problem?

I think it must be. I downloaded the latest sources from CVS (MOZILLA_1_8_BRANCH) and an older revision of nsMathMLChar.cpp (revision 1.108), and after compiling http://org-pc23.fy.chalmers.se/forum/viewtopic.php?t=28 works again. I have updated http://www.dd.chalmers.se/~von/mathmlproblems/ with new screenshots.

Another Windows user, claiming not to have installed the MathML fonts, did not experience problems (http://aia.yi.org/aia-ffox.jpg). And comparing http://aia.yi.org/aia-ffox.jpg and http://www.dd.chalmers.se/~von/mathmlproblems/firefox-windows-gecko20060111-0.png I think it is clear that they are using different fonts for the parentheses, making me think that this problem could be font related.
Comment 15 Jitse Niesen 2006-04-18 23:38:48 PDT
Created attachment 218945 [details]
Screenshot of Firefox displaying MathML incorrectly

I'm having similar problems (pages with MathML in them are not redrawn properly) on Debian Linux (etch distribution) and Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.1) Gecko/20060418 Debian/1.5.dfsg+1.5.0.1-4 Firefox/1.5.0.1. The problems appeared after installing the Mathematica 4.1 fonts. Recompiling with nsMathMLChar.cpp revision 1.108, as suggested above, fixed this problem. However, the MathML still is not displayed properly (see screenshot). Even the plus and equal signs are not displayed, but replaced by an 'unknown glyph' box.
Comment 16 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-04-19 07:37:00 PDT
regression
Comment 17 Christian von Schultz 2006-04-23 06:37:41 PDT
(In reply to comment #15)
> The problems appeared after installing the Mathematica 4.1
> fonts. Recompiling with nsMathMLChar.cpp revision 1.108, as suggested above,
> fixed this problem.

I do not experience problems with the Mathematica*.ttf fonts, but rather with the Math2 font. I downloaded firefox from CVS (MOZILLA_1_8_BRANCH), "Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8) Gecko/20060423 BonEcho/2.0a1", and did some testing with and without different fonts, details and screenshots at http://www.dd.chalmers.se/~von/mathmlproblems/
Comment 18 rbs 2006-09-25 20:16:18 PDT
I have tracked this bug down to --enable-pango that some distros seem to have (instead of bug 307157 as first thought).

Find your usr/bin/firefox script and in there, comment (as necessary):

#MOZ_ENABLE_PANGO=1
#export MOZ_ENABLE_PANGO

and set:

MOZ_DISABLE_PANGO=1
export MOZ_DISABLE_PANGO
Comment 19 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-09-25 20:25:11 PDT
How does that correspond to comment 14?
Comment 20 rbs 2006-09-25 20:35:50 PDT
No idea yet. Any feedback from those who observe the bug? Specifically, I would like you guys to first confirm or deny whether MOZ_DISABLE_PANGO does the trick for you in general. (It did for me in my limited testing.)
Comment 21 rbs 2006-09-25 23:25:53 PDT
Tentatively marking a dependency with bug 349904, which we know caused MathML not to work on Pango-enabled build.
Comment 22 rbs 2006-09-26 00:01:46 PDT
I can confirm that this is related to pango. I just made my own Linux build with --enable-pango, and this bug showed up in full force. And when I set MOZ_DISABLE_PANGO and relaunched firefox, the bug went away. My previous (default) build did not have --enable-pango, and I wasn't seeing this bug.

=========
Note: --enable-pango breaks the build process. When it happened, I had to go to the directory (the OBJ/gfx/src/gtk), copy-paste the failing command and add -lpangoft2-1.0, before re-doing another make -f client.mk build_all.
Comment 23 rbs 2006-09-26 00:20:03 PDT
I should add that my build does not use cairo since it breaks mathml.
Comment 24 Behdad Esfahbod 2006-09-26 09:06:23 PDT
I can reproduce this problem only when using the Math fonts, not with Computer Modern fonts.  And when I do, I can reproduce with both Pango and Xft renderers of Firefox 1.5.
Comment 25 rbs 2006-09-26 11:34:54 PDT
Created attachment 240179 [details] [diff] [review]
undo the fix of bug 307157

behdad, do you ming trying out this patch and telling me what you see?
Comment 26 rbs 2006-09-26 12:12:09 PDT
Filed bug 354357 about the fact that --enable-pango breaks th build process.
Comment 27 Behdad Esfahbod 2006-09-26 12:29:19 PDT
(In reply to comment #25)
> Created an attachment (id=240179) [edit]
> undo the fix of bug 307157
> 
> behdad, do you ming trying out this patch and telling me what you see?

Yes, that makes the problem completely go away.
Comment 28 Christopher Aillon (sabbatical, not receiving bugmail) 2006-09-26 18:02:16 PDT
Can we get this reviewed and committed then?  I'd really like to get this into the Red Hat RPMs and on the varying branches.
Comment 29 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-09-26 18:05:53 PDT
> Can we get this reviewed and committed then? 

What?  All the patch does is back out a fix that we really do want.
Comment 30 rbs 2006-09-26 22:07:17 PDT
> Comment #27 
> Yes, that makes the problem completely go away.

This is quite intriguing. The commented code is actually read-only. It doesn't set anything in the rendering context. Here is how the one of nsMathMLChar::PaintVertically looks like for example:

    nscoord overlap = onePixel;
#if 0
    nsCOMPtr<nsIFontMetrics> fm;
    aRenderingContext.GetFontMetrics(*getter_AddRefs(fm));
    nsMathMLFrame::GetRuleThickness(fm, overlap);
    overlap = 2 * PR_MAX(overlap, onePixel);
    while (overlap > onePixel && bmdata[3].ascent + bmdata[3].descent <= 2*overlap)
      overlap -= onePixel;

    // to protect against gaps, pretend the glue is smaller than 
    // it says to allow a small overlap when adjoining it
    bmdata[3].ascent -= overlap;
    bmdata[3].descent -= overlap;
#endif

With
====
NS_IMETHODIMP nsRenderingContextGTK::GetFontMetrics(nsIFontMetrics *&aFontMetrics)
{
  NS_IF_ADDREF(mFontMetrics);
  aFontMetrics = mFontMetrics;
  return NS_OK;
}

static void
nsMathMLFrame::GetRuleThickness(nsIFontMetrics* fm,
                   nscoord&        ruleThickness)
{
  nscoord xHeight;
  fm->GetXHeight(xHeight);
  ruleThickness = NSToCoordRound(40.000f/430.556f * xHeight);
}
=============

So it is all mysterious. 

behdad, as you can clearly see the bug, would mind helping to debug. You could try changing the added #if 0 to #if 1. Then, on a small example of a single markup, e.g., 
<math xmlns="http://www.w3.org/1998/Math/MathML">
  <mrow><mo minsize="10">(</mo></mrow>
<math>
print the overlap value that ended up being used, and then check if it turns out to be zero and is the culprit, or if gradually decreasing/increasing the overlap value, e.g. using n=1,2,... as indicated below, makes any difference:

n = 1; // each change requires a re-compilation
overlap = n*onePixel;
    bmdata[3].ascent -= overlap;
    bmdata[3].descent -= overlap;
Comment 31 Behdad Esfahbod 2006-09-27 07:40:01 PDT
(In reply to comment #29)
> > Can we get this reviewed and committed then? 
> 
> What?  All the patch does is back out a fix that we really do want.

In other words: It backs out a broken change.


(In reply to comment #30)
> > Comment #27 
> > Yes, that makes the problem completely go away.
> 
> This is quite intriguing. The commented code is actually read-only. It doesn't
> set anything in the rendering context. Here is how the one of
> nsMathMLChar::PaintVertically looks like for example:
> 
>     nscoord overlap = onePixel;
> #if 0
>     nsCOMPtr<nsIFontMetrics> fm;
>     aRenderingContext.GetFontMetrics(*getter_AddRefs(fm));
>     nsMathMLFrame::GetRuleThickness(fm, overlap);
>     overlap = 2 * PR_MAX(overlap, onePixel);
>     while (overlap > onePixel && bmdata[3].ascent + bmdata[3].descent <=
> 2*overlap)
>       overlap -= onePixel;
> 
>     // to protect against gaps, pretend the glue is smaller than 
>     // it says to allow a small overlap when adjoining it
>     bmdata[3].ascent -= overlap;
>     bmdata[3].descent -= overlap;
> #endif
> 
> With
> ====
> NS_IMETHODIMP nsRenderingContextGTK::GetFontMetrics(nsIFontMetrics
> *&aFontMetrics)
> {
>   NS_IF_ADDREF(mFontMetrics);
>   aFontMetrics = mFontMetrics;
>   return NS_OK;
> }
> 
> static void
> nsMathMLFrame::GetRuleThickness(nsIFontMetrics* fm,
>                    nscoord&        ruleThickness)
> {
>   nscoord xHeight;
>   fm->GetXHeight(xHeight);
>   ruleThickness = NSToCoordRound(40.000f/430.556f * xHeight);
> }
> =============
> 
> So it is all mysterious. 

Not to me:

>     while (overlap > onePixel && bmdata[3].ascent + bmdata[3].descent <=
> 2*overlap)
>       overlap -= onePixel;
> 
>     // to protect against gaps, pretend the glue is smaller than 
>     // it says to allow a small overlap when adjoining it
>     bmdata[3].ascent -= overlap;
>     bmdata[3].descent -= overlap;

I bet it's the case that one of ascent or descent is smaller than overlap and becoming a negative or underflowed value.

> behdad, as you can clearly see the bug, would mind helping to debug.

Ok, will debug later today.
Comment 32 rbs 2006-09-27 08:24:42 PDT
>I bet it's the case that one of ascent or descent is smaller than overlap and
>becoming a negative or underflowed value.

Perhaps. But that is a local variable for local purposes, and is not poking at an internal buffer that the life of Pango or Xft depend on. Plus there do exists chars with negative natural metrics. Plus why does it then break all chars. Anyway, let see what your debugging will say.

Here is Math2. It does have tiny glyphs indeed:
http://www.mozilla.org/projects/mathml/fonts/encoding/math2.gif

BTW, the backout patch is simple enough that we can get it in the branches at the last minute (so I hope). But I would hope that we can instead first try to get to the bottom of the real matter and perhaps resolve it. I am requesting branch blocker status as a way of putting this bug on the radar of drivers, given the existing fallback patch.
Comment 33 Mike Connor [:mconnor] 2006-09-27 10:12:56 PDT
not going to block 1.8.1 on this, this only occurs with pango, so distros using pango can make their own decisions.
Comment 34 Daniel Veditz [:dveditz] 2006-09-27 10:35:10 PDT
1.8.0.8 is just for parity with the Firefox 2 release (mainly security fixes). The next "real" 1.8.0.x release is 1.8.0.9, but please don't nominate until this patch is reviewed and landed on trunk or 1.8-branch for verification (see tree rules at top of the Mozilla1.8.0 tinderbox).
Comment 35 Christopher Aillon (sabbatical, not receiving bugmail) 2006-09-27 11:57:38 PDT
Red Hat, along with other distributions, has had significant battles to fight in trying to redistribute the browser as Firefox with Pango enabled, with MathML not working being a significant part of the argument against us being able to keep the Firefox branding.  With the work we've done to make sure that pango and math work, I'd like to make sure it continues to work (even though this appears to be related to specific fonts and *not pango*).  Given shaver's comments at https://bugzilla.mozilla.org/show_bug.cgi?id=349904#c6 along with the trademarking issues, I'd say there is an interest for the Mozilla Corporation to ensure that all distros choosing to build pango (are there any that don't?) are guaranteed to have working MathML in the branches.

Thus, re-nominating to block branches.  I'm not saying we should back this fix out right now if people are going to be working on a real fix, but I want this bug on the radar to make sure that if there is no real fix for it before releases, we do take the backout at that point.
Comment 36 rbs 2006-09-27 14:04:06 PDT
I back caillon comments above. We have this bug on track. Please, drivers, this is a raisonnable blocker request. If you wish a resolution on the bug because either of the branches is about to be closed, we can provide one in a minute with the fallback patch -- the time to get r+sr+a. So it won't hold the branches up.
Comment 37 rbs 2006-09-27 14:05:56 PDT
Comment on attachment 240179 [details] [diff] [review]
undo the fix of bug 307157

bz, in the light of the previous comments, would you care to give your r+sr to secure the patch?
Comment 38 Boris Zbarsky [:bz] (Out June 25-July 6) 2006-09-27 17:15:33 PDT
Comment on attachment 240179 [details] [diff] [review]
undo the fix of bug 307157

I don't think I actually can -- I don't understand this code, and won't have time to in the timeframe we're talking about.

Trying roc, since he reviewed bug 307157.
Comment 39 rbs 2006-09-27 17:34:29 PDT
Comment on attachment 240179 [details] [diff] [review]
undo the fix of bug 307157

roc, this is a patch that  backout the fix of 307157 because it is causing an obscure bug on Linux. I ideally would not like to backout on the trunk, but I would like to have this patch ready to go on the branches. However to make a case for blocker status, I would like to get r+sr so that I can check it on the branches at the last minute if need be. Thanks.
Comment 40 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2006-09-27 19:13:14 PDT
Comment on attachment 240179 [details] [diff] [review]
undo the fix of bug 307157

The backout is OK, but someone needs to figure out what's really going on here.
Comment 41 Mike Schroepfer 2006-09-27 22:59:31 PDT
If this is what you want for 1.8.1 branch I thin we'd be happy to take it.  Just nominate the patch you want approved asap. 
Comment 42 Mike Beltzner [:beltzner, not reading bugmail] 2006-09-28 10:45:11 PDT
Not blocking. As schrep said, nominate a patch for 1.8.1approval by this afternoon PDT today if you want it in for tomorrow's code freeze on the MOZILLA_1_8_1_BRANCH
Comment 43 Mike Connor [:mconnor] 2006-09-28 10:46:58 PDT
Comment on attachment 240179 [details] [diff] [review]
undo the fix of bug 307157

a=mconnor on behalf of drivers, please land ASAP
Comment 44 rbs 2006-09-28 14:46:47 PDT
Checked in the m1.8 branch. Better have some gaps than nothing. 

The trunk remains with the status quo, until we get further insight as to what is really going on.
Comment 45 Daniel Veditz [:dveditz] 2006-09-29 11:07:48 PDT
adding fixed1.8.1 keyword, rbs checked in yesterday.
Comment 46 Daniel Veditz [:dveditz] 2006-09-29 11:09:24 PDT
Comment on attachment 240179 [details] [diff] [review]
undo the fix of bug 307157

Like the 1.8.1 team we won't block on it, but do approve the patch if it lands in time. a=dveditz for drivers
Comment 47 Daniel Veditz [:dveditz] 2006-09-29 11:09:52 PDT
approved for 1.8.0.8 that is.
Comment 48 rbs 2006-09-29 16:31:38 PDT
fixed1.8.0.8 - checked in the 1.8.0 branch.
Comment 49 Behdad Esfahbod 2006-10-04 19:01:54 PDT
I'm debugging this.  It's happening only when descent becomes negative (with torture test 22):

onePixel is 15
BEFORE: overlap 34; ascent=105 descent=0
AFTER: overlap 34; ascent=71 descent=-34


Investigating it more.  But, this bug or not, isn't it silly to use as much overlap to make the glue essentially zero-width or zero-height?  What would happen then?  Use an infinitely many of them?
Comment 50 Behdad Esfahbod 2006-10-04 19:10:07 PDT
Ok, this is where the bug lies:

      while (dy + bm.descent < start[i+1]) {
        if (count++ < 2) {
          stride = bm.descent;
          bm = bmdata[3]; // glue
          stride += bm.ascent;
        }
        // defensive code against odd things such as a smallish TextZoom...
        NS_ASSERTION(1000 != count, "something is probably wrong somewhere");
        if (stride < onePixel || 1000 == count) return NS_ERROR_UNEXPECTED;
        dy += stride;
        aGlyphTable->DrawGlyph(aRenderingContext, aFont, glue, dx, dy);
      }

See how after count >= 2 it keeps adding bmdata[3].descent and bail out after count == 1000?
Comment 51 Behdad Esfahbod 2006-10-04 19:24:02 PDT
Ok, ignore the previous comment.  Here is the exact analysis, and it's happening because of the horizontal one, not vertical:

    nscoord overlap;
#if 1
    nsCOMPtr<nsIFontMetrics> fm;
    aRenderingContext.GetFontMetrics(*getter_AddRefs(fm));
    nsMathMLFrame::GetRuleThickness(fm, overlap);
    overlap = 2 * PR_MAX(overlap, onePixel);
    while (overlap > onePixel && bmdata[3].rightBearing - bmdata[3].leftBearing <= 2*overlap)
      overlap -= onePixel;

    // to protect against gaps, pretend the glue is smaller than 
    // it says to allow a small overlap when adjoining it
    bmdata[3].leftBearing += overlap;
    bmdata[3].rightBearing -= overlap;
#endif

    for (i = 0; i < 2; i++) {
      PRInt32 count = 0;
      dx = offset[i];
      clipRect.SetRect(end[i], aRect.y, start[i+1]-end[i], aRect.height);
      clipRect.Inflate(overlap, overlap);
      aRenderingContext.PushState();
      aRenderingContext.SetClipRect(clipRect, nsClipCombine_kIntersect);
      bm = bmdata[i];
      while (dx + bm.rightBearing < start[i+1]) {
        if (count++ < 2) {
          stride = bm.rightBearing;
          bm = bmdata[3]; // glue
          stride -= bm.leftBearing;
        }
        // defensive code against odd things such as a smallish TextZoom...
        NS_ASSERTION(1000 != count, "something is probably wrong somewhere");
        if (stride < onePixel || 1000 == count) return NS_ERROR_UNEXPECTED;
        dx += stride;
        aGlyphTable->DrawGlyph(aRenderingContext, aFont, glue, dx, dy);
      }
      aRenderingContext.PopState();
    }
  }

Now, after overlap computation, if rbearing-lbearing becomes smaller than onePixel, the "if (stride < onePixel..." will trigger in the second run of the while loop.
Comment 52 Behdad Esfahbod 2006-10-04 19:31:57 PDT
Created attachment 241278 [details] [diff] [review]
proposed patch
Comment 53 rbs 2006-10-05 03:08:55 PDT
Comment on attachment 241278 [details] [diff] [review]
proposed patch

Looks reasonnable. Since overlap can now end up negative, there is a final thing to adjust:
+    if (overlap > 0) {
...
}

Add:
+ nscoord leak = PR_MAX (overlap, onePixel);

- clipRect.Inflate(overlap, overlap);
+ clipRect.Inflate(leak, leak);

[This is needed to mitigate the effect of the glyphs' ink leaking out of our positioning in case of subpixel rounding errors that cause some partial glyphs in some fonts to be painted in some devices at more or less the requested (dx,dy) and end up looking "hairy"]
Comment 54 rbs 2006-10-06 08:28:09 PDT
Created attachment 241453 [details] [diff] [review]
patch to be checked in

Iteration on Behdad's patch to include the fixups while here.
Comment 55 rbs 2006-10-06 08:31:13 PDT
Comment on attachment 241453 [details] [diff] [review]
patch to be checked in

r+sr=rbs

Hope we can get this on the branches too so as to backout the backout that went there.
Comment 56 rbs 2006-10-06 08:39:10 PDT
Checked in.
Comment 57 rbs 2006-10-06 09:04:28 PDT
->FIXED

Thanks Behdad for the patch and to all of you guys for keeping the ball rolling here -- please be sure to get a nightly and ensure that you don't see the bug anymore. It is not a nice feeling to discover later that such bugs are in the final release.

Christian von Schultz -- your page http://www.dd.chalmers.se/~von/mathmlproblems/
was particular informative.

You had this screenshot entitled:
"With everything but Math2 installed: looks fine, apart from a few 'unknown glyph' boxes."
http://www.dd.chalmers.se/~von/mathmlproblems/firefox-linux-gecko20060423-6.png

Please be sure to check you don't get even these few 'unknown glyph' boxes, Otherwise, file another bug to pursue the matter.
Comment 58 rbs 2006-10-08 23:27:19 PDT
Comment on attachment 241453 [details] [diff] [review]
patch to be checked in

drivers, we now have a real fix. In the heat of the action, we earlier had to checkin a backout. Is that possible to backout that backout and take this real fix instead? (I am not yet up to speed with the current branch approval flags  -- but I hope these mean the ones for the trees pulled with the 1.8 and 1.8.0 cvs tags.)
Comment 59 Mike Connor [:mconnor] 2006-10-10 10:44:38 PDT
Comment on attachment 241453 [details] [diff] [review]
patch to be checked in

too late for 1.8.1, pushing to 1.8.1.1 noms
Comment 60 Daniel Veditz [:dveditz] 2006-10-10 16:07:41 PDT
Comment on attachment 241453 [details] [diff] [review]
patch to be checked in

Would be nice to have, but the 1.8.0.8 charter is parity with 1.8.1 so I've got to bump this out.
Comment 61 Daniel Veditz [:dveditz] 2006-11-27 11:52:10 PST
I'm unclear on the current status of this bug on the branches -- sounds like there was a backout that's hiding this bug there anyway?
Comment 62 rbs 2006-11-27 12:01:40 PST
Yes, there was a backout that is hiding this bug there, but it is a tradeoff that comes at a price of something else. That's why branch approvals are still requested for the right fix that does not have the known side effect.
Comment 63 Daniel Veditz [:dveditz] 2006-11-27 17:50:02 PST
Comment on attachment 241453 [details] [diff] [review]
patch to be checked in

approved for 1.8/1.8.0 branches, a=dveditz for drivers
Comment 64 rbs 2006-11-30 04:02:07 PST
Correct fix landed on the 1.8.0 and 1.8 branches.
Comment 65 Carsten Book [:Tomcat] 2006-12-14 13:26:57 PST
Verified fixed for 1.8.0.9 and 1.8.1.1 with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.1) Gecko/20061208 Firefox/2.0.0.1 ID:2006120814 
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.9) Gecko/20061206 Firefox/1.5.0.9
on Fedora FC6.

Removing also removing keywords fixed1.8.0.8/1.8.1, because 1.8.0.8/1.8.1 is still shipped without this patch.

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