Mozilla's computed x-heights don't match real font x-heights

VERIFIED FIXED

Status

()

Core
CSS Parsing and Computation
VERIFIED FIXED
16 years ago
16 years ago

People

(Reporter: Greg K., Assigned: rbs)

Tracking

(Blocks: 1 bug, {css2, testcase})

Trunk
PowerPC
All
css2, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(6 attachments)

(Reporter)

Description

16 years ago
When an "x" character glyph is displayed next to an image whose height is
defined to be 1ex in CSS, the height of the x glyph and the image aren't the same.
(Reporter)

Updated

16 years ago
Blocks: 153699
(Reporter)

Comment 1

16 years ago
Created attachment 88990 [details]
Minimal testcase
(Reporter)

Updated

16 years ago
Keywords: css2, testcase
(Reporter)

Comment 2

16 years ago
See also bug 18814. This may just be broken on Mac.
This is a dup of the bug to implement getBoundingMetrics on Mac (or something).
(Reporter)

Comment 4

16 years ago
Bug 74821?
(Reporter)

Comment 5

16 years ago
I think this should stay open, but depend on that bug.
Depends on: 74821
(Assignee)

Comment 6

16 years ago
Taking. This is needed for nicer MathML alignments.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
OS: MacOS X → All
(Assignee)

Comment 7

16 years ago
--> meant to re-assign.
Assignee: dbaron → rbs
Status: ASSIGNED → NEW
Created attachment 91720 [details]
testcase showing multiple fonts

Note that the width of the images makes it easy to see whether we're varying
the definition by font.  (We currently are on Linux.)
(Assignee)

Comment 9

16 years ago
Created attachment 91775 [details] [diff] [review]
patch - tested

Does this do the trick? I read the documentation at:
http://developer.apple.com/technotes/te/te_21.html#Section8
http://www.mactech.com/articles/develop/issue_09/Ressler_article.html

[BTW, there seems to be a difference of GfxMac vs. other platforms. It seems
that the Mac gets its CSS metrics (e.g., emheight, maxAscent, maxDescent, etc)
from the first font, instead of the first ASCII font. So if somebody uses
"font-family: Symbol, Times", the metrics might come from Symbol. That isn't
the case on other platforms where the CSS metrics are taken from the first
ASCII font. Also, I am not sure if the CSS ordering (i.e., font switching) is
totally honored when hunting for glyphs. There seems to be some voodoo going on
w.r.t. the 'script' business and since I can't test, I cannot confirm / deny
what I am alluding.]
(Reporter)

Updated

16 years ago
Keywords: review

Comment 10

16 years ago
roger, your patch works correctly for me.
(Assignee)

Comment 11

16 years ago
dbaron/peterv/sfraser, either of you care to r/sr based on schofield's testing?
http://bugzilla.mozilla.org/attachment.cgi?id=91775&action=edit

I will be driving his patch for bug 107146 and closing this little gem will 
clear the way (and my tree...). Also, the API that I used is the same that he is 
using over there.
Status: NEW → ASSIGNED
(Reporter)

Comment 12

16 years ago
rbs@maths.uq.edu.au, are you saying that bug 107146 depends on this bug being fixed?
(Assignee)

Comment 13

16 years ago
No. I am saying that I am the one who will apply and checkin the bigger patch
for the other bug -- and it would be convenient if my tree is clean, and that it
isn't bad to checkin a little snippet that is using the measuring API that is
being used for the other bigger patch.
(Assignee)

Updated

16 years ago
Attachment #91775 - Attachment description: tentative patch - untested → patch - tested

Comment 14

16 years ago
Comment on attachment 91775 [details] [diff] [review]
patch - tested

r=ftang
Attachment #91775 - Flags: review+

Comment 15

16 years ago
Comment on attachment 91775 [details] [diff] [review]
patch - tested

sr=sfraser
Attachment #91775 - Flags: superreview+

Comment 16

16 years ago
Comment on attachment 91775 [details] [diff] [review]
patch - tested

a=asa (on behalf of drivers) for checkin to 1.1
Attachment #91775 - Flags: approval+
(Assignee)

Comment 17

16 years ago
Checked in -- When I hit enter, I realised that I forgot to add the a= on the
checkin comment...
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
(Reporter)

Comment 18

16 years ago
This does appear to be fixed in FizzillaCFM/2002080203. However, there does
appear to be occasional discrepancy due to text antialiasing. I'll attach a
screenshot of testcase 2.
(Reporter)

Comment 19

16 years ago
Created attachment 93723 [details]
Screenshot of testcase 2 in FizzillaCFM/2002080203

Note that, in examples 4 and 5, the 5em x is 1px taller than the img.
(Assignee)

Comment 20

16 years ago
Do you have a comparative screenshot using an earlier build without the patch?
(Reporter)

Comment 21

16 years ago
Created attachment 93782 [details]
Screenshot in Moz 1.1b, before patch

Sure, here's one. This shows how bad it was before your patch.
(Reporter)

Comment 22

16 years ago
Created attachment 93786 [details]
New testcase with size change capability

Use this testcase to observe what I'm talking about. The difference is most
apparent at even px sizes between about 74 and 90. This may not be a problem
with your patch, or even with Mozilla, but could be related to the text
antialiasing. Or, it could be a rounding error in Mozilla, who knows.

Comment 23

16 years ago
verified
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.