480249 - gfxWindowsPlatform::ResolveFontName() almost fails since direct access to mName member.

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Hiroyuki Ikezoe (:hiro)]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; ja; rv:1.8.0.4) Gecko/20060508 Firefox/3.0
Build Identifier: 

gfxPlatformGtk::ResolveFontName() almost fails since direct access to mName member.
But mName in FontFamily is always empty.

Reproducible: Always

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Attachment: Use Name() method of gfxFontFamily

I actually want to remove mName in FontFamily. It is confusable but I can't imagine the removal impcat.

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 364246 [details] [diff] [review]
Use Name() method of gfxFontFamily

this looks like a pretty innocuous change, but you'll need review from a gfx peer.

Also, this is marked as a windows mobile bug, but this is patching gtk code.  Is the platform marked incorrectly?

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Attachment: Correct patch

Arrrgh.
I post a wrong patch.

This is the right one. sorry.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Anyway, gfxFontFamily and FontFamily in gfxFT2Font.h has the same name of member variable. It is confusable.

From gfxFT2Font.h

class FontFamily : public gfxFontFamily
{
public:
    FontFamily(const nsAString& aName) :
        gfxFontFamily(aName) { }

    FontEntry *FindFontEntry(const gfxFontStyle& aFontStyle);

protected:
    virtual PRBool FindWeightsForStyle(gfxFontEntry* aFontsForWeights[], const gfxFontStyle& aFontStyle);

public:
    nsTArray<nsRefPtr<FontEntry> > mFaces;
    nsString mName; <-- The bad guy is this.
};

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 364249 [details] [diff] [review]
Correct patch

changing the review request to vlad, who owns gfx

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Attachment: Update patch [Checkin: Comment 12]

I found another direct access to a member.

Comment 7

[John Daggett (:jtd)]

Comment on attachment 365371 [details] [diff] [review]
Update patch
[Checkin: Comment 12]

looks good

Comment 8

[Brad Lassey [:blassey] (use needinfo?)]

switching the platform to linux(embedded) since this is not a windows mobile bug

Comment 9

[Hiroyuki Ikezoe (:hiro)]

No, Brad. This fix is for windows mobile. 

My first patch (wrong patch) was for linux but the updated patch is for windows mobile. As far as I investigate, Fennec uses only system font in most cases.

Comment 10

[Brad Lassey [:blassey] (use needinfo?)]

the summary still says "gfxPlatformGtk::ResolveFontName"

Comment 11

[Hiroyuki Ikezoe (:hiro)]

Oops! I am a foolish.

Comment 12

[Serge Gautherie (:sgautherie)]

Comment on attachment 365371 [details] [diff] [review]
Update patch
[Checkin: Comment 12]


http://hg.mozilla.org/mozilla-central/rev/cd149fbe6f4b