Closed Bug 67840 Opened 24 years ago Closed 24 years ago

Missing/incomplete nsIFontEnumerator::HaveFontFor method

Categories

(Core :: Internationalization, defect, P1)

Product:
Core
Component:
Internationalization
Platform:
x86
All
Type:
defect

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: cls, Assigned: ftang)

References

Details

(Keywords: platform-parity)

Attachments

(1 file)

the rest (I think) of the necessary stubs
2.09 KB, patch
Details | Diff | Splinter Review 
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?
Severity: critical → blocker
Keywords: pp
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?
Stub added to OS2 and BeOS
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Changed QA contact to ftang@netscape.com since QA cannot verify this.
QA Contact: teruko → ftang
I just added the stub (copied from GTK) to the xlib port as well (after getting
approval from cls and blizzard on IRC).
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*)

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 ?
r=cls.  Still need a sr= and then an a= from drivers@mozilla.org.  We need to
get this into the 0.8 branch too. 

Maybe we should also fix photon and qt?  Then we should have all implementations
of nsIFontEnumerator fixed.
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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
<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
 };
Priority: -- → P1
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?
>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.
Status: REOPENED → ASSIGNED
can you #ifdef in idl ?
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 ? :)


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
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.
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
Blocks: 49947
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?
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.
*** Bug 69465 has been marked as a duplicate of this bug. ***
check in fix for qt , phonton and xprint.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: