Summary: | REGRESSION: svg/W3C-SVG-1.1/fonts-elem-03-b.svg shows worse behavior on TOT | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||
Component: | SVG | Assignee: | Nikolas Zimmermann <zimmermann> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | ap, zimmermann | ||||
Priority: | P2 | ||||||
Version: | 523.x (Safari 3) | ||||||
Hardware: | Mac | ||||||
OS: | OS X 10.4 | ||||||
Attachments: |
|
Description
Eric Seidel (no email)
2007-10-28 18:40:18 PDT
This seems to be a very recent regression... maybe it came from my <font-face> support. Created attachment 18305 [details]
SVG Fonts support
Uploading SVG Fonts support follow-up patch here, as it fixes this problem.
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
Hmm, that may only effect a few header include barriers 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... Whyfore the CString.h include in SVGfonts? 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.
+ 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). (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 (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. (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. 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?
(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. Comment on attachment 18305 [details]
SVG Fonts support
Based on comments from hyatt and others, r=me
Landed in r29246. |