[BeOS]EnumFonts uses obsolete lossy conversion method

RESOLVED FIXED

Status

Core Graveyard
GFX: BeOS
RESOLVED FIXED
12 years ago
9 years ago

People

(Reporter: Sergei Dolgov, Assigned: Sergei Dolgov)

Tracking

({fixed1.8})

Trunk
x86
BeOS
fixed1.8

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

12 years ago
Somewhat folloup for Bug 200589.
http://lxr.mozilla.org/seamonkey/source/gfx/src/beos/nsFontMetricsBeOS.cpp#568
AssignWithConversion don't look as best method for totally UTF-8 system.
Also we allocate there array for all existing in BeOS fonts, but actual array
may consist even from single font family matching language and type criteria
(Assignee)

Comment 1

12 years ago
Created attachment 198132 [details] [diff] [review]
patch

Uses NS_ConvertUTF8toUTF16(),
reallocs fontname array to real size,
also implements  nsFontEnumeratorBeOS::HaveFontFor() as it uses similar code to
EnumFonts().
P.S. It seems last one is used only for automated fontpackage download, but I
never saw this method in action.

Comment 2

12 years ago
IMHO I think the code would be much nicer by using 
for()
{
  if(wrong case)
    continue;

  do_right thing();
}
(The nested if's in the first block looks a bit ugly as well)
(Assignee)

Comment 3

12 years ago
Comment on attachment 198132 [details] [diff] [review]
patch

taking timeless notuce about null-pointer check after Realloc
Attachment #198132 - Attachment is obsolete: true
(Assignee)

Comment 4

12 years ago
Created attachment 198136 [details] [diff] [review]
patch

adding NS_ENSURE_TRUE(array, NS_ERROR_OUT_OF_MEMORY) after realloc
Attachment #198136 - Flags: review?(thesuckiestemail)
(Assignee)

Updated

12 years ago
Assignee: beos → sergei_d
(Assignee)

Comment 5

12 years ago
Comment on attachment 198136 [details] [diff] [review]
patch

take tqh notice about breaking loop in HaveFontFor()
Attachment #198136 - Attachment is obsolete: true
(Assignee)

Comment 6

12 years ago
Created attachment 198144 [details] [diff] [review]
patch

same as before, but loop in HaveFontFor() breaks at first matched font.
Attachment #198144 - Flags: review?(thesuckiestemail)

Comment 7

12 years ago
Comment on attachment 198144 [details] [diff] [review]
patch

r=thesuckiestemail@yahoo.se

Looks ok to me.
Attachment #198144 - Flags: review?(thesuckiestemail) → review+

Updated

12 years ago
Blocks: 266252
(Assignee)

Comment 8

12 years ago
landed:
new revision: 1.42; previous revision: 1.41 
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Comment 9

12 years ago
Comment on attachment 198144 [details] [diff] [review]
patch

BeOS-only fix. Won't affect other platforms. Requesting approval for
MOZILLA_1_8_BRANCH
Attachment #198144 - Flags: approval1.8b5?
Comment on attachment 198132 [details] [diff] [review]
patch

+	   if (!(array[j] = ToNewUnicode(NS_ConvertUTF8toUTF16((const
char*)family))))


maybe: UTF8ToNewUnicode(nsDependentCString(family)) ?

Comment 11

12 years ago
Comment on attachment 198144 [details] [diff] [review]
patch

appproving this beos only change. Please land ASAP.
Attachment #198144 - Flags: approval1.8b5? → approval1.8b5+
Checking in gfx/src/beos/nsFontMetricsBeOS.cpp;
/cvsroot/mozilla/gfx/src/beos/nsFontMetricsBeOS.cpp,v  <--  nsFontMetricsBeOS.cpp
new revision: 1.40.12.2; previous revision: 1.40.12.1
done

Updated

12 years ago
Keywords: fixed1.8

Comment 13

11 years ago
Comment on attachment 198136 [details] [diff] [review]
patch

removing old review req.
Attachment #198136 - Flags: review?(thesuckiestemail)
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.