Closed Bug 310680 Opened 16 years ago Closed 16 years ago

[BeOS]EnumFonts uses obsolete lossy conversion method

Categories

(Core Graveyard :: GFX: BeOS, defect)

x86
BeOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: sergei_d, Assigned: sergei_d)

References

Details

(Keywords: fixed1.8)

Attachments

(1 file, 2 obsolete files)

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
Attached patch patch (obsolete) — Splinter Review
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.
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)
Comment on attachment 198132 [details] [diff] [review]
patch

taking timeless notuce about null-pointer check after Realloc
Attachment #198132 - Attachment is obsolete: true
Attached patch patch (obsolete) — Splinter Review
adding NS_ENSURE_TRUE(array, NS_ERROR_OUT_OF_MEMORY) after realloc
Attachment #198136 - Flags: review?(thesuckiestemail)
Assignee: beos → sergei_d
Comment on attachment 198136 [details] [diff] [review]
patch

take tqh notice about breaking loop in HaveFontFor()
Attachment #198136 - Attachment is obsolete: true
Attached patch patchSplinter Review
same as before, but loop in HaveFontFor() breaks at first matched font.
Attachment #198144 - Flags: review?(thesuckiestemail)
Comment on attachment 198144 [details] [diff] [review]
patch

r=thesuckiestemail@yahoo.se

Looks ok to me.
Attachment #198144 - Flags: review?(thesuckiestemail) → review+
Blocks: 266252
landed:
new revision: 1.42; previous revision: 1.41 
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
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 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
Keywords: fixed1.8
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.