189353 - BeOS Menu Font does not use OS-wide menu font size preference

Status: RESOLVED FIXED

(Core Graveyard :: GFX: BeOS, defect)

Description

[Simon]

User-Agent:       Mozilla/5.0 (BeOS; U; BeOS BePC; en-US; rv:1.3a) Gecko/20021204
Build Identifier: Mozilla/5.0 (BeOS; U; BeOS BePC; en-US; rv:1.3a) Gecko/20021204

The font used for the mozilla menu bar and context menus is the same as the font
specified in Be->preferences->menu. However, the font size is not obeyed.

Reproducible: Always

Steps to Reproduce:
1. Load Mozilla
2. Go to Be->Preferences->Menu and change the font size
3. Close and restart mozilla

Actual Results:  
Menu font size did not change

Expected Results:  
Menu font size changes to reflect setting in Menu prefs. Test this expected
behaviour with a Be-native app.

Hopefully pretty easy to fix this one, but I still haven't got a
moz-building-capable computer so I can't try to create a patch myself. Still,
browsing the source online has found what I hope will fix it.

I think the problem is in nsDeviceContextBeOS. mMenuFont is set to have the font
family and style of the user-preference menu font, but the size is never specified. 

From nsDeviceContextBeOS.cpp:
155   menu_info info;
156   get_menu_info(&info);
157   mMenuFont.SetFamilyAndStyle(info.f_family,info.f_style);

the probable fix is to add this one line:
mMenuFont.SetSize(info.font_size);

Could someone (paul?) try this out and get it submitted if it works?

Comment 1

[Sergei Dolgov]

Unfortunately i can't test this problem. My chrome is broken for long time,
perhaps something with setting (this RDF thing reminds me sometimes Windows
registry nightmare),
so i don't see menus using menu font at all, and i cannot change themes. 
Btw, half-offtopic - you can ask on BeShare Kancept and/or Zaranthos if they can
provide you with additional harddisk in order to do work on BeZilla. Especially
if you live in US or Canada.

Comment 2

[Sergei Dolgov]

Simon, adding font size in such way in nsDeviceContext*.cpp doesn't help. Size
setting is performing in nsFontMetricsBeOS.cpp in Init() method. Taken from
nsFont properties. And i think maybe better idea is allow Mozilla to handle menu
and other widgets font size itself, as it is set in css/xul. At least it needs
further investigation

Comment 3

[Simon]

OK then.

I did a test, closing mozilla and changing the BeOS menu font. When I reopened
the browser, the menu FONT (typeface) was still the same, however the SIZE was
different. Non-expected behaviour?

All skins seem to use the same menu font, so why can't we load the BeOS-wide
font. Obviously this was done at some point, from the code I copied about
get_menu_font.

Ps: Got my new hardware now, but the HDD is missing a pin on the interface - it
works, but no DMA. I'll have to send it back and wait another 2 weeks for a
replacement.

Comment 4

[Ryo Hattori]

Attachment: Enable OS-wide font setting

Comment 5

[Sergei Dolgov]

I'm ready to put review here, only problem is what is general rule for Mozilla -
should it use system-wide preferences or not for menu font sizes, or it depends
on theme?
Maybe timeless knows answer?

Ryo, i will send you today duplicates of my mails to koki, as my sources for
gfx/beos, because it seems you still didn't get it.

Comment 6

[Sergei Dolgov]

adding Paul

Comment 7

[Simon]

All the themes I've tried have the same menu font, and changing the menu font 
in the system prefs does alter how mozilla's menu is displayed (size seems to 
vary). So there is a bug - if the font is set in CSS then it's reading it 
wrongly (or getting the size from somewhere system-wide), if the font is meant 
to be the system one, it's not loading it properly.

I don't know about how the other platforms get the menu font, but I think it 
must be the system-wide one. Moz's window looks very strange next to other ones 
with a native BMenu. Also, as there is no way for the user to change the menu 
font inside moz, it becomes an accessibility issue as well.

My vote is to get the system menu font.

ps: Replacement HD FINALLY arrived, so I've got BeOS running on a 20gig 
partition now. I may try d/l a tar and doing a build of moz at some point. (56k 
so that may take some time)

Comment 8

[Sergei Dolgov]

at moment adding other style elements there, something like:
    theFont->GetFamilyAndStyle(&family, &style);   
 //   aFont->name.AssignWithConversion( family ); helping CJK people here, if
they have fonts with Japanese names as system-wide
    aFont->name.Assign(NS_ConvertUTF8toUCS2(family));
          
    theFont->GetHeight(&height); 
    aFont->size =
NSIntPixelsToTwips(uint32(height.ascent+height.descent+height.leading),
mPixelsToTwips); 
    if((theFont->Face() & B_BOLD_FACE))
    	aFont->weight = NS_FONT_WEIGHT_NORMAL + 1;
    else
    	aFont->weight == NS_FONT_WEIGHT_NORMAL;
    	
    if((theFont->Face() & B_ITALIC_FACE))
    	aFont->style |= NS_FONT_STYLE_ITALIC;
    if((theFont->Face() & B_STRIKEOUT_FACE))
    	aFont->decorations |= NS_FONT_DECORATION_LINE_THROUGH;
    if((theFont->Face() & B_UNDERSCORE_FACE))
    	aFont->decorations |= NS_FONT_DECORATION_UNDERLINE;

Comment 9

[Sergei Dolgov]

btw, experience was bad when i applied patch for menu font size given by Mozilla
in experimental builds - got lot of blame. Some people complained that it is too
big, some - too small. (it was about year ago or more)
Inspite i was totally satisfied with results myself 

Comment 10

[Sergei Dolgov]

Confirming.
I hope it will be fixed soon.

Comment 11

[tqh]

What's the status on this one?

Comment 12

[Sergei Dolgov]

I'm working at big font metrics patch which will include this one

Comment 13

[Sergei Dolgov]

Comment on attachment 121542 [details] [diff] [review]
Enable OS-wide font setting 

obsoleting.
New patch will follow with taking in account more system settings.
I decided that nsDeviceContext can be patched separately without affecting big
metrics patch

Comment 14

[Sergei Dolgov]

Attachment: patch

Adding more font parameters to transfer to nsFontMetricsBeOS::Init().
It may require some minor changes in future also in nsFontMetrics, but
shouldn't broke it current work.

Also fixing issue for non-set mWidget.

Adding estimation of DPI.

Allowing future use of non-roman font names (corresponding change will made
also in nsFontMetrics, there is already bug for that)

Comment 15

[tqh]

Comment on attachment 174882 [details] [diff] [review]
patch

I'm not very good at font-handling, maybe should have different reviewer.
Anyway there is some mixing of tabs and spaces, but otherwise it looks ok.

Comment 16

[Sergei Dolgov]

Checked in with stile fixes (tabs and spaces) and unification:
2005-02-20 13:05
"Bug 189353: BeOS Menu Font does not use OS-wide menu font size preference.
BeOS-only, no sr required. r=[e-mail]"
Marking fixed

Comment 17

[tqh]

Attachment: problem with BeZilla rendering

I think this page may render differently with this patch.

Comment 18

[tqh]

Attachment: Also a button problem

This is not because I have monospaced font to 12, but may be because of
HaikuFox-theme. I will check that. Never seen it before.

Comment 19

[tqh]

The button problem is not because of the theme.

Comment 20

[Sergei Dolgov]

per
https://bugzilla.mozilla.org/show_bug.cgi?id=189353#c18

set fonts and sizes for Unicode. livejournal uses UTF-8

Comment 21

[Simon]

The patch seems to have a bug - the value set in preferences is never used. The
calulation to guess a dpi should store the result in OSVal. The preferences
value should be the preferred one.

Correcting this allows the one-pixel scrolling errors to be fixed. The problem
appears when 1440/dpi is an even number - set dpi in preferences to 70 or 74 if
you like the BeOS default of 72 - 96 also works fine (windows default dpi).

The easiest way to set dpi is to filter for screen_res in about:config

Comment 22

[Sergei Dolgov]

yeah, calcuclation should be made fro OS-value, not PrefValue.

Comment 23

[Sergei Dolgov]

Attachment: patch

fixing confusing between osvalue and prefvalue

Comment 24

[Simon]

Comment on attachment 175715 [details] [diff] [review]
patch

Fine.

I'm not sure how accurate the estimate is, but it seems to be the best we can
do on BeOS.

r=[e-mail]

Comment 25

[Sergei Dolgov]

nsDeviceContextBeOS.cpp
new revision: 1.31; previous revision: 1.30
Hope now we can close bug

Comment 26

[Doug Shelton]

I've tested this here and have not had any problems.  Is there anything I can do
to help get this committed to CVS?

Comment 27

[tqh]

It's already in CVS as far as I can see. Closing bug.

Comment 28

[Sergei Dolgov]

Looks like some recent (probably rounding problem) fix didn't pass in branch.
Here menu font size is smaller in SM than in other OS menus. Will look for other bugs and checkins

Comment 29

[Doug Shelton]

looks like we still need to apply this to the branch.

Comment 30

[Doug Shelton]

(In reply to comment #29)
> looks like we still need to apply this to the branch.
> 
Or not.  Bug 335076 fixes DPI to 85.  This is also set in current branch code.  Not sure what would cause the SeaMonkey issue sergei describes in comment 28.