310680 - [BeOS]EnumFonts uses obsolete lossy conversion method

Status: RESOLVED FIXED

(Core Graveyard :: GFX: BeOS, defect)

Description

[Sergei Dolgov]

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

Comment 1

[Sergei Dolgov]

Attachment: 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

[tqh]

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 3

[Sergei Dolgov]

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

taking timeless notuce about null-pointer check after Realloc

Comment 4

[Sergei Dolgov]

Attachment: patch

adding NS_ENSURE_TRUE(array, NS_ERROR_OUT_OF_MEMORY) after realloc

Comment 5

[Sergei Dolgov]

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

take tqh notice about breaking loop in HaveFontFor()

Comment 6

[Sergei Dolgov]

Attachment: patch

same as before, but loop in HaveFontFor() breaks at first matched font.

Comment 7

[tqh]

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

r=[e-mail]

Looks ok to me.

Comment 8

[Sergei Dolgov]

landed:
new revision: 1.42; previous revision: 1.41 

Comment 9

[Niels Reedijk]

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

BeOS-only fix. Won't affect other platforms. Requesting approval for
MOZILLA_1_8_BRANCH

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

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

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


maybe: UTF8ToNewUnicode(nsDependentCString(family)) ?

Comment 11

[Scott MacGregor]

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

appproving this beos only change. Please land ASAP.

Comment 12

[Blake Kaplan (:mrbkap) (inactive)]

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

Comment 13

[tqh]

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

removing old review req.