Bug 15741 – REGRESSION: svg/W3C-SVG-1.1/fonts-elem-03-b.svg shows worse behavior on TOT

• Component: SVG
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

svg/W3C-SVG-1.1/fonts-elem-03-b.svg
has never been correct, and wont' be until we implement SVG fonts, but at least the font size used to be correct before. Now it just spews about how it couldn't create a font.  We should recognize .svg urls and not try to load them using the standard font-face stuff.

ERROR: unable to initialize with font (null) at not known
(/Stuff/Projects/WebKit/WebCore/platform/mac/FontDataMac.mm:147 void WebCore::FontData::platformInit())
ERROR: Corrupt font detected, using (null) in place of (null) located at "not known".
(/Stuff/Projects/WebKit/WebCore/platform/mac/FontDataMac.mm:154 void WebCore::FontData::platformInit())
ERROR: failed to set up font, using system font ?;}?
(/Stuff/Projects/WebKit/WebCore/platform/mac/FontDataMac.mm:161 void WebCore::FontData::platformInit())
ERROR: unable to initialize with font (null) at not known
(/Stuff/Projects/WebKit/WebCore/platform/mac/FontDataMac.mm:147 void WebCore::FontData::platformInit())
ERROR: Corrupt font detected, using (null) in place of (null) located at "not known".
(/Stuff/Projects/WebKit/WebCore/platform/mac/FontDataMac.mm:154 void WebCore::FontData::platformInit())
ERROR: failed to set up font, using system font ?;}??i??
(/Stuff/Projects/WebKit/WebCore/platform/mac/FontDataMac.mm:161 void WebCore::FontData::platformInit())
ERROR: unable to initialize with font (null) at not known
(/Stuff/Projects/WebKit/WebCore/platform/mac/FontDataMac.mm:147 void WebCore::FontData::platformInit())
ERROR: Corrupt font detected, using (null) in place of (null) located at "not known".
(/Stuff/Projects/WebKit/WebCore/platform/mac/FontDataMac.mm:154 void WebCore::FontData::platformInit())
ERROR: failed to set up font, using system font ?;}??i??
(/Stuff/Projects/WebKit/WebCore/platform/mac/FontDataMac.mm:161 void WebCore::FontData::platformInit())

[Eric Seidel (no email)]

This seems to be a very recent regression... maybe it came from my <font-face> support.

[Nikolas Zimmermann]

Created attachment 18305 [details]
SVG Fonts support

Uploading SVG Fonts support follow-up patch here, as it fixes this problem.

[Oliver Hunt]

Comment on attachment 18305 [details]
SVG Fonts support

Leave the ENABLE(SVG_FONTS) checks in, and just change the build-webkit script to have it on by default

[Oliver Hunt]

Hmm, that may only effect a few header include barriers

[Oliver Hunt]

in FontData* CSSFontSelector::getFontData there are a set of 3 "if (!face && !...)" checks -- i can't see how these are being made to only effect svg fonts...

[Oliver Hunt]

Whyfore the CString.h include in SVGfonts?

[Oliver Hunt]

Comment on attachment 18305 [details]
SVG Fonts support

Other than my prior comments this looks good to me.  However I would very much prefer it if you could get Dan to look over the arabic form-fu

Am going to do a code style check now.

[Alexey Proskuryakov]

+        virtual ~SVGGlyphElement() { }

Constructors and destructors of polymorphic classes should not be inline, as they cause significant binary size bloat otherwise (for the same reason, they should be explicitly defined).

[Nikolas Zimmermann]

(In reply to comment #7)
> (From update of attachment 18305 [details] [edit])
> Other than my prior comments this looks good to me.  However I would very much
> prefer it if you could get Dan to look over the arabic form-fu
> 
> Am going to do a code style check now.
Dan, already okayed the arabic-fu :-)

CString.h include removed.

CSSFontSelector: Indeed need to find a way to only do this for SVG fonts. hmmm

Greetings,
Niko

[Nikolas Zimmermann]

(In reply to comment #3)
> (From update of attachment 18305 [details] [edit])
> Leave the ENABLE(SVG_FONTS) checks in, and just change the build-webkit script
> to have it on by default
Yeah, I did that.

[Nikolas Zimmermann]

(In reply to comment #8)
> +        virtual ~SVGGlyphElement() { }
> 
> Constructors and destructors of polymorphic classes should not be inline, as
> they cause significant binary size bloat otherwise (for the same reason, they
> should be explicitly defined).
Interessting, didn't know that. Fixed.

[Oliver Hunt]

Comment on attachment 18305 [details]
SVG Fonts support

"CSSFontSelector: Indeed need to find a way to only do this for SVG fonts."
It causes no regressions despite this?

[Nikolas Zimmermann]

(In reply to comment #12)
> (From update of attachment 18305 [details] [edit])
> "CSSFontSelector: Indeed need to find a way to only do this for SVG fonts."
> It causes no regressions despite this?
> 
Right, no regressions.

[Oliver Hunt]

Comment on attachment 18305 [details]
SVG Fonts support

Based on comments from hyatt and others, r=me

[Nikolas Zimmermann]

Landed in r29246.