67840 - Missing/incomplete nsIFontEnumerator::HaveFontFor method

Status: RESOLVED FIXED

(Core :: Internationalization, defect, P1)

Description

[cls]

The bewildered tinderbox has been red since ftang's check ins this morning.  I
looked at the checkins and I noticed that a) there was no bug number for the
checkin, b) only windows has a complete implementation of the method and c) none
of the other toolkits (xlib, qt) even have the dummy implementation.  I'm still
waiting for OS/2 to go red.  Was it really necessary for this change to go in
this incomplete the morning that the tree is closing for a release?

Comment 1

[Daniel (Leaf) Nunes]

I have to echo chris's view here... a single sr= is not sufficient (r= and sr=
are what's specified), and neither is it ok to bust other platforms (especially
just prior to a tree closure).

What's the justification for not backing these changes out?

Comment 2

[Frank Tang]

Stub added to OS2 and BeOS

Comment 3

[Teruko Kobayashi]

Changed QA contact to [e-mail] since QA cannot verify this.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I just added the stub (copied from GTK) to the xlib port as well (after getting
approval from cls and blizzard on IRC).

Comment 5

[Roland Mainz]

Xprint (X11 print system; add --with-xprint to Linux|Solaris2.7/2.8 builds)
seems to suffer from the same problem (I filed bug 68441 for this) - can anyone
of the gurus here have a look into the problem, please ?

Shared library module libgfxxprt.so is rejected like this:
-- snip --
**************************************************
nsNativeComponentLoader:
SelfRegisterDll(/bigtmp/gisburn/mozilla-2001-02-10-08-Mtrunk/mozilla/objdir_ws6_xlib/dist/bin/components/libgfxxprt.so)
Load FAILED with error: ld.so.1: ./mozilla-bin: fatal: relocation error: file
/bigtmp/gisburn/mozilla-2001-02-10-08-Mtrunk/mozilla/objdir_ws6_xlib/dist/bin/components/libgfxxprt.so:
symbol __1cSnsFontEnumeratorXPLHaveFontFor6Mpkcpi_I_: referenced symbol not
found
**************************************************
-- snip --

% dem __1cSnsFontEnumeratorXPLHaveFontFor6Mpkcpi_I_ 
__1cSnsFontEnumeratorXPLHaveFontFor6Mpkcpi_I_ == unsigned
nsFontEnumeratorXP::HaveFontFor(const char*,int*)

Comment 6

[Roland Mainz]

Fixed tis issue myself... :-)
diff for
mozilla-2001-02-10-08-Mtrunk/mozilla/gfx/src/xprint/nsFontMetricsXP.cpp:
-- snip --
mozilla-2001-02-10-08-Mtrunk/mozilla/gfx/src/xprint% diff -c -w
nsFontMetricsXP.cpp.original nsFontMe
tricsXP.cpp
*** nsFontMetricsXP.cpp.original        Sun Feb 11 04:15:42 2001
--- nsFontMetricsXP.cpp Sun Feb 11 04:21:09 2001
***************
*** 2983,2985 ****
--- 2983,3002 ----
    // XXX still need to implement aLangGroup and aGeneric
    return EnumerateAllFonts(aCount, aResult);
  }
+ 
+ 
+ /* gisburn: copy&paste clone from nsFontEnumeratorXlib::HaveFontFor in
+  * mozilla/gfx/src/xlib/nsFontMetricsXlib.cpp
+  */
+ NS_IMETHODIMP
+ nsFontEnumeratorXP::HaveFontFor(const char* aLangGroup, PRBool* aResult)
+ {
+   NS_ENSURE_ARG_POINTER(aResult);
+   *aResult = PR_FALSE;
+   NS_ENSURE_ARG_POINTER(aLangGroup);
+ 
+   *aResult = PR_TRUE; // always return true for now.
+   // Finish me - ftang
+   return NS_OK;
+ }
+               
-- snip --

Can anyone check this in, please ?

Comment 7

[cls]

r=cls.  Still need a sr= and then an a= from [e-mail].  We need to
get this into the 0.8 branch too. 

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Maybe we should also fix photon and qt?  Then we should have all implementations
of nsIFontEnumerator fixed.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

BTW, is this really going to be implemented in the other ports?  If not, it
should probably not be in nsIFontEnumerator, so we don't have to write unused
code for all the other ports.

Reopening bug, since there are still 3 ports where the stub hasn't been checked in.

Comment 10

[Brendan Eich [:brendan]]

So is Windows the only platform with a non-stub impl?  How about we use #ifdefs
to provide a stub for all other platforms, in some XP file, instead of forcing a
long march through the ports?

Or are non-stub impls for other platforms really forthcoming, soon?  Frank, what
is the plan?

/be

Comment 11

[cls]

<Insert token comment about how non-XP checkins are A Bad Thing(tm).>

diff -u -r1.2 nsIFontEnumerator.idl
--- nsIFontEnumerator.idl       2001/02/06 12:48:44     1.2
+++ nsIFontEnumerator.idl       2001/02/16 10:13:31
@@ -47,9 +47,11 @@
    */
   void EnumerateFonts(in string aLangGroup, in string aGeneric,
     out PRUint32 aCount, [retval, array, size_is(aCount)] out wstring aResult);
+#ifdef XP_WIN32
   /*
     @param  aLangGroup language group
     @return bool do we have a font for this language group
    */
   void HaveFontFor(in string aLangGroup, [retval] out boolean aResult);
+#endif
 };

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

If it's only going to be used on Windows, there's no need for it to be in the
IDL at all, since the windows usage is from within the windows code where it's
used as an nsFontEnumeratorWin...

ftang:  Do you intend to implement this on other platforms, or is it only for
the Windows code?

Comment 13

[Frank Tang]

>ftang:  Do you intend to implement this on other platforms, or is it only for
the Windows code?

My answer is YES. I will implement the real code for Linux and Mac. And I will
call it from JavaScript from Pref.

Comment 14

[Frank Tang]

can you #ifdef in idl ?

Comment 15

[Frank Tang]

1. I really think adding #ifdef code in idl and these kind of API is bad2
2. I don't mind add stub for other port but I do not feel comfortable to check
in some code that I didn't compile in those platform. I can add them if mozilla
allow me to check in without compile on qt, phanton, etc. (I do build my stub on
Mac, Linux before I check in because I have these build environment in  my cube).

Brendan- what do you think? Which rule you want me to break? ignore ports, or
check in code which I didn't compile ? :)

Comment 16

[Brendan Eich [:brendan]]

ftang: earlier I had suggested using a combination of #ifdefs and stubs.  There
is no need to make an identical stub for every platform you or a platform
champion has not yet supported.  As for adding a stub that might not compile
(presumably you are worried about a typo), it can't be any worse than not having
the stub in the first place, and toasting a bunch of platforms.

/be

Comment 17

[Erik van der Poel]

I agree with Frank that it would be better to avoid ifdefs inside the IDL file,
and just have a stub in each platform implementation file. I don't think it
should be a requirement for Frank (or anybody else) to compile on all of the
platforms before checking in. Just copy the stub to all platforms, check in,
and watch tinderbox (for those platforms that have one).

An ifdef/stub combo might be possible too, but I personally don't think it's
worth the trouble.

Comment 18

[Brendan Eich [:brendan]]

Just to re-iterate:

Erik's right, how to stub and share stubs is up to the person who adds the
stubs, and http://tinderbox.mozilla.org/showbuilds.cgi?tree=SeaMonkey-Ports is
there to keep an eye on, and always has been.  There is no rule that you have to
compile your code on all ports platforms *before* checking it in.

/be

Comment 19

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: the rest (I think) of the necessary stubs

Comment 20

[Erik van der Poel]

sr=erik for attachment id 25575

David, it's great that you're doing this, but Frank, maybe it would have been
nice if you did this in the first place?

Comment 21

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

ftang - Any interest in reviewing this and checking it in?  I agree that I
probably shouldn't be the one doing this.  It also should have been done two
weeks ago, since this is causing build bustage for a number of people and
forcing them to each fix it on their own.

Comment 22

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

*** Bug 69465 has been marked as a duplicate of this bug. ***

Comment 23

[Frank Tang]

check in fix for qt , phonton and xprint.