need to exclude truly invisible default_ignorable_code_point's from draw_string

RESOLVED WONTFIX

Status

P3
normal
RESOLVED WONTFIX
16 years ago
4 years ago

People

(Reporter: jshin1987, Assigned: jshin1987)

Tracking

(Depends on: 1 bug, Blocks: 1 bug, {fixed1.8.1, intl})

1.8 Branch
mozilla1.6alpha
fixed1.8.1, intl
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments, 4 obsolete attachments)

(Assignee)

Description

16 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.
(Assignee)

Comment 1

16 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'. 

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.

Comment 3

16 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...
(Assignee)

Comment 4

16 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/﷒0
Summary: need to exclude default_ignorable_code_point's from draw_string → need to exclude truly invisible default_ignorable_code_point's from draw_string

Comment 5

16 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.

Comment 6

16 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.

Comment 7

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

Updated

16 years ago
Depends on: 204993

Updated

16 years ago
Blocks: 167434
-> smontagu
Assignee: kmcclusk → smontagu
Priority: -- → P3
Target Milestone: --- → Future

Comment 9

16 years ago
*** Bug 213930 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 10

15 years ago
Created attachment 131669 [details] [diff] [review]
a patch for GFX:Win

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?
(Assignee)

Comment 11

15 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? 



(Assignee)

Comment 12

15 years ago
Created attachment 131679 [details] [diff] [review]
patch v2 (for GFX:Win) 

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

Updated

15 years ago
Attachment #131669 - Attachment is obsolete: true

Comment 13

15 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?
(Assignee)

Comment 14

15 years ago
Created attachment 131740 [details] [diff] [review]
patch v3 with non-BMP coverage 

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.
(Assignee)

Updated

15 years ago
Attachment #131679 - Attachment is obsolete: true
(Assignee)

Comment 15

15 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.
Attachment #131740 - Flags: superreview?(rbs)
Attachment #131740 - Flags: review?(smontagu)
(Assignee)

Comment 16

15 years ago
assigning to myself and setting the target milestone
Assignee: smontagu → jshin
Keywords: intl
Target Milestone: Future → mozilla1.6alpha
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.
(Assignee)

Comment 18

15 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.
 
Status: NEW → ASSIGNED

Comment 19

15 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
Attachment #131740 - Flags: superreview?(rbs) → superreview+
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"
(Assignee)

Comment 21

15 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/﷒0> "blackhole" becoming a verb

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

Comment 22

15 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
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.)
(Assignee)

Comment 24

15 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 

Comment 25

15 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_ &shy; problem, it seems
fine to me to have this fix, while keeping open the possibility of a common fix.
Which &shy; problem?  The fact that we don't support it?  (The main problem is
that our line-breaking algorithm is extremely simple, but beyond that, &shy;
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?

Comment 27

15 years ago
> Which &shy; problem?

By the &shy; 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 &shy; 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 &nbsp;) and not rendered unless linebreaking occurs.]
Did you actually test that that's the case?  Considering that &nbsp; 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.

Comment 29

15 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. &InvisibleComma; 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.

Comment 30

15 years ago
> &nbsp; is replaced by space at the same point where IS_DISCARDED is handled
            ^^^^^^^^
BTW, that's the key difference. It is _replaced_ vs. _discarded_.
(Assignee)

Comment 31

15 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&#xad;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.
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.
(Assignee)

Comment 33

15 years ago
*** Bug 105145 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 34

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

removing obsolete r request.
Attachment #131740 - Flags: review?(smontagu)
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.
(Assignee)

Comment 36

15 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)? 

(Assignee)

Updated

15 years ago
Blocks: 251291

Comment 37

15 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.

Updated

14 years ago
Blocks: 118589

Updated

14 years ago
Blocks: 285718

Comment 38

13 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.

Comment 39

13 years ago
Created attachment 212649 [details] [diff] [review]
mac port

Please see Comment #38 for what's this doing.
Attachment #212649 - Flags: review?(jshin1987)
(Assignee)

Comment 40

13 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
Attachment #212649 - Flags: review?(jshin1987) → review+

Comment 41

13 years ago
Created attachment 212725 [details] [diff] [review]
constant renaming & unification

DONT_RENDER -> ZERO_WIDTH_FONT_NUM
defining BAD_FONT_NUM and BAD_SCRIPT at nsUnicodeFontMappingMac.h once and for all.
Attachment #212649 - Attachment is obsolete: true
Attachment #212725 - Flags: superreview?
Attachment #212725 - Flags: review+

Updated

13 years ago
Attachment #212725 - Flags: superreview? → superreview?(rbs)

Comment 42

13 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).
(Assignee)

Comment 43

13 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' :-)

Comment 44

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

sr=rbs, 

Go for IGNORABLE_FONT_NUM. I like that better.
Attachment #212725 - Flags: superreview?(rbs) → superreview+

Comment 45

13 years ago
Created attachment 212815 [details] [diff] [review]
checkin patch (w/ IGNORABLE_FONT_NUM)

JShin, could you check this in? thanks in advance.
Attachment #212725 - Attachment is obsolete: true
(Assignee)

Comment 46

13 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.
(Assignee)

Comment 47

13 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.
Version: Trunk → 1.8 Branch

Updated

13 years ago
Blocks: 331803

Comment 48

13 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).

Comment 49

13 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?
(Assignee)

Comment 50

13 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)? 
 
Created attachment 219289 [details] [diff] [review]
proposed patch

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

Comment 52

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

Comment 53

13 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.

Updated

13 years ago
No longer blocks: 331803
(Assignee)

Comment 54

13 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.
Attachment #212815 - Flags: superreview+
Attachment #212815 - Flags: review+
Attachment #212815 - Flags: approval-branch-1.8.1?

Updated

13 years ago
Attachment #212815 - Flags: approval-branch-1.8.1? → approval1.8.1?

Updated

13 years ago
Attachment #212815 - Flags: approval1.8.1? → approval1.8.1+
(Assignee)

Comment 55

13 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). 
Keywords: fixed1.8.1
Product: Core → Core Graveyard
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]
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.