Closed Bug 229378 Opened 21 years ago Closed 10 years ago

get rid of duplicated codes in nsFontFreeType and nsFontPSFreeType

Categories

(Core Graveyard :: GFX, defect)

x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: jshin1987, Assigned: jshin1987)

Details

Attachments

(1 file)

33.28 KB, patch
jshin1987
: review?
roland.mainz
Details | Diff | Splinter Review
While working on bug 226288, almost identical methods are defined separately for
both nsFontFreeType and nsFontPSFreeType. (bug 226288 comment #6).
They have to be factored out to reduce the footprint and ease the maintenance.
Attached patch a patchSplinter Review
.
Comment on attachment 137960 [details] [diff] [review]
a patch

asking for r/sr.
Attachment #137960 - Flags: superreview?(rbs)
Attachment #137960 - Flags: review?(Louie.Zhao)
What about making |nsFontPSFreeType| to inherit from |nsFreeTypeFont|. Or
something along those lines, to give a chance to methods of nsFontPSFreeType
(e.g., GetBoundingMetrics) that remain unimplemented so far. Otherwise, I don't
see a significant pay off from your patch and don't mind the little duplication.
I'll see if that's feasible. Note that nsFontPSFreetype inherits nsFontPS and
nsFreetypeFont inherits nsFontGTK so it can get complicated.
BTW, I think the patch as it stands is still worthwhile at least for the easy
maintenance if not for the reduction in the code size.
I keep forgetting things (hmm.....). In bug 228911, I considered that
possibility (I haven't added any comment about that), but making
nsFontPSFreetype inherit nsFreetypeFont is not so simple as it looks because in
addition to the reason I gave in the previous comment, MathML fonts are handled
by a derived class of nsFontFreetype. It might be possible to factor out
something. When I filed this bug, I planned to use mozFT2Utils helper class as a
sorta launching pad for that.
Comment on attachment 137960 [details] [diff] [review]
a patch

Since this has been a while, I am removing it from my sr queue. Re-ask when the
reviewer is happy.
Attachment #137960 - Flags: superreview?(rbs)
Comment on attachment 137960 [details] [diff] [review]
a patch

Roland, do you care to review this? 

I think every bit of the code reduction is worth trying. Besides, as I wrote in
comment #0, it's cumbersome to make exactly the same change twice when I need
to make changes. 

I'll make sure to replace NPL in the patch with MPL.
Attachment #137960 - Flags: review?(Louie.Zhao) → review?(roland.mainz)
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: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: