445626 - layout/reftests/first-letter/399941-9.html fails on x86_64

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

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

layout/reftests/first-letter/399941-9.html passes on x86 builds but fails on x86_64 builds (nothing shows up in the :first-letter).  In particular:

 * in the i686 Linux nightly 2008-07-16-02-mozilla-central, the test passes

 * in the x86_64 Linux nightly 2008-07-16-04-mozilla-central, no characters are styled

smontagu suggests that the problem may be in the x-ccmap code.

Comment 1

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

If I change ALU_SIZE at the top of nsCompressedCharMap.h from PR_BITS_PER_LONG to 32, the bug goes away.

Comment 2

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

(because nsContentUtils::IsPunctuationMark(0x012470) returns true instead of false)

Note that I only rebuild nsContentUtils after that change, and that was sufficient.

Comment 3

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

So I think the problem relates to an inconsistency in how the flag bits are placed between ccmapbin and the code.

Ccmaps begin with two PRUint16s of flag bits.  However, in the 64-bit versions, these are padded to take up 64 bits instead of 32 bits, I presume to ensure alignment.  However, the CCMAP_SIZE and CCMAP_FLAG macros assume that the two PRUint16s that are used for the flags are the two immediately preceding the beginning of the ccmap itself; however, ccmapbin generates the flag bits in the first and third PRUint16 units rather than the third and fourth, for the 64-bit case.

It seems like it would be simpler to leave the macros as-is and change ccmapbin, although it would produce a bigger patch.

I haven't verified that this is the problem, but it looks like the code in question doesn't work right.

Comment 4

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

Attachment: patch

I fixed ccmapbin.pl and then regenerated the three ccmap files I could find using the new script, and the commented out chunk that was already in each of those files (and could be used to regenerate them).  I needed to restore some extra comments in punct_marks.x-ccmap that weren't autogenerated.

I notice that ignorable.x-ccmap looks unused; we should probably remove it.

I think it would probably be good to get this in on the branch as well...

Comment 5

[Simon Montagu :smontagu]

Comment on attachment 330089 [details] [diff] [review]
patch

r=me if you also modify the comment at http://mxr.mozilla.org/mozilla-central/source/intl/unicharutil/tools/ccmapbin.pl#419 to say that we are adding padding instead of what it now says about 32bit integers.

It looks as if both of the ignorable*.x-ccmap files are unused and could be removed. ignorables_abjadpoints.x-ccmap is still referenced by nsSaveAsCharset.cpp, but there are no longer any callers that set attr_IgnoreIgnorables.

In view of that, I don't think we need to bother with this on the branch, unless we want to take bug 399941 on the branch.

Comment 6

[Simon Montagu :smontagu]

> It looks as if both of the ignorable*.x-ccmap files are unused and could be
> removed.

Filed bug 445877

Comment 7

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

Fixed: http://hg.mozilla.org/mozilla-central/index.cgi/rev/99410f7c6a61