GetTextCharset is unreliable for bitmap fonts on NT3.51

RESOLVED WONTFIX

Status

Core Graveyard
GFX: Win32
RESOLVED WONTFIX
14 years ago
3 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: neil@parkwaycc.co.uk)

Tracking

Trunk
x86
Windows NT

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

14 years ago
I've found an ANSI font for which GetTextCharset returns OEM_CHARSET. Since
GetTextMetrics returns a .tmCharSet of ANSI_CHARSET for the same font, I suggest
that we switch to using GetTextMetrics instead.
(Assignee)

Comment 1

14 years ago
Created attachment 144656 [details] [diff] [review]
Proposed patch
Assignee: win32 → neil.parkwaycc.co.uk
Status: NEW → ASSIGNED
(Assignee)

Updated

14 years ago
Attachment #144656 - Flags: superreview?(rbs)
Attachment #144656 - Flags: review?(ere)

Comment 2

14 years ago
Comment on attachment 144656 [details] [diff] [review]
Proposed patch

>+    GetTextMetrics(aDC, &tm);
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/gdi/fontext_6r
lf.asp
Return Values

If the function succeeds, the return value is nonzero.

If the function fails, the return value is zero. 

Please check the return value (i know lots of our code doesn't, that's not a
good excuse)
(Assignee)

Comment 3

14 years ago
Actually, that's somewhat unlikely... the function called another function that
passed a GetTextMetrics call shortly before arriving here. I can investigate
folding the two GetTextMetrics calls together if you like.
Comment on attachment 144656 [details] [diff] [review]
Proposed patch

You don't need the & (~0xFF)) check anymore? 

thanks for making this patch.
(Assignee)

Comment 5

14 years ago
(In reply to comment #4)
>(From update of attachment 144656 [details] [diff] [review])
>You don't need the & (~0xFF)) check anymore? 
No, metrics.tmCharSet is a BYTE.
(Assignee)

Comment 6

14 years ago
Created attachment 144744 [details] [diff] [review]
Alternative

This takes the call to GetTextMetrics out of GetNAME and into the caller...

Comment 7

14 years ago
I cannot vouch for this patch based on a single example, and no other history on
the matter.

Why, then, should GetTextMetrics() be more trustworthy than GetTextCharset() in
general? Because of that single font? What if there is another font which causes
GetTextMetrics() to be wrong and GetTextCharset() to be correct. There are cases
of badly designed fonts, and your font could be one. Since there are tons of
legacy code out there that depend on GetTextCharset(), solid evidence/history
(e.g., a KB article) is needed to consider it unreliable.

BTW, your alternative is not equivalent. Suppose that the new GetNAME() fails
_latter_, for example on the line with buffer.EnsureElemCapacity(), the caller
will not have the information that it is "other error" anymore.
(Assignee)

Comment 8

14 years ago
Created attachment 144825 [details]
Font in questions

(In reply to comment #7)
>There are cases of badly designed fonts, and your font could be one.
I'd better complain to Microsoft for badly designing their font then?

(From update of attachment 144744 [details] [diff] [review])
The new GetNAME is only called for truetype fonts so it does not have to
distinguish the non-truetype "error" from the other error cases.

Comment 9

14 years ago
Comment on attachment 144656 [details] [diff] [review]
Proposed patch

I see no problem in using GetTextMetrics. The font is apparently ok as it works
fine in WinXP, but NT 3.51 has a problem with GetTextCharSet. r=ere
Attachment #144656 - Flags: review?(ere) → review+
(Assignee)

Comment 10

14 years ago
I was confused, because my WordPad does not list the font as a bitmap font.
Trust Microsoft to make my life difficult ;-)
(Assignee)

Updated

14 years ago
Summary: GetTextCharset is unreliable → GetTextCharset is unreliable for bitmap fonts on NT3.51

Comment 11

14 years ago
Comment on attachment 144656 [details] [diff] [review]
Proposed patch

Needs more info.
Attachment #144656 - Flags: superreview?(rbs)
(Assignee)

Comment 12

13 years ago
Created attachment 187124 [details] [diff] [review]
Alternative updated for bitrot
Attachment #144744 - Attachment is obsolete: true
Product: Core → Core Graveyard
This bug has been buried in the graveyard and has not been updated in over 5 years. It is probably safe to assume that it will never be fixed, so resolving as WONTFIX.

[Mass-change filter: graveyard-wontfix-2014-09-24]
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.