RFE: Add support for ISO-8859-6-16 encoded fonts

RESOLVED WONTFIX

Status

()

Core
Layout: Text
--
enhancement
RESOLVED WONTFIX
15 years ago
10 years ago

People

(Reporter: Roland Mainz, Assigned: Roland Mainz)

Tracking

({relnote})

Trunk
All
Linux
relnote
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

15 years ago
RFE: Add support for ISO-8859-6-16 encoded fonts (see
http://www.langbox.com/arabic/fontara16.html).

Comment 1

15 years ago
I am curious. Is this charset officially registered in IANA charset 
registry?
http://www.iana.org/assignments/character-sets
Thanks for pointing that out. This encoding doesn't seem to be registered in
IANA, so we should use a name like x-iso-8859-6-16.

As far as I know, it's never used as a content encoding, and the intention is
only to add a decoder to be used by the GTK and Xlib rendering layers.
(Assignee)

Comment 3

15 years ago
Simon Montagu wrote:
> Thanks for pointing that out. This encoding doesn't seem to be registered in
> IANA, so we should use a name like x-iso-8859-6-16.

AFAIK ISO8859-6.8x isn't registered by IANA either... this is just another
encoding for arabic X11 fonts...
(Assignee)

Comment 4

15 years ago
Created attachment 104373 [details]
Arabic *-iso8859-6.16 encoded bitmap fonts for testing as tar.gz

The tar.gz archive attached here contains two BDF sample fonts in the
"*-iso8859-6.16"-encoding (donation from langbox.com :)
-lbi-naskhi16-medium-r-normal--16-160-75-75-p-22-iso8859-6.16
-lbi-arabic16-medium-r-normal--15-130-75-75-c-100-iso8859-6.16
Created attachment 105522 [details] [diff] [review]
Patch v.0

There are still some issues to be resolved here. Arabic text looks fine, but
English text on Arabic pages isn't always shown correctly.
Comment on attachment 105522 [details] [diff] [review]
Patch v.0

The symmetric characters '(' ')' '[' and ']' also need correction. The
8859-6-16 encoding reverses them!
Attachment #105522 - Flags: needs-work+
(Assignee)

Comment 7

15 years ago
Comment on attachment 105522 [details] [diff] [review]
Patch v.0

Nice patch... can you fix the nits listed in your prevous comment and update
the patch to match "trunk" (I'd like to see it as part of Mozilla 1.4a if
possible... :) , please ?
Looking again at the font mapping at
http://www.langbox.com/arabic/fontara16.html, the encoding maps Arabic glyphs to
codepoints in the US-ASCII range, which makes it impractical for most web
content. This is the cause of the problem I mention in comment 5, and it will
occur on any site that does not rigidly mark up every change of language with
<span lang="ar"> <span lang="en"> etc.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → WONTFIX
I was hasty WONTFIXing this, the problems are all soluble
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Created attachment 117981 [details] [diff] [review]
Patch v.1

This also moves both the Langbox Arabic encoding into the 
#ifdef MOZ_EXTRA_X11CONVERTERS, since we don't need them elsewhere
Attachment #105522 - Attachment is obsolete: true

Comment 11

15 years ago
I'm competent to review the changes to nsFontMetricsGTK.cpp (and probably
nsFontMetricsXlib.cpp) and those look fine to me, so r=bstell@ix.netcom.com
on those parts.

The rest needs a qualified reviewer like ftang to approve them.
(Assignee)

Comment 12

15 years ago
The patch is OK in general (it's a simple |nsBasicEncoder|, it's unlikely to
make much mistakes in this code... :) ...
... jumping into superstrict nitpicking review mode I see some minor issues:

-- SON (start-of-nitpicking) -- 
> Index: intl/uconv/src/nsUConvModule.cpp
> ===================================================================
> RCS file: /cvsroot/mozilla/intl/uconv/src/nsUConvModule.cpp,v
> retrieving revision 1.42
> diff -u -r1.42 nsUConvModule.cpp
> --- intl/uconv/src/nsUConvModule.cpp    15 Mar 2003 01:02:56 -0000      1.42
> +++ intl/uconv/src/nsUConvModule.cpp    21 Mar 2003 07:02:50 -0000
> @@ -197,7 +197,10 @@
>  #include "nsUnicodeToMacGujarati.h"
>  #include "nsUnicodeToMacGurmukhi.h"
>  #include "nsUnicodeToMacHebrew.h"
> +#ifdef MOZ_EXTRA_X11CONVERTERS
>  #include "nsUnicodeToLangBoxArabic8.h"
> +#include "nsUnicodeToLangBoxArabic16.h"
> +#endif

#endif /* MOZ_EXTRA_X11CONVERTERS */ (assuming that this #if...#endif may grow)
...

>  // ucvibm
>  #include "nsUCvIBMCID.h"
> @@ -364,7 +367,10 @@
>  NS_UCONV_REG_UNREG_ENCODER("UTF-16" , NS_UNICODETOUTF16_CID)
>  NS_UCONV_REG_UNREG_ENCODER("Adobe-Symbol-Encoding" , NS_UNICODETOSYMBOL_CID)
>  NS_UCONV_REG_UNREG_ENCODER("x-zapf-dingbats" , NS_UNICODETOZAPFDINGBATS_CID)
> +#ifdef MOZ_EXTRA_X11CONVERTERS
>  NS_UCONV_REG_UNREG_ENCODER("x-iso-8859-6-8-x" ,
NS_UNICODETOLANGBOXARABIC_CID)
> +NS_UCONV_REG_UNREG_ENCODER("x-iso-8859-6-16" ,
NS_UNICODETOLANGBOXARABIC16_CID)
> +#endif

...

>    // ucvibm
>  NS_UCONV_REG_UNREG("IBM850", NS_CP850TOUNICODE_CID, NS_UNICODETOCP850_CID)
> @@ -462,7 +468,10 @@
>  NS_GENERIC_FACTORY_CONSTRUCTOR(nsUnicodeToUTF16);
>  NS_GENERIC_FACTORY_CONSTRUCTOR(nsUnicodeToUTF32BE);
>  NS_GENERIC_FACTORY_CONSTRUCTOR(nsUnicodeToUTF32LE);
> +#ifdef MOZ_EXTRA_X11CONVERTERS
>  NS_GENERIC_FACTORY_CONSTRUCTOR(nsUnicodeToLangBoxArabic8);
> +NS_GENERIC_FACTORY_CONSTRUCTOR(nsUnicodeToLangBoxArabic16);
> +#endif

see above...

>  // ucvibm
>  
> @@ -1343,11 +1352,18 @@
>      NS_UNICODEENCODER_CONTRACTID_BASE "x-mac-hebrew",
>      nsUnicodeToMacHebrewConstructor, 
>    },
> +#ifdef MOZ_EXTRA_X11CONVERTERS
>    { 
>      ENCODER_NAME_BASE "x-iso-8859-6-8-x" , NS_UNICODETOLANGBOXARABIC_CID, 
>      NS_UNICODEENCODER_CONTRACTID_BASE "x-iso-8859-6-8-x",
>      nsUnicodeToLangBoxArabic8Constructor, 
>    },
> +  { 
> +    ENCODER_NAME_BASE "x-iso-8859-6-16" , NS_UNICODETOLANGBOXARABIC16_CID, 
> +    NS_UNICODEENCODER_CONTRACTID_BASE "x-iso-8859-6-16",
> +    nsUnicodeToLangBoxArabic16Constructor, 
> +  },
> +#endif

ditto.

>    // ucvibm
>    { 
>      DECODER_NAME_BASE "IBM850" , NS_CP850TOUNICODE_CID, 
> Index: intl/uconv/ucvlatin/Makefile.in
> ===================================================================
> RCS file: /cvsroot/mozilla/intl/uconv/ucvlatin/Makefile.in,v
> retrieving revision 1.49
> diff -u -r1.49 Makefile.in
> --- intl/uconv/ucvlatin/Makefile.in     31 Jan 2003 23:26:15 -0000      1.49
> +++ intl/uconv/ucvlatin/Makefile.in     21 Mar 2003 07:02:50 -0000
> @@ -129,6 +129,7 @@
>                 nsUnicodeToKOI8R.cpp \
>                 nsUnicodeToKOI8U.cpp \
>                 nsUnicodeToLangBoxArabic8.cpp \
> +               nsUnicodeToLangBoxArabic16.cpp \
>                 nsUnicodeToMacCE.cpp \
>                 nsUnicodeToMacGreek.cpp \
>                 nsUnicodeToMacTurkish.cpp \
> Index: intl/uconv/ucvlatin/nsUCvLatinCID.h
> ===================================================================
> RCS file: /cvsroot/mozilla/intl/uconv/ucvlatin/nsUCvLatinCID.h,v
> retrieving revision 1.36
> diff -u -r1.36 nsUCvLatinCID.h
> --- intl/uconv/ucvlatin/nsUCvLatinCID.h 8 Oct 2002 23:58:43 -0000       1.36
> +++ intl/uconv/ucvlatin/nsUCvLatinCID.h 21 Mar 2003 07:02:50 -0000
> @@ -601,9 +601,11 @@
>  #define NS_UNICODETOUESCAPE_CID \
>    { 0x319ff9c3, 0x51d2, 0x11d3, {0xb3, 0xc3, 0x0, 0x80, 0x5f, 0x8a, 0x66,
0x70}}
>  
> +#ifdef MOZ_EXTRA_X11CONVERTERS
>  // {49B38F11-6193-11d3-B3C5-00805F8A6670}
>  #define NS_UNICODETOLANGBOXARABIC8_CID \
>    { 0x49b38f11, 0x6193, 0x11d3, {0xb3, 0xc5, 0x0, 0x80, 0x5f, 0x8a, 0x66,
0x70}}
> +#endif

...

>  // {49B38F12-6193-11d3-B3C5-00805F8A6670}
>  #define NS_UNICODETOUTF16_CID \
> @@ -661,8 +663,14 @@
>  #define NS_UNICODETOMACGURMUKHI_CID \
>    { 0x6803cacf, 0x1e3b, 0x11d5, { 0xa1, 0x45, 0x0, 0x50, 0x4, 0x83, 0x21,
0x42 } }
>  
> +#ifdef MOZ_EXTRA_X11CONVERTERS
>  // {4DBBD94F-0153-44cb-966A-7F39B9CB477D}
>  #define NS_UNICODETOLANGBOXARABIC_CID \
>    { 0x4dbbd94f, 0x153, 0x44cb, { 0x96, 0x6a, 0x7f, 0x39, 0xb9, 0xcb, 0x47,
0x7d } }
> +
> +// {8E269A63-94B4-46e5-A31F-11F0EDE6065B}
> +#define NS_UNICODETOLANGBOXARABIC16_CID \
> +  { 0x8e269a63, 0x94b4, 0x46e5, { 0xa3, 0x1f, 0x11, 0xf0, 0xed, 0xe6, 0x6,
0x5b } }
> +#endif

...

>  #endif /* nsUCvLatinCID_h___ */
> Index: intl/uconv/ucvlatin/nsUnicodeToLangBoxArabic16.cpp
> ===================================================================
> RCS file: intl/uconv/ucvlatin/nsUnicodeToLangBoxArabic16.cpp
> diff -N intl/uconv/ucvlatin/nsUnicodeToLangBoxArabic16.cpp
> --- /dev/null   1 Jan 1970 00:00:00 -0000
> +++ intl/uconv/ucvlatin/nsUnicodeToLangBoxArabic16.cpp  21 Mar 2003 07:02:50
-0000
> @@ -0,0 +1,327 @@
> +/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
*/
> +/* ***** BEGIN LICENSE BLOCK *****
> + * Version: NPL 1.1/GPL 2.0/LGPL 2.1
> + *
> + * The contents of this file are subject to the Netscape Public License
> + * Version 1.1 (the "License"); you may not use this file except in
> + * compliance with the License. You may obtain a copy of the License at
> + * http://www.mozilla.org/NPL/
> + *
> + * Software distributed under the License is distributed on an "AS IS" basis,
> + * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
> + * for the specific language governing rights and limitations under the
> + * License.
> + *
> + * The Original Code is Mozilla Communicator client code.
> + *
> + * The Initial Developer of the Original Code is
> + * Netscape Communications Corporation.
> + * Portions created by the Initial Developer are Copyright (C) 2002
> + * the Initial Developer. All Rights Reserved.
> + *
> + * Contributor(s): Simon Montagu

email?
+ * Contributor(s):
+ *   Simon Montagu <smontagu@netscape.com>

[snip]
> +  0x79, // U+FEFB ARABIC LIGATURE LAM ALEF ISOLATED FORM
> +  0xFD, // U+FEFC ARABIC LIGATURE LAM ALEF FINAL FORM
        ^^^

One comma too much...

> + };
> +
> +NS_IMETHODIMP nsUnicodeToLangBoxArabic16::Convert(
> +      const PRUnichar * aSrc, PRInt32 * aSrcLength,
> +      char * aDest, PRInt32 * aDestLength)
[snip]
> +NS_IMETHODIMP nsUnicodeToLangBoxArabic16::FillInfo(PRUint32* aInfo)
> +{
> +   PRUnichar i;
> +
> +   // ASCII range
> +   for(i=0x0000; i <= 0x007F; i++)
> +     CLEAR_REPRESENTABLE(aInfo, i);

What about referencing http://bugzilla.mozilla.org/show_bug.cgi?id=172491#c8 ?
:)

> +   for(i=0x0020; i <= 0x002B; i++)
> +     SET_REPRESENTABLE(aInfo, i);
> +   for(i=0x002D; i <= 0x002F; i++)
> +     SET_REPRESENTABLE(aInfo, i);
> +   SET_REPRESENTABLE(aInfo, 0x003A);
> +   for(i=0x003C; i <= 0x003E; i++)
> +     SET_REPRESENTABLE(aInfo, i);
> +   SET_REPRESENTABLE(aInfo, 0x0040);
> +   for(i=0x005B; i <= 0x005F; i++)
[snip]
> Index: intl/uconv/ucvlatin/nsUnicodeToLangBoxArabic16.h
> ===================================================================
> RCS file: intl/uconv/ucvlatin/nsUnicodeToLangBoxArabic16.h
> diff -N intl/uconv/ucvlatin/nsUnicodeToLangBoxArabic16.h
> --- /dev/null   1 Jan 1970 00:00:00 -0000
> +++ intl/uconv/ucvlatin/nsUnicodeToLangBoxArabic16.h    21 Mar 2003 07:02:50
-0000
> @@ -0,0 +1,75 @@
> +/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
*/
> +/* ***** BEGIN LICENSE BLOCK *****
> + * Version: NPL 1.1/GPL 2.0/LGPL 2.1
> + *
> + * The contents of this file are subject to the Netscape Public License
> + * Version 1.1 (the "License"); you may not use this file except in
> + * compliance with the License. You may obtain a copy of the License at
> + * http://www.mozilla.org/NPL/
> + *
> + * Software distributed under the License is distributed on an "AS IS" basis,
> + * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
> + * for the specific language governing rights and limitations under the
> + * License.
> + *
> + * The Original Code is Mozilla Communicator client code.
> + *
> + * The Initial Developer of the Original Code is 
> + * Netscape Communications Corporation.
> + * Portions created by the Initial Developer are Copyright (C) 2002
> + * the Initial Developer. All Rights Reserved.
> + *
> + * Contributor(s): Simon Montagu

email?

> + *
> + *
> + * Alternatively, the contents of this file may be used under the terms of
> + * either the GNU General Public License Version 2 or later (the "GPL"), or
> + * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
> + * in which case the provisions of the GPL or the LGPL are applicable instead
> + * of those above. If you wish to allow use of your version of this file only
> + * under the terms of either the GPL or the LGPL, and not to allow others to
> + * use your version of this file under the terms of the NPL, indicate your
> + * decision by deleting the provisions above and replace them with the notice
> + * and other provisions required by the GPL or the LGPL. If you do not delete
> + * the provisions above, a recipient may use your version of this file under
> + * the terms of any one of the NPL, the GPL or the LGPL.
> + *
> + * ***** END LICENSE BLOCK ***** */
> +#ifndef nsUnicodeToLangBoxArabic16_h__
> +#define nsUnicodeToLangBoxArabic16_h__
> +#include "nsUCSupport.h"

Actually a |#ifndef nsUCSupport_h___ ... #endif| would be nice - but
nsUCSupport.h uses |nsUCvJaSupport_h___| instead (why?!) ...

> +
> +//----------------------------------------------------------------------
> +// Class nsUnicodeToLangBoxArabic16 [declaration]
> +class nsUnicodeToLangBoxArabic16 : public nsBasicEncoder
> +{
[snip]
> +};
> +
> +#endif // nsUnicodeToLangBoxArabic16_h__

#endif // !nsUnicodeToLangBoxArabic16_h__
-- EON (end-of-nitpicking) --

Compiles, browses and prints arabic (using Xprint) and the patch is OK in
general ...

... r=roland.mainz@informatik.med.uni-giessen.de (no need to file a new patch
before checkin unless the superreviewer wants to see other issues fixed, too)
(Assignee)

Updated

15 years ago
Attachment #117981 - Flags: superreview?(bzbarsky)
Attachment #117981 - Flags: review+
+   if(*aSrc<=0) {
+      *aDestLength = 0;
+      return NS_OK;
+   }

Isn't this supposed to be "*aSrcLength"?
If it is supposed to be "*aSrcLength", you can get rid of the whole 'if'
statement because the rest of the code will Do The Right Thing.
(Assignee)

Comment 14

15 years ago
Robert O'Callahan wrote:
> +   if(*aSrc<=0) {
> +      *aDestLength = 0;
> +      return NS_OK;
> +   }
>
> Isn't this supposed to be "*aSrcLength"?

Yes, this should be AFAIK "*aSrcLength"... OK, I am blind... ;-/
... and |nsUnicodeToLangBoxArabic8| has the same bug...

> If it is supposed to be "*aSrcLength", you can get rid of the whole 'if'
> statement because the rest of the code will Do The Right Thing.

Well, this would be a shortcut and to safeguard against later changes below in
the code...
We should just get the while loop right and not write unnecessary code. Please
take the "if" out in both files where it appears.
Created attachment 118384 [details] [diff] [review]
Patch v.2

Addressed all but the most super-pedantic nits.
Attachment #117981 - Attachment is obsolete: true
(Assignee)

Comment 17

15 years ago
Comment on attachment 118384 [details] [diff] [review]
Patch v.2

nice work :)
r=roland.mainz@informatik.med.uni-giessen.de
Attachment #118384 - Flags: superreview?(roc+moz)
Attachment #118384 - Flags: review+
Attachment #118384 - Flags: superreview?(roc+moz) → superreview+
Patch checked in. Reassigning to gisburn to deal with release notes.
Assignee: smontagu → Roland.Mainz
Status: REOPENED → NEW
Keywords: relnote
(Assignee)

Updated

15 years ago
Blocks: 199741
Attachment #117981 - Flags: superreview?(bzbarsky)

Updated

15 years ago
No longer blocks: 199741
(Assignee)

Updated

15 years ago
Blocks: 199741
(Assignee)

Comment 19

15 years ago
Filed http://bugs.xfree86.org//cgi-bin/bugzilla/show_bug.cgi?id=420 ("RFE: Add
encodings files for Arabic LangBox encodings iso8859-6.8, iso8859-6.8x and
iso8859-6.16") to get support for these encodings in Xfree86...

Comment 20

14 years ago
The patch was checked in over a year ago, is there any reason why this bug is
still open?

Prog.
(In reply to comment #20)
> The patch was checked in over a year ago, is there any reason why this bug is
> still open?
> 
> Prog.

No reply for the last 3 and a half years, so I'm going to go ahead and assume "no".
Status: NEW → RESOLVED
Last Resolved: 15 years ago10 years ago
Resolution: --- → FIXED
Oh historical irony. Since this was left unresolved until now (waiting for the release notes mentioned in comment 18), and the code checked in in 2003 was removed just last week in bug 413613, it should really be WONTFIX.
Resolution: FIXED → WONTFIX

Updated

10 years ago
Component: Layout: BiDi Hebrew & Arabic → Layout: Text
QA Contact: zach → layout.fonts-and-text
You need to log in before you can comment on or make changes to this bug.