Closed Bug 731222 Opened 8 years ago Closed 8 years ago

Use mozilla::unicode::GetGeneralCategory in IsPunctuationMark instead of the precompiled ccmap

Categories

(Core :: Layout: Text and Fonts, defect, minor)

defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla13

People

(Reporter: smontagu, Assigned: smontagu)

Details

Attachments

(1 file)

As a followup to bug 724826, we can use the harfbuzz unicode character routines in IsPunctuationMark and get rid of punct_marks.x-ccmap.
Attached patch PatchSplinter Review
Attachment #601279 - Flags: review?(jfkthame)
Comment on attachment 601279 [details] [diff] [review]
Patch

Review of attachment 601279 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/base/src/nsContentUtils.cpp
@@ +947,4 @@
>  
>  // static
>  bool
>  nsContentUtils::IsPunctuationMark(PRUint32 aChar)

Delighted to get rid of the ccmap file, but seeing this prompts me to think that "IsPunctuationMark" is a poor name for this method, as it returns false for characters of categories Pd and Pc, which clearly _are_ punctuation. This should've been named IsPunctuationForFirstLetter or something, to make its special-purpose nature more explicit.

(And as a result, I wouldn't be surprised if we used this in places where we really want a full IsPunctuationMark test. I feel a possible follow-up coming on...)
Attachment #601279 - Flags: review?(jfkthame) → review+
Hmm, I see your tryserver build failed on Win64. I believe you need to add
  
    DEFINES += -DHB_DONT_DEFINE_STDINT

to the makefile in content/base/src in order to include hb-common.h safely.
Checked in with changes from comments: 
https://hg.mozilla.org/integration/mozilla-inbound/rev/7d4c7ca828a2
Target Milestone: --- → mozilla13
https://hg.mozilla.org/mozilla-central/rev/7d4c7ca828a2
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.