39811 - Mac text layout has bad spacing at style changes

Status: VERIFIED FIXED

(Core :: Layout, defect, P3)

Description

[Simon Fraser [no longer active]]

Text layout on Mac is currently hosed, in that it adds lots of horizonatal space, 
or has too little space, at style changes.

Comment 1

[Simon Fraser [no longer active]]

Attachment: Screen shot of bad text layout

Comment 2

[Simon Fraser [no longer active]]

Nominating for beta2.

Comment 3

[Simon Fraser [no longer active]]

See also bug 20211, which is similar but only applies to postscript fonts.

Comment 4

[Henri Sivonen (:hsivonen)]

The horizontal space Mozilla reserves for text does not change with the computed font 

size when logical resolution is changed.



On a related note: Mac OS also lacks an implemtation of GetBoundingMetrics() for 

MathML.

Comment 5

[Mike Pinkerton (not reading bugmail)]

adding self to cc list.

Comment 6

[Erik van der Poel]

Frank, please take a look at this. Do you know something about this bug?

Comment 7

[dcone (gone)]

This is Franks. Thanks

Comment 8

[Frank Tang]

Is this caused by the same problem of 36417 ?

Comment 9

[Frank Tang]

reassign to [e-mail]

Comment 10

[Simon Fraser [no longer active]]

bug 38152 may be the same as this.

Comment 11

[leger]

Putting on [nsbeta2-] radar.  

Comment 12

[Mike Pinkerton (not reading bugmail)]

whoa, have you _seen_ this? it's not just obvious and embarrasing but it makes it 
very hard to read the text on a page. i'd say this was even dogfood. removing for 
re-evaluation.

Comment 13

[Simon Fraser [no longer active]]

I agree with pink. The Mac build sucks ass enough already without being made even 
more embarrassing by obvious text layout problems.

Gary, what's up with a fix for this?

Comment 14

[Mike Pinkerton (not reading bugmail)]

if this were win32, this would be a beta stopper no questions asked.

Comment 15

[leger]

Putting on [nsbeta2+][6/15] radar to fix the additional spaces.  If problem is 
worse than attachment shown, please add a new one and resubmit.

Comment 16

[Mike Pinkerton (not reading bugmail)]

Attachment: Better view of really bad layout

Comment 17

[Mike Pinkerton (not reading bugmail)]

Attachment: more badness on another page

Comment 18

[Pierre Saslawsky]

*** Bug 38152 has been marked as a duplicate of this bug. ***

Comment 19

[Pierre Saslawsky]

Marked blocker: it screws up the rendering on too many pages and affects the 
verifications I can do for other style & layout bugs.

Examples of pages where the layout is obviously messed up are:
- http://www.w3.org/TR/html4/
- http://www.w3.org/MarkUp/
- http://www.w3.org/
- Viewer Demo #5 (look at the 1st line)
- Viewer Demo #0
- http://home.netscape.com/index1.html (look at the links under "Today's 
Features...")

Reassigned to myself and removed the "[nsbeta2+][6/15]" tag per leger's request. 
It should be changed to "[nsbeta2+][ASAP]".

Comment 20

[buster]

Robert OCallahan attached a patch to bug 36417 that has something to do with Mac 
font measurements.  Could these be related?  Maybe even the same problem?

Comment 21

[Simon Fraser [no longer active]]

Adding a testcase in the URL. There are two causes to this, and I have a fix for 
one (in nsGfxUtils.h:70).

Comment 22

[Simon Fraser [no longer active]]

Fixed. Bad port font maintenance broke text measurement.

Comment 23

[Chris Petersen]

Excellent, this is fixed ! Tested with the May 29th Mac build (2000052920).

Comment 24

[Avi Drissman]

Attachment: Still broken for me

Comment 25

[Avi Drissman]

Running 2000053011. Go to Preferences, set the font to Georgia. Now go to the About box.

Comment 26

[Simon Fraser [no longer active]]

Yeah, I see that too. I think that the © entity is messing up text 
measurement.

Comment 27

[Simon Fraser [no longer active]]

I updated the test case to show the problem. You should set your Serif font to 
something wacky to see the problem. I'm also seeing that only the first text run 
on a line is drawing with the correct font; the rest seems to use Times. Eh?

Comment 28

[Avi Drissman]

I can see the font changes too. As for your test case, I can't see your "jazz" machine so I can't comment.

Comment 29

[Pierre Saslawsky]

Sent back to Simon: the bug and the first part of the fix was his.

Comment 30

[Pierre Saslawsky]

Attachment: Testcase from Simon for those behind the firewall

Comment 31

[leger]

Putting on [nsbeta2+][dogfood-] radar.  Does not need a fix ASAP for daily work, 
but we should fix this for beta2.

Comment 32

[Simon Fraser [no longer active]]

So it turns out that the Mac GFX code is deficient in that it fails to look at 
font prefs when mapping generic fonts to platform fonts, at least when drawing 
non-unicode text. The Unicode case is handled properly (which is why this works 
when the string being drawn contains 8-bit ASCII).

Comment 33

[Simon Fraser [no longer active]]

ftang: can you review this patch? I'm making use of nsUnicodeMappingUtil, which 
already contains a lookup table of generic family -> font, taking prefs into 
account. I'm always calling it for the smRoman script, since we don't have a 
lanuage group here, and this code is only called when drawing char*s (assumed to 
be US-ASCII).

Index: nsFontMetricsMac.cpp
===================================================================
RCS file: /cvsroot/mozilla/gfx/src/mac/nsFontMetricsMac.cpp,v
retrieving revision 1.51
diff -w -c -1 -r1.51 nsFontMetricsMac.cpp
*** nsFontMetricsMac.cpp	2000/05/30 17:53:20	1.51
--- nsFontMetricsMac.cpp	2000/05/31 05:05:32
***************
*** 27,28 ****
--- 27,29 ----
  #include "nsUnicodeFontMappingMac.h"
+ #include "nsUnicodeMappingUtil.h"
  #include "nsGfxUtils.h"
***************
*** 115,117 ****
--- 116,131 ----
    // the CSS generic names (conversions from the old Mac Mozilla code for now)
+   nsUnicodeMappingUtil* unicodeMappingUtil = 
nsUnicodeMappingUtil::GetSingleton();
+   if (unicodeMappingUtil)
+   {
+     nsString*   foundFont = unicodeMappingUtil->GenericFontNameForScript(
+           smRoman,      // should we be looking at a language-group here?
+           unicodeMappingUtil->MapGenericFontNameType(aGenericFamily));
+     if (foundFont)
+     {
+       aFontFace = *foundFont;
+       return;
+     }
+   }
    
+   // fall back onto hard-coded font names
    if (aGenericFamily.EqualsIgnoreCase("serif"))

Comment 34

[Mike Pinkerton (not reading bugmail)]

the patch looks fine to me, tested and running with it with no problems so far.

Comment 35

[Henri Sivonen (:hsivonen)]

Does it work with the logical resolution set to a value other than 96 ppi?

Comment 36

[Simon Fraser [no longer active]]

Yes. I tried 72, and it worked fine.

Comment 37

[Simon Fraser [no longer active]]

Fix checked in.

Comment 38

[Chris Petersen]

Fixed in the June 2nd Mac Build.