Closed
Bug 257978
Opened 20 years ago
Closed 19 years ago
Firefox does not display minus sign inside mathml 'mo' tag but Netscape 7.1 does.
Categories
(Core :: MathML, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: alex, Assigned: makotoy)
References
()
Details
(Keywords: fixed1.8.1)
Attachments
(4 files, 2 obsolete files)
(deleted),
application/octet-stream
|
Details | |
(deleted),
application/octet-stream
|
Details | |
(deleted),
image/gif
|
Details | |
(deleted),
patch
|
rbs
:
review+
rbs
:
superreview+
dbaron
:
approval-branch-1.8.1+
mtschrep
:
approval1.8.0.1-
asa
:
approval1.8rc2-
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a3) Gecko/20040720 Firefox/0.9.1+ Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a3) Gecko/20040720 Firefox/0.9.1+ Firefox behaves inconsistently with Netscape 7.1. Basic support of rendering the 'mo' (math operator) tag seems to not be working. The example URL should render '-1' but it doesn't in Firefox. In Netscape 7.1, it works just fine. So, the big question is whether there is a different version of the Mathml implemenation at work here given that these two browsers were released at different times. Obviously, this is a severe bug. The minus sign is rather important. :) Reproducible: Always Steps to Reproduce: 1. 2. 3. Actual Results: 1 Expected Results: -1
Updated•20 years ago
|
Assignee: firefox → rbs
Component: General → MathML
Product: Firefox → Browser
QA Contact: firefox.general → ian
Version: unspecified → Trunk
There is no problem with the Trunk version Camino. Mac OS X 10.3.5 2004090321 (v0.8+)
Reporter | ||
Comment 2•20 years ago
|
||
(In reply to comment #1) > There is no problem with the Trunk version Camino. > > Mac OS X 10.3.5 > 2004090321 (v0.8+) > Netscape 7.1 will render the document correctly but Netscape 7.2 and Firefox 0.9 will not
Reporter | ||
Comment 4•20 years ago
|
||
Camino does not have MathML enabled. Please look at: http://www.w3.org/Math/testsuite/testsuite/Presentation/TokenElements/mo/mo5.xml You should see a fraction of a/b and you do not in Camino. Yes, the minus sign shows up in Camino but that is because it is not rendering the MathML.
oops... It reproduced with the trunk version mozilla. Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a4) Gecko/20040910
Reporter | ||
Comment 6•20 years ago
|
||
On line 230 of nsMathMLmoFrame.cpp the regular minus sign is suppose to be translated to 0x2212. See: http://lxr.mozilla.org/mozilla/source/layout/mathml/base/src/nsMathMLmoFrame.cpp#230 Interestingly enough, the character 0x2212 (minus sign from 'Math Operators') does display properly on Mac OS X. See: http://www.milowski.com/badminus.xhtml But the regular minus sign does not display.
Reporter | ||
Comment 8•20 years ago
|
||
Reporter | ||
Comment 9•20 years ago
|
||
Here's an additional document: http://www.milowski.com/badminus.xhtml It shows how the regular minus sign (0x0d) does not show up but the math code page minus does.
Comment 10•20 years ago
|
||
Can you update you document (badminus) with this: <p>The hypen-minus sign: -</p> <!--- add this line --> <p>The Unicode minus sign: −</p> ... keep same content ... Then attach a screenshot? The reason I am asking this is to confirm/deny whether the problem comes just from the re-mapping of '-' to 0x2212 (cf. your comment 6). Or if there is something deeper.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Reporter | ||
Comment 11•20 years ago
|
||
Reporter | ||
Comment 12•20 years ago
|
||
I've updated http://www.milowski.com/badminus.xhtml as requested and attached a screenshot.
Assignee | ||
Comment 14•19 years ago
|
||
> Is this bug still there with recent builds? Yes, there is, and in fact there is aleady a workaround for this. Set "font.mathfont-family.\u2212.base" to "Times" or "Lucida Grande" or whatever a valid font name on Mac OS X. I guess this is caused by the line font.mathfont-family.\u2212.base = Symbol in mathfont.properties together with the fact that we should write "Sigma-psi-mu-beta-omicron-lambda" to refer to the Symbol font (at least on Mac). Have soomething to do with the bug 213702 ? Interestingly, if the minus sign is specified directly as u2212 in the source mathml this problem does not happen. If we use u002D, it's not rendered (of course there is no problem outside mathml).
Comment 15•19 years ago
|
||
Do you mean that <mo>−</mo> works fine? (but <mo>-</mo> doesn't work.)
Assignee | ||
Comment 16•19 years ago
|
||
Yes. Mac people please check out the rendering of http://www.ms.u-tokyo.ac.jp/~makotoy/ffmath/minus.xml by out-of-box DeerPark and by the one with font.mathfont-family.\u2212.base configuration.
Comment 17•19 years ago
|
||
What about ≤ (u2264) and ≥ (u2265)? [These are the other characters for which the .base font is set as Symbol in mathfont.properties] Do they disappear too?
Assignee | ||
Comment 18•19 years ago
|
||
Oddly they don't. Moreover if I set font.mathfont-family.\u2264.base to "Sigma-lambda-...", u2264 is rendered as a double prime or something. u2265 is not affected anyway.
Comment 19•19 years ago
|
||
I am beginning to suspect that this is a general problem that also happens in plain HTML -- not just MathML. For example, it seems to me that you might also see the problem with: <span style="font-family:Symbol"> − ≥ ≤ </span> inside HTML.
Assignee | ||
Comment 20•19 years ago
|
||
I've forgotten to point out important things... #14> whatever a valid font name includes "Sigma-psi-.." and commenting out the line ..base of mathfont.properties works too. I incorporated your suggestion into the test page http://www.ms.u-tokyo.ac.jp/~makotoy/ffmath/minus.xml and it looks like general-HTML (unicode) codepoint-glyph correspondence is broken for Symbol. The attribute style="font-family:Symbol" is no-op and style="font-family:Sigma-psi-.." (actually greek letters used) makes the english text "greeked." For "true greek" letters some fallback font is used instead of Symbol. I attached the screenshot of that test page by Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20051024 Firefox/1.6a1 with no configuration. u2264 and u2265 are disappearing there but this does not reproduce always (contrary to u002D), see the test page images.
Comment 21•19 years ago
|
||
Any luck when adding Math1 in mathfont.properties? That is, try changing: font.mathfont-family.\u2264.base = Symbol font.mathfont-family.\u2265.base = Symbol font.mathfont-family.\u2212.base = Symbol to: font.mathfont-family.\u2264.base = Symbol, Math1 font.mathfont-family.\u2265.base = Symbol, Math1 font.mathfont-family.\u2212.base = Symbol, Math1 Math1 has all the glyphs that Symbol has (and more): http://www.mozilla.org/projects/mathml/fonts/encoding/math1.gif http://www.mozilla.org/projects/mathml/fonts/encoding/symbol.gif
Comment 22•19 years ago
|
||
Or alternatively, "Math1, Symbol". This will however give precedence to Math1 on all platforms (for the listed characters). If it works on the Mac, it will be worth the trade-off.
Assignee | ||
Comment 23•19 years ago
|
||
I've put screenshots of http://www.ms.u-tokyo.ac.jp/~makotoy/ffmath/minus.xml under different configs in that page. Math1 is not working for the minus sign (but working for le & ge), u002D and u2212 are rendered differently in some cases, etc... It looks best to just eliminate those three lines from mathfont.properties if avoid Sigma-psi-.. or Times or something. I'm goin to look into gfx/src/mac to see if there can be a fundamental fix for this bug (e.g. nsATSUIUtils.cpp has some ovious bugs).
Comment 24•19 years ago
|
||
simon, do you have a Mac build environment? If so, would you mind chiming in here?!? Here is the story: to paint normal chars like − (\u2212), all we do here is set the font-family (fetched from font.mathfont-family.\u2212.base) in the font.name struct (using the SetBaseFamily() helper function), then call aRenderingContext.SetFont() and then DrawString() in the usual way. This is the corresponding code fragment. You can uncomment the printf and the color to see it red: http://lxr.mozilla.org/mozilla/source/layout/mathml/base/src/nsMathMLChar.cpp#2019 2021 PRUint32 len = PRUint32(mData.Length()); 2022 if (1 == len) { 2023 SetBaseFamily(mData[0], theFont); 2024 } 2025 aRenderingContext.SetFont(theFont, nsnull); 2026 //printf("Painting %04X like a normal char\n", mData[0]); 2027 //aRenderingContext.SetColor(NS_RGB(255,0,0)); 2028 aRenderingContext.DrawString(mData.get(), len, mRect.x, mRect.y + mBoundingMetrics.ascent) This should in fact be equivalent to what we do for <span style="font-family: Symbol">...</span>, except that the langGroup parameter is passed as null in the MathML's SetFont() call -- this defaults to the user's locale langGroup (see DeviceContextImpl::GetMetricsFor), as opposed to be the element langGroup as nsTextFrame's SetFontFromStyle() does. I am wondering if this brings a difference in the <span> vs. <mo> on the Mac that other platforms don't have.
Assignee | ||
Comment 25•19 years ago
|
||
Sorry for being so late, but I have a fix for the "minus sign missing" issue (not for the Symbol font handling issue, I've learned these two are separated) That was caused by poor TEC fallback algorithm for GetBoundingMetrics/Draw process (ll 292-300 and 346-353 in nsUnicodeRenderingToolkit.cpp). It assumes nsUnicodeFontMapping knows about fallback font determined by the script, and does nothing if it fauks the unicode->scriptdata conversion. Just giving up then i.e. append "else { return PR_FALSE; }" there is the solution. In addition ASTUI fallback has a room for improvement in handling fontID and italic/boldness. I gonna prepare the patch ASAP. BTW, can I put "CFLAGS += -funsigned-char" in Makefile.in?
Assignee | ||
Comment 26•19 years ago
|
||
TEC fallback refinement and nsATSUIUtils: get font family by Font Manager API (some fonts are not accessible via ATSUFONDtoFontID that is "not recommended" by Apple), Italic and boldness flags are reflected
Attachment #201612 -
Flags: superreview?(rbs)
Attachment #201612 -
Flags: review?(rbs)
Comment 27•19 years ago
|
||
(In reply to comment #24) > simon, do you have a Mac build environment? Sorry, no. Passing the buck to Jungshik and Asaf.
Comment 28•19 years ago
|
||
Index: nsATSUIUtils.cpp =================================================================== +#define MY_ATTR_CNT 5 + ATSUAttributeTag theTag[MY_ATTR_CNT]; + ByteCount theValueSize[MY_ATTR_CNT]; + ATSUAttributeValuePtr theValue[MY_ATTR_CNT]; Nicer to move the #define at the header -- next to the existing "#define FloatToFixed(a)". Also drop "MY_" and simply say ATTR_CNT. //--- Font ID & Face ----- ATSUFontID atsuFontID; - - err = ::ATSUFONDtoFontID(aFontNum, (StyleField)((aBold ? bold : normal) | (aItalic ? italic : normal)), &atsuFontID); - if(noErr != err) { - NS_WARNING("ATSUFONDtoFontID failed"); - // goto errorDoneDestroyStyle; + FMFontFamily myFontFam; + FMFontStyle fbStyle; + myFontFam = ::FMGetFontFamilyFromATSFontFamilyRef(aFontNum); + if (::FMGetFontFromFontFamilyInstance(myFontFam, 0, &atsuFontID, &fbStyle) == kFMInvalidFontErr) + { + NS_WARNING("FMGetFontFromFontFamilyInstance failed"); This was supposed to be ATSU-based... there you go back again to FM. Some FM functions were also reported as broken in bug 246527 comment 3. Care to give a reference/link for your comment that "ATSUFONDtoFontID is not recommended by Apple" theValue[2] = (ATSUAttributeValuePtr) &color; //--- Color ----- + //--- Boldness ---- + Boolean isBold = false; + if (aBold) { + isBold = true; + } Is this not equivalent to "Boolean isBold = aBold"? [Or at the very least Boolean isBold = aBold ? true : false] Also a leave a blank line after "Color" and simply say "Bold" instead of "Boldness". + theTag[3] = kATSUQDBoldfaceTag; + theValueSize[3] = (ByteCount) sizeof(Boolean); + theValue[3] = (ATSUAttributeValuePtr) &isBold; + //--- Italicness ---- + Boolean isItalic; + if (aItalic) { + isItalic = true; + } Same as above. Also another spot to insert a blank line and say "Italic". Index: nsUnicodeRenderingToolkit.cpp =================================================================== RCS file: /cvsroot/mozilla/gfx/src/mac/nsUnicodeRenderingToolkit.cpp,v if(fontMapping.ConvertUnicodeToGlyphs(scriptFallbackFonts[fallbackScript], aCharPt, 1, buf, STACK_TRESHOLD, outLen, processBytes, kUnicodeLooseMappingsMask)) { ::TextFont(scriptFallbackFonts[fallbackScript]); GetScriptTextBoundingMetrics(buf, outLen, fallbackScript, oBoundingMetrics); ::TextFont(fontNum); - } + } else { + return PR_FALSE; + } return PR_TRUE; } What about moving the |return PR_TRUE| inside the |if|. This way, you get if(fontMapping.ConvertUnicodeToGlyphs)) { ... return PR_TRUE; } return PR_FALSE; for(fallbackScript = 0; fallbackScript < 32; fallbackScript++) { if(BAD_FONT_NUM != scriptFallbackFonts[fallbackScript]) { if(fontMapping.ConvertUnicodeToGlyphs(scriptFallbackFonts[fallbackScript], aCharPt, 1, buf, STACK_TRESHOLD, outLen, processBytes, kUnicodeLooseMappingsMask)) { NS_PRECONDITION(0 == (processBytes % 2), "strange conversion result"); ::TextFont(scriptFallbackFonts[fallbackScript]); GetScriptTextBoundingMetrics(buf, outLen, fallbackScript, oBoundingMetrics); ::TextFont(fontNum); break; - } + } else { + return PR_FALSE; + } } } Why the return here? Seems to me you want to let the char to fall through and put in the cache. if(0 == outLen) fallbackScript = BAD_SCRIPT; // put into cache cache->Set(*aCharPt, fallbackScript); @@ -344,17 +348,19 @@ PRBool nsUnicodeRenderingToolkit :: TECF return PR_FALSE; if(fontMapping.ConvertUnicodeToGlyphs(scriptFallbackFonts[fallbackScript], aCharPt, 1, buf, STACK_TRESHOLD, outLen, processBytes, kUnicodeLooseMappingsMask)) { ::TextFont(scriptFallbackFonts[fallbackScript]); DrawScriptText(buf, outLen, x, y, oWidth); ::TextFont(origFontNum); - } + } else { + return PR_FALSE; + } return PR_TRUE; } return PR_FALSE; Also move the |return PR_TRUE| in the |if| so as not to use the |else|.
Comment 29•19 years ago
|
||
(In reply to comment #27) > (In reply to comment #24) > > simon, do you have a Mac build environment? > > Sorry, no. Passing the buck to Jungshik and Asaf. Ok. I'll try the latest patch here (well, perhaps, rbs' comments are addressed) on Mac.
Assignee | ||
Comment 30•19 years ago
|
||
Sorry for naive writing :(
Attachment #201612 -
Attachment is obsolete: true
Attachment #201794 -
Flags: superreview?(rbs)
Attachment #201794 -
Flags: review?(rbs)
Attachment #201612 -
Flags: superreview?(rbs)
Attachment #201612 -
Flags: review?(rbs)
Comment 31•19 years ago
|
||
Comment on attachment 201794 [details] [diff] [review] TEC Fallback, nsATSUIUtils patch refinement Looks good to me. Asking r=jshin per comment #29 so that the extra verifications can help with getting the approval for the 1.8 branch.
Attachment #201794 -
Flags: review?(rbs) → review?(jshin1987)
Comment 32•19 years ago
|
||
Comment on attachment 201794 [details] [diff] [review] TEC Fallback, nsATSUIUtils patch refinement I applied the patch and confirmed that '-' gets rendered well. >+ // The use of ATSUFONDtoFontID is not recommended, see http://developer.apple.com/documentation/Carbon/Reference/ATSUI_Reference/atsu_reference_Reference/chapter_1.2_section_19.html >+ // the origin of the value of aFontNum is nsDeviceContextMac.cpp#820, FMGetNextFontFamily. So we cannot expecet that to be a simple FOND resource value. Please, make the above comment fit within 80 chars-long lines except for the url taking a line by itself. // The use of .... , see // http://developer.... very long url // The origin of the value of aFontNum ..... // ....So we cannot ...
Attachment #201794 -
Flags: review?(jshin1987) → review+
Assignee | ||
Comment 33•19 years ago
|
||
Sorry for re-requesting your review but I found out that I've misunderstood the data types. FMFontFamily is not equivalent to ATSFontFamilyRef and nsDeviceContextMac.cpp#820 generates exactly an FMFontFamily value. So + // the origin of the value of aFontNum is nsDeviceContextMac.cpp#820, FMGetNextFontFamily. So we cannot expecet that to be a simple FOND resource value. + FMFontFamily fontFam; + FMFontStyle fbStyle; + fontFam = ::FMGetFontFamilyFromATSFontFamilyRef(aFontNum); + if (::FMGetFontFromFontFamilyInstance(fontFam, 0, &atsuFontID, &fbStyle) == kFMInvalidFontErr) { ... should be just + // by FMGetNextFontFamily, as an FMFontFamily. + FMFontStyle fbStyle; + if (::FMGetFontFromFontFamilyInstance(aFontNum, 0, &atsuFontID, &fbStyle) == kFMInvalidFontErr) { ...
Attachment #201794 -
Attachment is obsolete: true
Attachment #201838 -
Flags: superreview?(rbs)
Attachment #201838 -
Flags: review?(jshin1987)
Attachment #201794 -
Flags: superreview?(rbs)
Comment 34•19 years ago
|
||
No need for this comment: + // the value of aFontNum is born in nsDeviceContextMac.cpp#820, + // by FMGetNextFontFamily, as an FMFontFamily. aFontNum is an argument ('a'FontNum). Anyone patching a function must trace where arguments come from, so saying where one comes from make no difference to the basic tenet. Also, the #line notation is is not meant for permanent references, the code evolves and line numbers change all the time.
Comment 35•19 years ago
|
||
Comment on attachment 201838 [details] [diff] [review] nsATSUIUtils patch refinement sr=rbs, the unneeded comment can be removed at check-in,
Attachment #201838 -
Flags: superreview?(rbs) → superreview+
Comment 36•19 years ago
|
||
Comment on attachment 201838 [details] [diff] [review] nsATSUIUtils patch refinement Transfering jshin's review.
Attachment #201838 -
Flags: review?(jshin1987) → review+
Comment 37•19 years ago
|
||
Checked in.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Comment 38•19 years ago
|
||
Comment on attachment 201838 [details] [diff] [review] nsATSUIUtils patch refinement Requesting the approval of drivers for the 1.8 branch. This patch is necessary to display the minus sign in MathML. Without the patch, the minus sign does not show up, hence expressions like "a - b" show up as "a b". As you can imagine, this corrupts the meaning of expressions, making them confusing and unintelligible. The matter is made even worse by the fact that minus is a common place symbol used all the time. The patch touches two files. In nsUnicodeRenderingToolkit.cpp, it fixes misplaced |return|s in the TEC fallback code of GetBoundingMetrics/Drawtring -- this the root of the bug. In nsATSUIUtils, it replaces a non-recommended Apple API (ATSUFONDtoFontID) by something more reliable, and in the process enables the support of bold and italic. http://developer.apple.com/documentation/Carbon/Reference/ATSUI_Reference/atsu_reference_Reference/chapter_1.2_section_19.html Unfortunately, as the Mac doesn't receive as much attention as other platforms, bugs such as these tend to hang around for a long time. The patch was finally produced by YAMASHITA Makoto, tested and reviewed by jshin on his Mac, and super-reviewed by me. Approving the patch would mean that MathML is essentially in parity on all platforms out-of-the-box (modulo getting the fonts -- which all platforms have to do anyway, and setting to pref to get rid of the harmless alert about missing fonts).
Attachment #201838 -
Flags: approval1.8rc2?
Comment 39•19 years ago
|
||
Comment on attachment 201838 [details] [diff] [review] nsATSUIUtils patch refinement we've already got final bits and this is not a new problem. Mac will have to lag behind for one more release. We can try to get this in for the follow-up release.
Attachment #201838 -
Flags: approval1.8rc2? → approval1.8rc2-
Comment 40•19 years ago
|
||
What are the potential effects of this patch on non-MathML content? Is this code used only for MathML, or for other things?
Comment 41•19 years ago
|
||
I've tried out the fix (together with prefs.js adjustments). Looks good to me. Very much appreciated by mac users methinks!
Updated•19 years ago
|
Flags: blocking1.8.1?
Comment 42•19 years ago
|
||
Re: comment #40 It is a very low-risk for non-MathML content. The code affected is in a special fallback codepath where we do the ConvertUnicodeToGlyphs business to map chars to glyphs in a symbolic font. Normal content rarely gets there because their glyphs are found early on in the normal fonts that don't involve the special symbolic fonts that MathML needs.
Attachment #201838 -
Flags: approval1.8.0.1?
Comment 43•19 years ago
|
||
Comment on attachment 201838 [details] [diff] [review] nsATSUIUtils patch refinement Please consider for 1.8.1 -this is too risky for a minor patch.
Attachment #201838 -
Flags: approval1.8.1?
Attachment #201838 -
Flags: approval1.8.0.1?
Attachment #201838 -
Flags: approval1.8.0.1-
Updated•19 years ago
|
Attachment #201838 -
Flags: approval1.8.1? → branch-1.8.1?(dbaron)
Comment 45•19 years ago
|
||
Comment on attachment 201838 [details] [diff] [review] nsATSUIUtils patch refinement branch-1.8.1=dbaron, but please get it in reasonably early
Attachment #201838 -
Flags: branch-1.8.1?(dbaron) → branch-1.8.1+
*** Bug 330723 has been marked as a duplicate of this bug. ***
You need to log in
before you can comment on or make changes to this bug.
Description
•