Jungshik Shin Assignee
	Description • 22 years ago

This is the dual of bug 204993.

Unicode defines a set of characters with 
default_ignorable_code_point 'attribute'. By default, these characters should 
be rendered with invisible (usually non-advancing) glyphs. However, some fonts 
have visible glyphs apparently to let some applications (e.g. 
(MathML/html/xml) editors, TeX frontends, Character picker, code charts and so 
forth) to show visible glyphs for invisible characters when necessary. 
Currently, Mozilla doesn't do anything about them and they're just passed to 
draw_string functions. This leads characters like U+2062(Invisible Times) to 
be rendered with a conspicuous glyph (as found in fonts like Code2000 : 
http://home.att.net/~jameskass). Before passing them to draw_string, we have 
to filter them out. 

rbs thinks that it had better  be done in gfx because nsTextFrame is kinda too 
complex at the moment.

	Jungshik Shin Assignee
	Comment 1 • 22 years ago

Characters with default_ignorable_code are listed in 

http://www.unicode.org/Public/4.0-Update/DerivedCoreProperties-4.0.0.txt
  

U+2062 (invisible times) is one of them (ZWJ/ZWNJ are not). According to Mark
Davis, characters with 'default_ignorable_code' can be ignored (be turned to
nothing), but can affect the rendering/layout if supported. That is, 'ab' can 
berendered a little differently from 'a⁢b'. 

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 2 • 22 years ago

I think we should do this in nsTextFrame since we may want to enable displaying
these characters (perhaps using a new CSS property). Plus, then we only have to
do it once instead of once per platform.

	rbs
	Comment 3 • 22 years ago

The global picture is the transliteration of non vanishing characters for which
there is no glyph on the system. That one is needed (platform-specific). But
what happens is that once it is there, it costs nothing to piggy back on it for
the other vanishing characters. So that nsTextFrame is left alone.

However, this system falls apart if there are troublesome fonts which claim to
have a glyph while they don't.

Perhaps a trick might be to make mLoadedFonts[0] always be a "black hole font",
i.e., a transliteration font that is only populated with the ignorable code
points, mLoadedFonts[0]->HasChar(aChar) only succeeds if aChar is an ignorable
char. This font returns 0 as getWidth(), draws nothing, ... it is an absorbing
black hole, except when CSS says otherwise as roc mentioned, in which case it
might behave as blizzard's |MiniFont| which draws a rectangle around the Unicode
point, or it might simply show a Numeric Character Reference &#xNNNN;
Seems there are more flexibility here, albeit with some redundant codes across
the platforms for the trade-off of simplicity (that would'n't be the first time).

Alternatively, going back to the of nsTextFrame...

	Jungshik Shin Assignee
	Comment 4 • 22 years ago

> this system falls apart if there are troublesome fonts which claim to > have a
glyph while they don't.
  
  To clarify, this bug is about 'troublesome? fonts' that claim to have glyphs
for default_ignoreable code points and actually  have visible glyphs. Against
them 'black-hole' font scheme is a nice guard. Needless to say, as you wrote,
it's also a good guard against genuinely troublesome fonts that lie about their
coverage. [1] 

> mLoadedFonts[0]->HasChar(aChar) only succeeds if aChar is an
> ignorable char...
   
   And unless a yet hypothetical 'turn invisible to visible' CSS property is set.
  

> when CSS says otherwise as roc mentioned, 
> in which case it might behave as blizzard's |MiniFont|

 In that case, I think mBlackHoleFont->HasChar(c) has to fail and the normal
glyph search kicks in. If no font has/claims it, transliteration[2] or minifont
will take care of it. That way, a font with nice _visible_ glyphs for the
invisible can be picked up. We have a trouble if the author of a document just
specfies generic fonts and a font with an invisible glyph is earlier in the
matched font list than one with a visible glyph. We can work around this, but it
seems to be getting complicated a bit...
  

BTW, I noticed that nsTextTransfomer.cpp has a macro 'IS_DISCARDED' [3]  that
currently handles SHY and CR only (and BIDI control chars).  SHY handling needs
to be left there (but , I guess, needs to be modified more in line with its role
as 'discretionary hyphen'.
Getting off the track, nsTextTransformer seems to   need some works. For one,
it's assuming that PRUnichar* is a string of UCS2, which is not the case.

Another BTW, I just realized that I had been wrong to say that ZWNJ/ZWJ are not
default ignorable code points. They ARE along with a lot of other characters
(Hangul Jamo Fillers, Mongolian Free Variation Selectors) that need
font-specific/platform/toolkit dependent context-sensitive treatment. We have to
throw into a 'black hole' only a subset of 'Default Ingorable Code points'. They
include U+2062 (invisible times) and the like ....

To determine exactly what to do where, we need to study more details of their
characteristics. I'm inserting the qualifier 'truly invisible' in the summary line.


[1] In case of Xft build, fontconfig already offers a similar protection. 

[2] Transliteration should not turn default_ignorable to nothingness. Instead it
has to resort to visible fallback or pass it through to minifont (in case of
Xft) because the fact that it reaches there means that it has to be visible per
CSS once we implement 'black-hole font' or equivalent.

[3]
http://lxr.mozilla.org/seamonkey/source/layout/html/base/src/nsTextTransformer.cpp#153

	rbs
	Comment 5 • 22 years ago

> it seems to be getting complicated a bit...

and it is doubtful that it is worth the trouble at since it is based on the
hypothetical CSS property.

> BTW, I noticed that nsTextTransfomer.cpp has a macro 'IS_DISCARDED'

which breaks linebreaking (a long standing bug).

> [2] Transliteration should not turn default_ignorable to nothingness.

That's right. Ignorable chars and non-ignorable chars will then be treated as
two mutually exclusive subsets. Just a matter of initializations. All this
(including the activities of the MiniFont) can really be implemented with just
one class, nsFont[GTK|Xlib}Win|Xft,etc]Substitute which is flagged properly at
initialization. For instance, it is trivial to setup a 'black hole font' on
Windows right now by just |new|ing a first font from the
existing nsFontWinSubstitute class and filling its ccmap appropriately.

	rbs
	Comment 6 • 22 years ago

Another alternative that jshin just suggested (in bug 192088) to bypass all
ignorable chars is to remove them from any font cmaps. This way, the single
substitute font (that exists at present) is guaranteed to kick in...

jshin's proposal might be easier and less disruptive to implement. Differences
between the approaches might be in the performance, though.  But there is room
for optimization in jshin's proposal. The problem with the substitute font is
that it kicks in only _after_ trying all the fonts in the system... So this can
be optimized by sync'ing a static global map of the chars of interest, and not
trying global fonts for those chars. With GfxWin, this will amount to an early
return of |nsnull| in |FindGlobalFont| for those chars.

	rbs
	Comment 7 • 22 years ago

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

	Kevin McCluskey (gone)
	Comment 8 • 22 years ago

-> smontagu

	rbs
	Comment 9 • 21 years ago

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

	Jungshik Shin Assignee
	Comment 10 • 21 years ago

I just took an easy path and made a new subclass of nsFontWin,
nsFontWinBlackHole.  It's possible to do it with nsFontWinSubstitute perhaps
with some perf. loss.

My list of 'truly invisible chars' is a bit arbitrary. I took the list from
Unicode 4.0 Derived..*TXT and then removed some obvious characters(e.g. ZWNJ,
ZWJ for Indic and Arabic) that should be passed down to 'real' fonts.

I also removed non-BMP characters. I have to revise my perl script ccmapbin.pl
to generate an extended compressed cmap file for non-BMP characters. 

rbs, can you take a look?

	Jungshik Shin Assignee
	Comment 11 • 21 years ago

re : comment #5
>> BTW, I noticed that nsTextTransfomer.cpp has a macro 'IS_DISCARDED'

> which breaks linebreaking (a long standing bug).

 So, linebreaker is invoked after this 'text transform' thing? If so, 
defining IS_INVISIBLE(ch) as 
'CCMAP_HAS_CHAR_EXT(gInvisibleCharsCCMap, ch)' and calling it in appropriate
places in nsTextTransform.cpp wouldn't work either, would it? 

	Jungshik Shin Assignee
	Comment 12 • 21 years ago

this one doesn't introduce a new class, but just uses nsFontWinSubstitute.

	rbs
	Comment 13 • 21 years ago

Second patch looks fine -- just what I had in mind in comment 5.

> re: IS_DISCARDED vs IS_INVISIBLE
not sure what it will take to make that work, if at all. something that you
might want to try. 

> I also removed non-BMP characters.
are there also invisible characters in the non-BMP planes?

	Jungshik Shin Assignee
	Comment 14 • 21 years ago

I revised intl/unicharutil/tools/ccmapbin.pl to generate an extended CCMap if
non-BMP charaacters are present in the input. nsFontMetricsWin.cpp was modified
slightly to work with the extended CCMap thus generated. 

As for non-BMP ignorable characters, there are a bunch of 'em :-) (see the
patch for the list) Well, most of them (language tags and reserved code points
in plane 15 and some musical symbols in plane 1) would never occur in normal
texts, but anyway there are. Some of them may have to be passed down the
rendering code and turned to nothingness at the end instead of at the very
beginning.

	Jungshik Shin Assignee
	Comment 15 • 21 years ago

Comment on attachment 131740 [details] [diff] [review]
patch v3 with non-BMP coverage 

Asking for r and sr.

The list of characters to put into this category has to be revised as we know
them better. In the meantime, let's get this done for GFX:Win with the
understanding that the list will be revised.

I'll also keep exploring the possibility of doing it in an XP-way in nsText*
(layout) as well as working other GFX ports.

	Jungshik Shin Assignee
	Comment 16 • 21 years ago

assigning to myself and setting the target milestone

	Simon Montagu :smontagu
	Comment 17 • 21 years ago

Comment on attachment 131740 [details] [diff] [review]
patch v3 with non-BMP coverage 

>@@ -3606,25 +3633,21 @@ nsFontMetricsWin::RealizeFont()
>     mXHeight = NSToCoordRound((float)metrics.tmAscent * dev2app * 0.56f); // 50% of ascent, best guess for true type
>     mSuperscriptOffset = NSToCoordRound(oMetrics.otmptSuperscriptOffset.y * dev2app);
>     mSubscriptOffset = NSToCoordRound(oMetrics.otmptSubscriptOffset.y * dev2app);
> 
>     mStrikeoutSize = PR_MAX(onePixel, NSToCoordRound(oMetrics.otmsStrikeoutSize * dev2app));
>     mStrikeoutOffset = NSToCoordRound(oMetrics.otmsStrikeoutPosition * dev2app);
>     mUnderlineSize = PR_MAX(onePixel, NSToCoordRound(oMetrics.otmsUnderscoreSize * dev2app));
>-    if (gDoingLineheightFixup) {
>-      if(IsCJKLangGroupAtom(mLangGroup.get())) {
>-        mUnderlineOffset = NSToCoordRound(PR_MIN(oMetrics.otmsUnderscorePosition, 
>-                                                 oMetrics.otmDescent + oMetrics.otmsUnderscoreSize) 
>-                                                 * dev2app);
>-        // keep descent position, use it for mUnderlineOffset if leading allows
>-        descentPos = NSToCoordRound(oMetrics.otmDescent * dev2app);
>-      } else {
>-        mUnderlineOffset = NSToCoordRound(PR_MIN(oMetrics.otmsUnderscorePosition*dev2app, 
>-                                                 oMetrics.otmDescent*dev2app + mUnderlineSize));
>-      }
>+    if (gDoingLineheightFixup &&
>+        IsCJKLangGroupAtom(mLangGroup.get())) {
>+      mUnderlineOffset = NSToCoordRound(PR_MIN(oMetrics.otmsUnderscorePosition, 
>+                                               oMetrics.otmDescent + oMetrics.otmsUnderscoreSize) 
>+                                               * dev2app);
>+      // keep descent position, use it for mUnderlineOffset if leading allows
>+      descentPos = NSToCoordRound(oMetrics.otmDescent * dev2app);

Is this hunk extraneous?

Other than that, r=smontagu for the C++ parts. I can't do reviews of Perl, I'm
afraid.

	Jungshik Shin Assignee
	Comment 18 • 21 years ago

Thanks for r and sorry for the 'pollution' from bug 219225.

As for Perl part, it's not a part of the build and I made sure that the old
script(in the trunk) and the new script produce identical outputs for BMP-only
cases so that I'm gonna just check that in once I get sr from rbs.
 

	rbs
	Comment 19 • 21 years ago

Comment on attachment 131740 [details] [diff] [review]
patch v3 with non-BMP coverage 

+// It's a pre-compiled extended ccmap so that the pointer
+// has to point at the 3rd element.
+static PRUint16 *gCharsToBeBlackHoledCCMap = gCharsToBeBlackHoledCCMapRaw + 2;

Why? (why not just discard the 2 first irrelevant entries when saving
the extended .ccmap?)

"black-hole" is catchy... but "ignorable" is the most common terminology
and seems more understandable for a newcomer without a background on the
implementation. What about s/BlackHole/Ignorable/g if this doesn't create
any uglyness of its own.

sr=rbs

	Simon Montagu :smontagu
	Comment 20 • 21 years ago

Oh yes, I was also going to suggest s/BlackHole/Ignorable/g but forgot.

I especially don't like "blackhole" becoming a verb as in "ToBeBlackHoled"

	Jungshik Shin Assignee
	Comment 21 • 21 years ago

> Why? (why not just discard the 2 first irrelevant entries when saving
> the extended .ccmap?)

 They're relevant (the first entry indicates whether non-BMP is present or not
and the second entry has the size of the CCMap for the BMP portion), but they
have to have _negative_  indices. See the definition of CCMAP_HAS_CHAR_EXT at
http://lxr.mozilla.org/seamonkey/source/gfx/public/nsCompressedCharMap.h#349

> "blackhole" becoming a verb

  English(inflectional) has become more and more like Chinese (isolating) :-)
Anyway, I'll replace 'blackhole' with 'ignorable' when checking in. 

	rbs
	Comment 22 • 21 years ago

Maybe you then need a different extension: .x-cxmap? 

...so that when you do:

static const PRUint16 gBlankCCMap[] = {
#include "blank.ccmap"
};
then it makes sense to have
#define SHOULD_BE_SPACE_CHAR(ch)  (CCMAP_HAS_CHAR(gBlankCCMap,ch))

and when one does:

static const PRUint16 gIgnorableCCMapExtRaw[] = {
#include "ignorable.x-ccmap"
};
it is also "clear" that one has to do:
gIgnorableCCMapExt = gIgnorableCCMapExtRaw + 2

	David Baron :dbaron:
	Comment 23 • 21 years ago

There's already code to do some things like this in nsTextTransformer.  Why
shouldn't we be doing this there?  Doing it in Gfx seems like the wrong place,
and it requires that you write cross-platform code separately for each platform.

That said, nsTextTransformer requires that you write the same code in about 4
places as well, so I'm not sure if it's actually better, although there is some
chance of improvement.

(I'm reasonably sure that the nsTextTransformer code I'm thinking of is invoked
after linebreaking, since it replaces the NBSP character with a space.)

	Jungshik Shin Assignee
	Comment 24 • 21 years ago

Fix checked in with concerns in comment #22 addressed. I also had to make a
little change in |ResolveForward| 

3937   if (currFont == mLoadedFonts[firstFont]) { 
3938     while (currChar < lastChar && 
3939            (currFont->HasGlyph(*currChar)) &&
3940            !CCMAP_HAS_CHAR_EXT(gIgnorableCCMapExt, *currChar))
3941       ++currChar;

As for doing it in nsTextTransform, ideally I think that's  better. It seems
like redefining IS_DISCARDED macro as 'CCMAP_HAS_CHAR_EXT(gIgnorableCCMap, ch)'
(with the list of ignorable characters adjusted a bit. I'm not sure how BIDI
control characters have to be treated in nsTextTransform)  works if we change
nsTextFragment:CharAt to return PRUint32 (for the full repertoire of Unicode) or
define a new method CharAt32 (or something like) to nsTextFragment. This
approach has some ripple effect 

	rbs
	Comment 25 • 21 years ago

re: comment #23: dbaron, it is a bit more demanding than you may think at first.
The characters have to be invisible (0-width), but still active for
linebreaking/bidi/etc.

There was an aborted attempt some while ago (it missed the required analysis to
justify that the proposed patch didn't break anything or bring ripple effects).
Whereas avoiding the chars in gfxWin is okay (a simple & well understood
leveraging of something already there -- just render the chars with 0-width). On
balance, having witnessed the related _long-standing_ ­ problem, it seems
fine to me to have this fix, while keeping open the possibility of a common fix.

	David Baron :dbaron:
	Comment 26 • 21 years ago

Which ­ problem?  The fact that we don't support it?  (The main problem is
that our line-breaking algorithm is extremely simple, but beyond that, ­
would be somewhat difficult to implement because the act of breaking changes the
text measurement.)

Or is there something wrong with the existing code in nsTextTransformer that
uses the IS_DISCARDED macro?

	rbs
	Comment 27 • 21 years ago

> Which ­ problem?

By the ­ problem, I mean the fact that it is expected to be invisible
(0-width) if it is inside a word, and only be visible (as a dash '-') if
linebreaking occurs right on it. [This is just what you seemed to describe too,
but expressed differently...]

There is a problem with IS_DISCARDED. In its present form, it cannot cater for
this bug. It would break bidi and prevent linebreaking from happening at the
very points where characters are discarded. The thinking is that by the (long)
time all the uncertainties in nsTextFrame are resolved, it doesn't harm to have
this simple gfx-based fix which does what is expected and doesn't have
correctness issues.

[I am starting to suspect that a fix for ­ may actually be to not discard
it. But instead to break on it, and treat the three pieces as different words,
with &shy special-cased (as  ) and not rendered unless linebreaking occurs.]

	David Baron :dbaron:
	Comment 28 • 21 years ago

Did you actually test that that's the case?  Considering that   is replaced
by space at the same point where IS_DISCARDED is handled, I think IS_DISCARDED
would work just fine for this bug, as I said in comment 23.

	rbs
	Comment 29 • 21 years ago

IS_DISCARDED might have some issues if those characters are relevant for
linebreaking. Here is a patch for the earlier attempt that I mentioned (it was
for the case of CH_ZWSP -- a.k.a. ⁣ in MathML):
http://bugzilla.mozilla.org/attachment.cgi?id=49397&action=view
IS_DISCARDED proved too simplistic, and prevented linebreaking from occurring on
CH_ZWSP, in a similar way as linebreaking isn't occurring on CH_SHY at the moment. 
Seems to me that IS_DISCARDED might even have to be removed/reworked in order to
fix CH_SHY too.

	rbs
	Comment 30 • 21 years ago

>   is replaced by space at the same point where IS_DISCARDED is handled
            ^^^^^^^^
BTW, that's the key difference. It is _replaced_ vs. _discarded_.

	Jungshik Shin Assignee
	Comment 31 • 21 years ago

I just found that discarding SHY (the way it's currently handled) has another
side effect (perhaps already known and filed as a bug). If I copy'n'paste a
string with SHY in the middle (say 'dis­card'), what actually gets
copied'n'pasted is different from what appears to get c&p'd. It appears that the
whole string is copy'n'pasted, but the final 'd' is dropped in the pasted
string.  (i.e instead of 'dis&#shy;card', 'dis&#shy;car' is copy'n'pasted). 

With invisible characters rendered as zero-width chars. in GFX, there's no such
problem. This is not to say that an XP solution is not to be sought, but until
we find one, we need to work on platform-specific solutions as rbs wrote.

	David Baron :dbaron:
	Comment 32 • 21 years ago

I just worry that adding lots of things like this to Windows GFX will make it
even harder than it already is to make design changes to GFX (where we already
have the burden of changing all the ports), some of which are badly needed.

	Jungshik Shin Assignee
	Comment 33 • 21 years ago

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

	Jungshik Shin Assignee
	Comment 34 • 21 years ago

Comment on attachment 131740 [details] [diff] [review]
patch v3 with non-BMP coverage 

removing obsolete r request.

	Brian Ryner (not reading)
	Comment 35 • 21 years ago

This is probably a lame question, but how is this different from the current
ignorable.x-ccmap?

The reason I'm wondering is that I've noticed in the Xft code we will try to
find a font for a &nbsp character and may actually load additional fonts if the
first font does not have the "glyph".  Is there a way we can avoid that?  Sorry
if this is the wrong bug.

	Jungshik Shin Assignee
	Comment 36 • 21 years ago

bryner, you brought up an interesting question, but i guess it's a wrong bug :-)
Characters dealt with here are zero-width, zero-visibility (unless an
yet-hypothetical CSS designation of 'render the invisible visible' is
materialized). NBSP, on the other hand, does have the width. Idealy, we'd want
both cases to be handled before they reach Gfx (chars belonging to the former
have to be stripped away while NBSP may as well be replaced by space). It seems
like the status of 'nsTextFrame' makes it a bit hard so that I'm dealing here
with the former case in each Gfx port. 

So, why don't you file a new bug to deal with NBSP (and its alikes if any)? 

	Robert Barnhardt
	Comment 37 • 20 years ago

In regard to this bug (and the subsequent patch), may I call your attention to
<a href="http://bugzilla.mozilla.org/show_bug.cgi?id=248708">bug 248708</a> and
<a href="http://bugzilla.mozilla.org/show_bug.cgi?id=245480">bug 245480</a>?

To summarize: 

1. The elimination of ignorable characters isn't quite complete, at least under
Win32: in certain circumstances, the measured width of a string containing
ignorable characters doesn't agree with the rendered width of the string,
causing problems with the rendering and selection of text.

2. For what it's worth, I question whether the lack of any visual representation
for characters that continue to exist in the underlying content is entirely
appropriate -- in any case, IMHO, but particularly in the case of form input fields.

	YAMASHITA Makoto
	Comment 38 • 19 years ago

Gfx:Mac has
nsUnicodeFontMappingMac::GetFontID(PRUnichar aChar)
maybe we can set DONT_RENDER or something and look for it at the start of *TextSegment funcs of nsUnicodeRenderingToolkit to implement this.

	YAMASHITA Makoto
	Comment 39 • 19 years ago

Please see Comment #38 for what's this doing.

	Jungshik Shin Assignee
	Comment 40 • 19 years ago

Comment on attachment 212649 [details] [diff] [review]
mac port

Thanks a lot for the patch.

>Index: nsUnicodeFontMappingMac.cpp

>@@ -48,16 +48,20 @@
> #include "nsMacUnicodeFontInfo.h"
> 
> #define BAD_FONT_NUM	-1
> #define BAD_SCRIPT 0x7f

>Index: nsUnicodeFontMappingMac.h

>+#define DONT_RENDER -2

Perhaps, 'INVISIBLE_FONT_NUM'  is better than 'DONT_RENDER'. Hmm. that's not so good either.rbs, do you have a better name? 

Anyway, can you put both in the same file? 'BAD_FONT_NUM' is defined in 4 cpp files in gfx/src/mac !! [1] If you're inclined to (although this bug is not about that), you may define it in one central place along with 'DONT_RENDER' or 'INVISIBLE_FONT_NUM'. 

BTW, does this patch cover 'ASCII codepath'? It's not very critical but control characters (0x00-0x1F and DEL) need to be turned into nothingness as well.


[1]
http://lxr.mozilla.org/seamonkey/ident?i=BAD_FONT_NUM

	YAMASHITA Makoto
	Comment 41 • 19 years ago

DONT_RENDER -> ZERO_WIDTH_FONT_NUM
defining BAD_FONT_NUM and BAD_SCRIPT at nsUnicodeFontMappingMac.h once and for all.

	YAMASHITA Makoto
	Comment 42 • 19 years ago

I forgot to mention this...
(In reply to comment #40)
> BTW, does this patch cover 'ASCII codepath'? It's not very critical but control
> characters (0x00-0x1F and DEL) need to be turned into nothingness as well.
It looks hard to implement this without introducing performance loss; the current code just passes data to QD with the primary font turned on blindly assuming the ASCII representability (bug 315199).

	Jungshik Shin Assignee
	Comment 43 • 19 years ago

(In reply to comment #42)
> I forgot to mention this...
> (In reply to comment #40)
> > BTW, does this patch cover 'ASCII codepath'? It's not very critical but control

> It looks hard to implement this without introducing performance loss; the
> current code just passes data to QD with the primary font turned on blindly

Perhaps, we can just hope that QD does the 'right thing' :-)

	rbs
	Comment 44 • 19 years ago

Comment on attachment 212725 [details] [diff] [review]
constant renaming & unification

sr=rbs, 

Go for IGNORABLE_FONT_NUM. I like that better.

	YAMASHITA Makoto
	Comment 45 • 19 years ago

JShin, could you check this in? thanks in advance.

	Jungshik Shin Assignee
	Comment 46 • 19 years ago

thanks for the patch. i've just landed it on the trunk.
to make it useful for bug 255990 and bug 9101, we need to land it in 1.8.x branch as well. let's bake it for a while before asking for branch-approval.

	Jungshik Shin Assignee
	Comment 47 • 19 years ago

An interesting test case is at http://www.crystalinks.com/z.html 
In the 2nd, 3rd and 4th paragraphs, every letter is followed by 0x00 (==> U+0000). Those 0x00 should not affect the rendering, but somehow even on Windows, letter-spacing got messed up. 

Presumably, trunk (with cairo) will have a better solution for this so that I'm making this for 1.8 branch.

	Stefan [:stefanh]
	Comment 48 • 19 years ago

This seem to have caused bug 331803 - missing modifiers in xul menuitems on mac (backing out attachment #212815 [details] [diff] [review] fixed the issue).

	YAMASHITA Makoto
	Comment 49 • 19 years ago

(In reply to comment #48)
> This seem to have caused bug 331803 - missing modifiers in xul menuitems on mac
> (backing out attachment #212815 [details] [diff] [review] [edit] fixed the issue).
> 
Apparently ignorable.x-ccmap has those modifiers. Is there more appropriate map for this bug? Or should we subtract the modifiers from gIgnorableCCMapExt by hand?

	Jungshik Shin Assignee
	Comment 50 • 19 years ago

(In reply to comment #49)

> Apparently ignorable.x-ccmap has those modifiers. Is there more appropriate map
> for this bug? Or should we subtract the modifiers from gIgnorableCCMapExt by
> hand?

What's the character in question (in terms of Unicode code point)? 
 

	Masatoshi Kimura [:emk]
	Comment 51 • 19 years ago

This may fix the problem, but I'm not sure (I have no build environment on Mac).
We should use the proper Unicode code points rather than abusing the control characters code range.
Mapping information source:
http://www.unicode.org/Public/MAPPINGS/VENDORS/APPLE/KEYBOARD.TXT

	YAMASHITA Makoto
	Comment 52 • 19 years ago

(In reply to comment #51)
> Created an attachment (id=219289) [edit]
> proposed patch
Thanks for the patch, this looks working.

	Stefan [:stefanh]
	Comment 53 • 19 years ago

(In reply to comment #52)
> (In reply to comment #51)
> > Created an attachment (id=219289) [edit]
> > proposed patch
> Thanks for the patch, this looks working.
> 

Yeah, this seems to fix the issue with the missing modifiers in xul menuitems.

	Jungshik Shin Assignee
	Comment 54 • 19 years ago

Comment on attachment 212815 [details] [diff] [review]
checkin patch (w/ IGNORABLE_FONT_NUM)

This has been baked on the trunk for three months. One regression was taken care of in bug 331803.

	Jungshik Shin Assignee
	Comment 55 • 19 years ago

Mac OS patch landed on 1.8.1 branch (I'm sorry I forgot to add r=me, sr=rbs, a=mtschrep) when landing). 

	Gordon P. Hemsley [:GPHemsley]
	Comment 56 • 10 years ago

This bug has been buried in the graveyard and has not been updated in over 5 years. It is probably safe to assume that it will never be fixed, so resolving as WONTFIX.

[Mass-change filter: graveyard-wontfix-2014-09-24]