493428 - Mail/News Account Settings far too narrow on OS/2

Status: VERIFIED FIXED

(Core :: Graphics, defect)

Description

[Peter Weilbacher]

Attachment: screenshot

As noted in the mozilla.dev.ports.os2 newsgroup by Peter Brown, the Mail/News Account Settings panel in SeaMonkey is far too narrow on OS/2. This regressed some time between 20090422135858 and 20090514112020. 

In comm-central, the most likely change that may have caused this is bug 317659. Either the new size is generally wrong for OS/2 (maybe because the default system font just has a weird zero width), then we need to special-case the values in AccountManager.dtd. Or we do something wrong when determining the "ch" unit (that would point to a problem in gfxOS2Fonts::GetMetrics).

Comment 1

[Peter Weilbacher]

Need to look at GetMetrics() anyway for bug 490561.

Comment 2

[Robert Kaiser]

I'm pretty sure this is a problem in determining to ch unit, and 1ch should be some approximation of the average width of a character, which makes it ideal for dialog width specifications and that's why we're switching to that in many places.
Fixing it for account manager specifically would just defer the problem to the next dialog that is being switched to use the "correct" unit for widths, which is ch.

Comment 3

[Peter Weilbacher]

OK, the metrics for the 'ch' unit were implemented in bug 363706; 'ch' uses the zeroOrAveCharWidth member of the metrics structure. AFAICS it is correctly derived on OS/2. For the standard Workplace Sans font (of size 15px) I get e.g.

   emHeight=15.000000
   maxHeight=16.785000
   xHeight=8.220000
   aveCharWidth=5.865000
   zeroOrAveCharWidth=4.875000
   spaceWidth=3.000000

If I artificially increase zeroOrAveCharWidth to 1.5x its natural size (i.e. 7.312500 in the example above), I do get a reasonably sized panel.
Forcing zeroOrAveCharWidth to be at least aveCharWidth is not enough. I don't see any reason to have to do either.

Comment 4

[Peter Weilbacher]

I compared this to a SM build on Linux where the fonts involved are Vera and VeraBd. There, LockedFTFace::GetMetrics computes
  emHeight=15.000000
  maxHeight=18.000000
  xHeight=8.000000
  aveCharWidth=10.000000
  zeroOrAveCharWidth=10.000000
  spaceWidth=5.000000
The horizontal values are quite a bit larger than on OS/2. I'll try to install the same fonts on OS/2 to see what I get there.

Perhaps there is a mistake in the horizontal scaling from font units to pixels. (But then, why don't I see any problems with that in other text display?!).

Comment 5

[Peter Weilbacher]

On OS/2 I didn't manage to get the exact same pixel size when setting Vera as standard dialog font, but I get either this
  emHeight=16,216667
  maxHeight=18,625000
  xHeight=8,750000
  aveCharWidth=9,835938
  zeroOrAveCharWidth=9,601562
  spaceWidth=6,000000
or this
  emHeight=14,666667
  maxHeight=17,460938
  xHeight=8,203125
  aveCharWidth=9,221191
  zeroOrAveCharWidth=9,001465
  spaceWidth=5,000000

While this shows some minor differences, the dialog is wide enough. So the problem could really be with the Workplace Sans font, although I don't know what that could be. Perhaps I should try vice-versa to set up Workplace Sans on Linux to check that.

Comment 6

[Peter Weilbacher]

OK, I set up Workplace Sans 9 (pt? GNOME doesn't give units) as default Application font in the GNOME Appearance settings app under Linux. That resulted in 
  emHeight=15.000000
  maxHeight=18.000000
  xHeight=9.250000
  aveCharWidth=8.250000
  zeroOrAveCharWidth=8.250000
  spaceWidth=4.250000

So indeed the OS/2 values (comment 3) for this font come out a bit weird, and a lot narrower than on Linux.

Comment 7

[Peter Weilbacher]

OK, so the difference is that OS/2 uses the glyph's width (of '0') to determine zeroOrAveCharWidth. I think this is correct, because the CSS3 working draft says

   http://www.w3.org/TR/css3-values/#relative0
   ch: The width of the "0" (ZERO, U+0030) glyph found in the font
       for the font size used to render.

On Linux, the x_advance is used which is larger because it takes into account offsets between characters in a text layout, same for Windows with GetTextExtentPoint32W() and the implementation in gfxAtsuiFonts.

I could simply change the OS/2 implementation to use metrics.horiAdvance instead of metrics.width, but given the CSS spec I don't think that's correct.

Zack, any comments on that?

Comment 8

[Zack Weinberg (:zwol)]

The intended use of 'ch' is for sizing boxes containing fixed width text.  For instance, a box styled with { width:80ch; font-face:monospace } should be able to hold 80 columns of monospace characters.  Thus I'm inclined to say that one should use the advance width of '0' rather than the glyph width (so Linux/Win/Mac are correct and OS/2 is wrong).

David, what do you think?

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Yeah, it should be the advance width.

Comment 10

[Peter Weilbacher]

OK, then. I'll fix OS/2. Do you have influence on the CSS3 spec so that this can be clarified in their text?

Comment 11

[Peter Weilbacher]

(In reply to comment #8)
> The intended use of 'ch' is for sizing boxes containing fixed width text.

Hmm, one could use this to argue that using 'ch' to size dialogs (which on most platforms contain non-monospace text) is misusing that unit. ;-)

Comment 12

[Peter Weilbacher]

OK, I pushed the fix:
http://hg.mozilla.org/mozilla-central/rev/c088e97f24d4
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/5e59a7808ad1

Other similar changes may have to be done for the width calculation of other metrics, but I'll postpone that to bug 490561.

Comment 13

[Robert Kaiser]

(In reply to comment #11)
> Hmm, one could use this to argue that using 'ch' to size dialogs (which on most
> platforms contain non-monospace text) is misusing that unit. ;-)

That would bring the question up of what the correct use for that unit is - one would guess that the intended use is actually to make boxes wide enough to hold a certain amount of characters in that font.

Comment 14

[Peter Weilbacher]

But you can't guarantee that if the font is not monospaced. If you fill a box with "width: 50ch" with 50 'm' characters (which in many fonts is wider than '0') then it is pretty sure that the text overflows that box. If you fill the same box with 50 'l' chars, then it's very likely that the box is much wider than that text.

Comment 15

[Robert Kaiser]

Right, but you're talking about extreme cases, and I'm talking about the average casee. Nobody says that any text with 50 characters needs to fit into a 50ch box, but if a text fits into a 50ch box with some margin in one font, the chance is pretty high that it will fit in a 50ch box in other fonts. And I think that's the actual intention of that unit.

Comment 16

[Peter Weilbacher]

If that is indeed the intent (searching around I found nothing to support your point), then you are right that it is useful for dialog sizing. But then it would not be very useful on the web. Because there one wants to separate style from content and not resize boxes when the text changes.

I guess this discussion (and an even longer one in bug 317659) just shows that 'ch' is underspecified and the spec (css3-values) should be clarified. Both in terms of implementation and intended use of the unit.

Comment 17

[Robert Kaiser]

I don't have influence on any spec, I'm just an observer and project manager.
Not sure about Zack, but surely David actually is one of those people who can bring up clarification of the spec.

(In reply to comment #16)
> If that is indeed the intent (searching around I found nothing to support your
> point), then you are right that it is useful for dialog sizing. But then it
> would not be very useful on the web. Because there one wants to separate style
> from content and not resize boxes when the text changes.

I disagree that it's not useful, as I've done a lot of website designing where I wanted exactly a unit that does this and went with em even though it's a height and no width unit (confusingly, knowing where its name comes from) and therefore doesn't change with more narrow or wider fonts. I wonder what you see as the usefulness or intent of the unit at all if not sizing boxes to fit a number of average-size characters (there is no CSS for "width of the string 'Page Navigation' plus some extra margin). But this discussion drifts away from what the bug is/was about.

Comment 18

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I posted a comment about the spec:
http://lists.w3.org/Archives/Public/www-style/2009Jul/0018.html

Comment 19

[Tony Chung [:tchung]]

Can someone that has OS/2 here verify this fix on trunk and a 1.9.1 nightly? 
Please change the status and keyword to verified also.   Thanks.

Comment 20

[Peter Weilbacher]

Maybe we should always just go to verified directly for OS/2 bugs to keep QA from finding them. (In this case, Steve now verified in the newsgroup that this is fixed with Workplace Sans 0.4.)

David, thanks for raising the point on the W3 list. :-)