Closed Bug 306543 Opened 17 years ago Closed 17 years ago

Inter-spacing improvements

Categories

(Core :: MathML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: rbs, Assigned: rbs)

References

Details

(Keywords: fixed1.8)

Attachments

(3 files, 2 obsolete files)

Many users (publicly in the n.p.m.mathml newsgroup and privately to me) have
reported that the inter-space between MathML frames is not always ideal. It is
an area that I have always been thinking of revisiting. I have now been able to
do so, and will submit my changes through this bug.
Attached patch patch (obsolete) — Splinter Review
The PDF printout was produced by browsing (with the patch) this MathML Page:

http://abel.math.harvard.edu/~dmharvey/blahtex/0.2/page-34.php

I simply printed from the browser to a PDF driver (PDFCreator in this case),
hence it is as-is. It is hard to tell the difference with TeX PNGs now. You
will notice that the maths use TeX fonts across the board instead of the usual
Times Roman. For the purpose of the experiment, I did set TeX fonts in
mathml.css -- that's all. They look okay on the PDF but don't alias very well
on the browser screen itself. That's why I still haven't set them as the
default in place of Times Roman in the release builds.
Attachment #194502 - Attachment is patch: false
Attachment #194502 - Attachment mime type: text/plain → application/pdf
Depends on: 247151
roc, sorry to flood you with these review requests. They seem inevitable to
make things better. As understood from the beginning, the intend is not to
grasp all the fiddly details, as it will simply stop the MathML code from
evolving. Please see the PDF printout (and compare with browsing the page) to
see the improvements that the patches bring in real life.
Attachment #194420 - Attachment is obsolete: true
Attachment #194647 - Flags: superreview?(roc)
Attachment #194647 - Flags: review?(roc)
Does this patch also fix the unusable square root symbols I see in Deer Park
(windows, if it matters), or is some other patch in your tree making the PDF
look good?
dveditz: that's probably you missing the necessary fonts. When I installed the
right fonts, those errors went away.
Comment on attachment 194647 [details] [diff] [review]
patch ready for review

+    fontstyle.AssignLiteral((1 == length) ? "italic" : "normal");

Don't do this. AssignLiteral must take a literal, and no other expression. You
can use AssignASCII instead.

rubber-stamped! I hope this fixes the nasty spacing problems I see in my build.
Attachment #194647 - Flags: superreview?(roc)
Attachment #194647 - Flags: superreview+
Attachment #194647 - Flags: review?(roc)
Attachment #194647 - Flags: review+
Checked in.

It does indeed fix those nasty spacing, which was compounded on Linux due to
invisible characters. They are explicitly filtered out in GfxWin, but not in
Linux, and so could be very bad there if a font returned a space for them
(instead of returning nothing). Now, the trick is to ignore them upfront,
directly from the MathML tokens, so that they don't reach the risky gfx.

This is a very useful fix. In your capacity as driver, is there any chance/time
that this might be considered for 1.5?!?
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
I'm not a branch driver but I would support you petitioning branch drivers to
land this.
Comment on attachment 194647 [details] [diff] [review]
patch ready for review

OK, let's see what happens... asking approval for the branch, this is a MathML
specific fix that has been much requested.
Attachment #194647 - Flags: approval1.8b5?
All the CSS does is to prepend CMR10 in the font-family of the <math> element,
and set CMMI10 as the font-family of <mi> when it says that it wants an italic
style.

dveditz, assuming you have the TeX fonts
(http://www.mozilla.org/projects/mathml/fonts/bakoma/texcm-ttf.zip), and update
your tree (now that all the patches have been checked in), and apply this CSS,
you should get what I got with PDFCreator
(http://sourceforge.net/projects/pdfcreator/) on my laptop booted with WinXP.
Ops... I clicked the wrong file. I meant to attach this simple diff.
Attachment #195090 - Attachment is obsolete: true
Nominating for 1.8b5 blocker to put on radar for 1.5b2.
Flags: blocking1.8b5?
Branch drivers: please note that the inter-spacing is a package. I am seeking
branch approval for all the pieces/bugs of the package as listed on the
dependency list. I have got positive feedback above the improvements on the trunk.
Depends on: 308045
RBS, it looks like 308045 isn't resolved yet. Does that need to happen first?
No, the order is not important. I can land this before going over to bug 308045.
Attachment #194647 - Flags: approval1.8b5? → approval1.8b5+
Flags: blocking1.8b5? → blocking1.8b5+
Checked in the 1.8 branch.
Keywords: fixed1.8
You need to log in before you can comment on or make changes to this bug.