229378 - get rid of duplicated codes in nsFontFreeType and nsFontPSFreeType

Status: RESOLVED WONTFIX

(Core Graveyard :: GFX, defect)

Description

[Jungshik Shin]

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.

Comment 1

[Jungshik Shin]

Attachment: a patch

.

Comment 2

[Jungshik Shin]

Comment on attachment 137960 [details] [diff] [review]
a patch

asking for r/sr.

Comment 3

[rbs]

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.

Comment 4

[Jungshik Shin]

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.

Comment 5

[Jungshik Shin]

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 6

[rbs]

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.

Comment 7

[Jungshik Shin]

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.

Comment 8

[Gordon P. Hemsley [:GPHemsley]]

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]