Mac text layout has bad spacing at style changes

VERIFIED FIXED in M16

Status

()

Core
Layout
P3
blocker
VERIFIED FIXED
18 years ago
18 years ago

People

(Reporter: Simon Fraser, Assigned: Simon Fraser)

Tracking

({platform-parity})

Trunk
PowerPC
Mac System 8.5
platform-parity
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta2+][dogfood-] fix awaiting review, URL)

Attachments

(5 attachments)

(Assignee)

Description

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

Comment 1

18 years ago
Created attachment 8852 [details]
Screen shot of bad text layout
(Assignee)

Comment 2

18 years ago
Nominating for beta2.
Keywords: nsbeta2
(Assignee)

Comment 3

18 years ago
See also bug 20211, which is similar but only applies to postscript fonts.
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.

Updated

18 years ago
Target Milestone: --- → M17
adding self to cc list.

Comment 6

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

Comment 7

18 years ago
This is Franks. Thanks
Assignee: dcone → ftang

Comment 8

18 years ago
Is this caused by the same problem of 36417 ?
Status: NEW → ASSIGNED

Comment 9

18 years ago
reassign to garywade@desisoftsystems.com
Assignee: ftang → garywade
Status: ASSIGNED → NEW
(Assignee)

Comment 10

18 years ago
bug 38152 may be the same as this.

Comment 11

18 years ago
Putting on [nsbeta2-] radar.  
Whiteboard: [nsbeta2-]
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.
Keywords: dogfood
Whiteboard: [nsbeta2-]
(Assignee)

Comment 13

18 years ago
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?
Keywords: pp
if this were win32, this would be a beta stopper no questions asked.
Severity: normal → major

Comment 15

18 years ago
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.
Whiteboard: [nsbeta2+][6/15]
Created attachment 9145 [details]
Better view of really bad layout
Created attachment 9146 [details]
more badness on another page

Comment 18

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

Comment 19

18 years ago
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]".
Assignee: garywade → pierre
Severity: major → blocker
Target Milestone: M17 → M16

Updated

18 years ago
Status: NEW → ASSIGNED
Whiteboard: [nsbeta2+][6/15]

Comment 20

18 years ago
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?
(Assignee)

Comment 21

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

Comment 22

18 years ago
Fixed. Bad port font maintenance broke text measurement.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 23

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

Status: RESOLVED → VERIFIED

Comment 24

18 years ago
Created attachment 9343 [details]
Still broken for me

Comment 25

18 years ago
Running 2000053011. Go to Preferences, set the font to Georgia. Now go to the About box.
(Assignee)

Comment 26

18 years ago
Yeah, I see that too. I think that the © entity is messing up text 
measurement.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 27

18 years ago
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

18 years ago
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

18 years ago
Sent back to Simon: the bug and the first part of the fix was his.
Assignee: pierre → sfraser
Status: REOPENED → NEW

Comment 30

18 years ago
Created attachment 9351 [details]
Testcase from Simon for those behind the firewall

Comment 31

18 years ago
Putting on [nsbeta2+][dogfood-] radar.  Does not need a fix ASAP for daily work, 
but we should fix this for beta2.
Whiteboard: [nsbeta2+][dogfood-]
(Assignee)

Comment 32

18 years ago
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).
(Assignee)

Comment 33

18 years ago
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"))
the patch looks fine to me, tested and running with it with no problems so far.
Does it work with the logical resolution set to a value other than 96 ppi?
(Assignee)

Comment 36

18 years ago
Yes. I tried 72, and it worked fine.
(Assignee)

Updated

18 years ago
Status: NEW → ASSIGNED
Whiteboard: [nsbeta2+][dogfood-] → [nsbeta2+][dogfood-] fix awaiting review
(Assignee)

Comment 37

18 years ago
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → FIXED

Comment 38

18 years ago
Fixed in the June 2nd Mac Build.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.