[Bug 458169] implement downloadable font support on Linux

2008-11-26 Thread bugzilla-daemon 
Do not reply to this email.  You can add comments to this bug at
https://bugzilla.mozilla.org/show_bug.cgi?id=458169





--- Comment #10 from David Baron [:dbaron] [EMAIL PROTECTED]  2008-11-26 
11:44:57 PST ---
There's a tiny bit of updating needed after bug 457821 landed:
 * you'll want to adjust the reftest.list for additional reftests that landed
 * you'll want to add the assertion to your UpdateFontList

-- 
Configure bugmail: https://bugzilla.mozilla.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.

___
Fedora-fonts-bugs-list mailing list
Fedora-fonts-bugs-list@redhat.com
https://www.redhat.com/mailman/listinfo/fedora-fonts-bugs-list

[Bug 458169] implement downloadable font support on Linux

2008-11-24 Thread bugzilla-daemon 
Do not reply to this email.  You can add comments to this bug at
https://bugzilla.mozilla.org/show_bug.cgi?id=458169





--- Comment #4 from Robert O'Callahan (:roc) (Mozilla Corporation) [EMAIL 
PROTECTED]  2008-11-24 00:57:56 PST ---
Basically looks great to me.

+ * Ownership of the returned gfxFontEntry is passed to the caller,
+ * who must either AddRef() or delete.

Should these functions be changed (in a separate patch) to return
already_AddRefed?

+// Helper function to be called from InitPattern() to change the pattern
+// so that it matches the CSS style descriptors and so gets properly
+// sorted in font selection.  This also avoids synthetic style effects
+// being added by the renderer when the style of the font itself does not
+// match the descriptor provided by the author.
+void ConformPattern();

I'd like a better name here. Perhaps AdjustPatternToCSS?

+ * gfxWebFcFontEntry:

I think gfxDownloadedFcFontEntry would be a better name.

+FcPatternGetInteger(mPattern, FC_WEIGHT, 0, fontWeight);
+int cssWeight = gfxFontconfigUtils::FcWeightForBaseWeight(mWeight);
+if (cssWeight != fontWeight) {
+FcPatternDel(mPattern, FC_WEIGHT);
+FcPatternAddInteger(mPattern, FC_WEIGHT, cssWeight);
+}

Is there a reason not to do Del/Add unconditionally here? Ditto for FC_SLANT
and setting FC_FULLNAME.

+PRUint8 savedStyle = aStyle.style;
+aStyle.style = FONT_STYLE_NORMAL;
+fontEntry = static_castgfxFcFontEntry*
+(mUserFontSet-FindFontEntry(utf16Family, aStyle, needsBold));
+aStyle.style = savedStyle;

This is yuck. Can we make aStyle a const reference and just use a temporary
copy here?

+// User fonts are already filtered by slant (but not size) in
+// mUserFontSet-FindFontEntry().

Aren't you working around that by retrying FindFontEntry with
FONT_STYLE_NORMAL, in FindFontPattern?

+if (!(list-Contains(fontName))) {

Lose unnecessary parens.

+PRBool
+gfxPlatformGtk::IsFontFormatSupported(nsIURI *aFontURI, PRUint32 aFormatFlags)
+{
+// reject based on format flags
+if (aFormatFlags  (gfxUserFontSet::FLAG_FORMAT_EOT |
gfxUserFontSet::FLAG_FORMAT_SVG)) {
+return PR_FALSE;
+}

Can we avoid blacklisting and write this code to just return true for the
formats we know we can support?

jdaggett, do you want to review this too?

-- 
Configure bugmail: https://bugzilla.mozilla.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.

___
Fedora-fonts-bugs-list mailing list
Fedora-fonts-bugs-list@redhat.com
https://www.redhat.com/mailman/listinfo/fedora-fonts-bugs-list

[Bug 458169] implement downloadable font support on Linux

2008-11-24 Thread bugzilla-daemon 
Do not reply to this email.  You can add comments to this bug at
https://bugzilla.mozilla.org/show_bug.cgi?id=458169





--- Comment #5 from Karl Tomlinson (:karlt) [EMAIL PROTECTED]  2008-11-24 
13:35:53 PST ---
(In reply to comment #4)
 + * Ownership of the returned gfxFontEntry is passed to the caller,
 + * who must either AddRef() or delete.
 
 Should these functions be changed (in a separate patch) to return
 already_AddRefed?

I think so.  I find it confusing to pass around references to objects that
have a reference count of zero.

 
 +// Helper function to be called from InitPattern() to change the pattern
 +// so that it matches the CSS style descriptors and so gets properly
 +// sorted in font selection.  This also avoids synthetic style effects
 +// being added by the renderer when the style of the font itself does not
 +// match the descriptor provided by the author.
 +void ConformPattern();
 
 I'd like a better name here. Perhaps AdjustPatternToCSS?

That sounds fine.

 + * gfxWebFcFontEntry:
 
 I think gfxDownloadedFcFontEntry would be a better name.

gfxDownloadedFcFontEntry has the advantage of indicating that the font has
already been downloaded.  And I guess src:local() faces would really be web
fonts too (but not downloaded fonts) because the family and style properties
are obtained from the web (even if the font data is not).

 
 +FcPatternGetInteger(mPattern, FC_WEIGHT, 0, fontWeight);
 +int cssWeight = gfxFontconfigUtils::FcWeightForBaseWeight(mWeight);
 +if (cssWeight != fontWeight) {
 +FcPatternDel(mPattern, FC_WEIGHT);
 +FcPatternAddInteger(mPattern, FC_WEIGHT, cssWeight);
 +}
 
 Is there a reason not to do Del/Add unconditionally here? Ditto for FC_SLANT
 and setting FC_FULLNAME.

I'd like to leave these conditional.

For FC_WEIGHT, the only reason is to avoid the reallocation of memory for the
property value and the memmove back and forth of all the trailing properties
and pointers to their values.  The weight is expected to often (maybe usually)
already have the right value.

For FC_SLANT, there is the additional benefit of retaining the distinction
between italic and oblique where possible.

For FC_FULLNAME, if there is an existing value, then that is the best value as
it comes from the OpenType name table.  Appending style to family should only
be a fallback.

 +PRUint8 savedStyle = aStyle.style;
 +aStyle.style = FONT_STYLE_NORMAL;
 +fontEntry = static_castgfxFcFontEntry*
 +(mUserFontSet-FindFontEntry(utf16Family, aStyle, needsBold));
 +aStyle.style = savedStyle;
 
 This is yuck. Can we make aStyle a const reference and just use a temporary
 copy here?

Yes, this is yuck.

Constructing a gfxFontStyle always requires a memory allocation because it has
an nsCString member, which is always forced to be non-empty, even though it
doesn't get used here.

What I think would look nicest here would be to change the FindFontPattern()
gfxFontStyle argument to thebes style and weight arguments.  That would avoid
the new gfxFontStyle allocation in SortPreferredFonts, and confine all the
gfxFontStyle yuck to within FindFontPattern.

Then modifying gfxUserFontSet::FindFontEntry arguments so that only the
information actually used needs to be provided, and/or modifying gfxFontStyle
so that the nsCString member can be empty, can be considered as future
improvements.

 
 +// User fonts are already filtered by slant (but not size) in
 +// mUserFontSet-FindFontEntry().
 
 Aren't you working around that by retrying FindFontEntry with
 FONT_STYLE_NORMAL, in FindFontPattern?

SlantIsAcceptable() also accepts faces with FONT_STYLE_NORMAL/FC_SLANT_ROMAN
when the requested style is not normal/roman (as an oblique can be synthesized
from normal/roman).

 +PRBool
 +gfxPlatformGtk::IsFontFormatSupported(nsIURI *aFontURI, PRUint32 
 aFormatFlags)
 +{
 +// reject based on format flags
 +if (aFormatFlags  (gfxUserFontSet::FLAG_FORMAT_EOT |
 gfxUserFontSet::FLAG_FORMAT_SVG)) {
 +return PR_FALSE;
 +}
 
 Can we avoid blacklisting and write this code to just return true for the
 formats we know we can support?

Maybe.

The editor's draft http://dev.w3.org/csswg/css3-fonts/#font-reference and the
2002 working draft http://www.w3.org/TR/css3-webfonts/#src currently suggests
returning true only for formats we know we can support: The user agent will
recognize the name of font formats that it supports, and will avoid
downloading fonts in formats that it does not recognize.

This code was copied from the code for Mac and Windows, so I suggest
considering making that change to all platforms in a separate bug, probably
bug 465452.

-- 
Configure bugmail: https://bugzilla.mozilla.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.

___
Fedora-fonts-bugs-list

[Bug 458169] implement downloadable font support on Linux

2008-11-24 Thread bugzilla-daemon 
Do not reply to this email.  You can add comments to this bug at
https://bugzilla.mozilla.org/show_bug.cgi?id=458169





--- Comment #6 from Robert O'Callahan (:roc) (Mozilla Corporation) [EMAIL 
PROTECTED]  2008-11-24 13:40:16 PST ---
(In reply to comment #5)
  Should these functions be changed (in a separate patch) to return
  already_AddRefed?
 
 I think so.  I find it confusing to pass around references to objects that
 have a reference count of zero.

Please file a bug on that.

 I'd like to leave these conditional.
 
 For FC_WEIGHT, the only reason is to avoid the reallocation of memory for the
 property value and the memmove back and forth of all the trailing properties
 and pointers to their values.  The weight is expected to often (maybe usually)
 already have the right value.
 
 For FC_SLANT, there is the additional benefit of retaining the distinction
 between italic and oblique where possible.
 
 For FC_FULLNAME, if there is an existing value, then that is the best value as
 it comes from the OpenType name table.  Appending style to family should only
 be a fallback.

OK.

  +PRUint8 savedStyle = aStyle.style;
  +aStyle.style = FONT_STYLE_NORMAL;
  +fontEntry = static_castgfxFcFontEntry*
  +(mUserFontSet-FindFontEntry(utf16Family, aStyle, needsBold));
  +aStyle.style = savedStyle;
  
  This is yuck. Can we make aStyle a const reference and just use a temporary
  copy here?
 
 Yes, this is yuck.
 
 Constructing a gfxFontStyle always requires a memory allocation because it has
 an nsCString member, which is always forced to be non-empty, even though it
 doesn't get used here.
 
 What I think would look nicest here would be to change the FindFontPattern()
 gfxFontStyle argument to thebes style and weight arguments.  That would avoid
 the new gfxFontStyle allocation in SortPreferredFonts, and confine all the
 gfxFontStyle yuck to within FindFontPattern.
 
 Then modifying gfxUserFontSet::FindFontEntry arguments so that only the
 information actually used needs to be provided, and/or modifying gfxFontStyle
 so that the nsCString member can be empty, can be considered as future
 improvements.

OK

  +// User fonts are already filtered by slant (but not size) in
  +// mUserFontSet-FindFontEntry().
  
  Aren't you working around that by retrying FindFontEntry with
  FONT_STYLE_NORMAL, in FindFontPattern?
 
 SlantIsAcceptable() also accepts faces with FONT_STYLE_NORMAL/FC_SLANT_ROMAN
 when the requested style is not normal/roman (as an oblique can be synthesized
 from normal/roman).

OK

 This code was copied from the code for Mac and Windows, so I suggest
 considering making that change to all platforms in a separate bug, probably
 bug 465452.

OK

-- 
Configure bugmail: https://bugzilla.mozilla.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.

___
Fedora-fonts-bugs-list mailing list
Fedora-fonts-bugs-list@redhat.com
https://www.redhat.com/mailman/listinfo/fedora-fonts-bugs-list

[Bug 458169] implement downloadable font support on Linux

2008-11-24 Thread bugzilla-daemon 
Do not reply to this email.  You can add comments to this bug at
https://bugzilla.mozilla.org/show_bug.cgi?id=458169





--- Comment #7 from John Daggett (:jtd) [EMAIL PROTECTED]  2008-11-24 
16:53:43 PST ---
(In reply to comment #3)
  A reference to the nsIStreamLoader is held to keep the font data in memory
  until the font entry is destroyed.
 
 The nsIStreamLoader* arg could be an nsISupports*, I assume.
 Let me know if you prefer that.

I think that seems better, it would eliminate the need for gfx to link against
necko, right?

-- 
Configure bugmail: https://bugzilla.mozilla.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.

___
Fedora-fonts-bugs-list mailing list
Fedora-fonts-bugs-list@redhat.com
https://www.redhat.com/mailman/listinfo/fedora-fonts-bugs-list

[Bug 458169] implement downloadable font support on Linux

2008-11-24 Thread bugzilla-daemon 
Do not reply to this email.  You can add comments to this bug at
https://bugzilla.mozilla.org/show_bug.cgi?id=458169





--- Comment #8 from Karl Tomlinson (:karlt) [EMAIL PROTECTED]  2008-11-24 
17:18:31 PST ---
(In reply to comment #7)
 I think that seems better, it would eliminate the need for gfx to link against
 necko, right?

Thanks to XPCOM (or virtual functions) gfx doesn't need to link against necko.

It would just save including nsIStreamLoader.h in gfxPangoFonts.h and adding
necko to the include path for gfx.

-- 
Configure bugmail: https://bugzilla.mozilla.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.

___
Fedora-fonts-bugs-list mailing list
Fedora-fonts-bugs-list@redhat.com
https://www.redhat.com/mailman/listinfo/fedora-fonts-bugs-list

[Bug 458169] implement downloadable font support on Linux

2008-11-24 Thread bugzilla-daemon 
Do not reply to this email.  You can add comments to this bug at
https://bugzilla.mozilla.org/show_bug.cgi?id=458169





--- Comment #9 from Karl Tomlinson (:karlt) [EMAIL PROTECTED]  2008-11-24 
17:19:07 PST ---
(In reply to comment #8)
 It would just save including nsIStreamLoader.h in gfxPangoFonts.h and

Sorry, in gfxPangoFonts.cpp.

-- 
Configure bugmail: https://bugzilla.mozilla.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.

___
Fedora-fonts-bugs-list mailing list
Fedora-fonts-bugs-list@redhat.com
https://www.redhat.com/mailman/listinfo/fedora-fonts-bugs-list

[Bug 458169] implement downloadable font support on Linux

2008-11-23 Thread bugzilla-daemon 
Do not reply to this email.  You can add comments to this bug at
https://bugzilla.mozilla.org/show_bug.cgi?id=458169


Karl Tomlinson (:karlt) [EMAIL PROTECTED] changed:

   What|Removed |Added

 Attachment #348692|0   |1
is obsolete||
 Attachment #349707||review?([EMAIL PROTECTED])
   Flag||




--- Comment #3 from Karl Tomlinson (:karlt) [EMAIL PROTECTED]  2008-11-23 
21:47:11 PST ---
Created an attachment (id=349707)
 -- (https://bugzilla.mozilla.org/attachment.cgi?id=349707)
src:url() v1.0

This version works even with older versions of fontconfig.
A build is available here:

https://build.mozilla.org/tryserver-builds/2008-11-23_20:[EMAIL PROTECTED]/

Note that this doesn't support src:local(), which I intend to do using a
different implementation of gfxFcFontEntry.

(In reply to comment #2)
 A reference to the nsIStreamLoader is held to keep the font data in memory
 until the font entry is destroyed.

The nsIStreamLoader* arg could be an nsISupports*, I assume.
Let me know if you prefer that.

-- 
Configure bugmail: https://bugzilla.mozilla.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.

___
Fedora-fonts-bugs-list mailing list
Fedora-fonts-bugs-list@redhat.com
https://www.redhat.com/mailman/listinfo/fedora-fonts-bugs-list

[Bug 458169] implement downloadable font support on Linux

2008-11-08 Thread bugzilla-daemon 
Do not reply to this email.  You can add comments to this bug at
https://bugzilla.mozilla.org/show_bug.cgi?id=458169





--- Comment #1 from David Baron [:dbaron] [EMAIL PROTECTED]  2008-11-08 
15:16:07 PST ---
When this lands, you'll need to adjust the fails-if and skip-if markers in
http://hg.mozilla.org/mozilla-central/file/tip/layout/reftests/font-face/reftest.list

-- 
Configure bugmail: https://bugzilla.mozilla.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.

___
Fedora-fonts-bugs-list mailing list
Fedora-fonts-bugs-list@redhat.com
https://www.redhat.com/mailman/listinfo/fedora-fonts-bugs-list

[Bug 458169] implement downloadable font support on Linux

2008-11-06 Thread bugzilla-daemon 
Do not reply to this email.  You can add comments to this bug at
https://bugzilla.mozilla.org/show_bug.cgi?id=458169


Bug 458169 depends on bug 449356, which changed state.

Bug 449356 Summary: Refactor gfxPangoFontGroup for user fonts
https://bugzilla.mozilla.org/show_bug.cgi?id=449356

   What|Old Value   |New Value

 Status|ASSIGNED|RESOLVED
 Resolution||FIXED



-- 
Configure bugmail: https://bugzilla.mozilla.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.

___
Fedora-fonts-bugs-list mailing list
Fedora-fonts-bugs-list@redhat.com
https://www.redhat.com/mailman/listinfo/fedora-fonts-bugs-list

[Bug 458169] implement downloadable font support on Linux

2008-10-06 Thread bugzilla-daemon 
Do not reply to this email.  You can add comments to this bug at
https://bugzilla.mozilla.org/show_bug.cgi?id=458169


Karl Tomlinson (:karlt) [EMAIL PROTECTED] changed:

   What|Removed |Added

 AssignedTo|[EMAIL PROTECTED]  |[EMAIL PROTECTED]
   Target Milestone|--- |mozilla1.9.1




-- 
Configure bugmail: https://bugzilla.mozilla.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.

___
Fedora-fonts-bugs-list mailing list
Fedora-fonts-bugs-list@redhat.com
https://www.redhat.com/mailman/listinfo/fedora-fonts-bugs-list

[Bug 458169] implement downloadable font support on Linux

2008-10-04 Thread bugzilla-daemon 
Do not reply to this email.  You can add comments to this bug at
https://bugzilla.mozilla.org/show_bug.cgi?id=458169


Bill Gianopoulos [EMAIL PROTECTED] changed:

   What|Removed |Added

 CC||[EMAIL PROTECTED]
 Blocks||410460




-- 
Configure bugmail: https://bugzilla.mozilla.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.

___
Fedora-fonts-bugs-list mailing list
Fedora-fonts-bugs-list@redhat.com
https://www.redhat.com/mailman/listinfo/fedora-fonts-bugs-list

[Bug 458169] implement downloadable font support on Linux

2008-10-02 Thread bugzilla-daemon 
Do not reply to this email.  You can add comments to this bug at
https://bugzilla.mozilla.org/show_bug.cgi?id=458169


John Daggett (:jtd) [EMAIL PROTECTED] changed:

   What|Removed |Added

   Priority|--  |P1




-- 
Configure bugmail: https://bugzilla.mozilla.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.

___
Fedora-fonts-bugs-list mailing list
Fedora-fonts-bugs-list@redhat.com
https://www.redhat.com/mailman/listinfo/fedora-fonts-bugs-list

[Bug 458169] implement downloadable font support on Linux

2008-10-02 Thread bugzilla-daemon 
Do not reply to this email.  You can add comments to this bug at
https://bugzilla.mozilla.org/show_bug.cgi?id=458169


Vladimir Vukicevic (:vlad) [EMAIL PROTECTED] changed:

   What|Removed |Added

   Priority|P1  |P2
   Flag|blocking1.9.1?  |blocking1.9.1+




-- 
Configure bugmail: https://bugzilla.mozilla.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.

___
Fedora-fonts-bugs-list mailing list
Fedora-fonts-bugs-list@redhat.com
https://www.redhat.com/mailman/listinfo/fedora-fonts-bugs-list