Last Comment Bug 310680 - [BeOS]EnumFonts uses obsolete lossy conversion method
: [BeOS]EnumFonts uses obsolete lossy conversion method
Status: RESOLVED FIXED
: fixed1.8
Product: Core Graveyard
Classification: Graveyard
Component: GFX: BeOS (show other bugs)
: Trunk
: x86 BeOS
: -- normal (vote)
: ---
Assigned To: Sergei Dolgov
: QA timeless
Mentors:
Depends on:
Blocks: 266252
  Show dependency treegraph
 
Reported: 2005-10-01 07:07 PDT by Sergei Dolgov
Modified: 2009-01-22 10:17 PST (History)
3 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (2.93 KB, patch)
2005-10-01 07:28 PDT, Sergei Dolgov
no flags Details | Diff | Splinter Review
patch (2.99 KB, patch)
2005-10-01 09:05 PDT, Sergei Dolgov
no flags Details | Diff | Splinter Review
patch (3.01 KB, patch)
2005-10-01 10:18 PDT, Sergei Dolgov
thesuckiestemail: review+
mscott: approval1.8b5+
Details | Diff | Splinter Review

Description Sergei Dolgov 2005-10-01 07:07:50 PDT
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 2005-10-01 07:28:55 PDT
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 tqh 2005-10-01 07:49:39 PDT
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 2005-10-01 09:03:40 PDT
Comment on attachment 198132 [details] [diff] [review]
patch

taking timeless notuce about null-pointer check after Realloc
Comment 4 Sergei Dolgov 2005-10-01 09:05:54 PDT
Created attachment 198136 [details] [diff] [review]
patch

adding NS_ENSURE_TRUE(array, NS_ERROR_OUT_OF_MEMORY) after realloc
Comment 5 Sergei Dolgov 2005-10-01 10:17:23 PDT
Comment on attachment 198136 [details] [diff] [review]
patch

take tqh notice about breaking loop in HaveFontFor()
Comment 6 Sergei Dolgov 2005-10-01 10:18:48 PDT
Created attachment 198144 [details] [diff] [review]
patch

same as before, but loop in HaveFontFor() breaks at first matched font.
Comment 7 tqh 2005-10-01 10:21:26 PDT
Comment on attachment 198144 [details] [diff] [review]
patch

r=thesuckiestemail@yahoo.se

Looks ok to me.
Comment 8 Sergei Dolgov 2005-10-01 10:28:41 PDT
landed:
new revision: 1.42; previous revision: 1.41 
Comment 9 Niels Reedijk 2005-10-01 13:19:24 PDT
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) 2005-10-01 17:19:26 PDT
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 2005-10-01 17:52:35 PDT
Comment on attachment 198144 [details] [diff] [review]
patch

appproving this beos only change. Please land ASAP.
Comment 12 Blake Kaplan (:mrbkap) 2005-10-03 23:35:00 PDT
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 2006-12-27 00:22:59 PST
Comment on attachment 198136 [details] [diff] [review]
patch

removing old review req.

Note You need to log in before you can comment on or make changes to this bug.