|Summary:||CSS3: Add support for font-effect: outline.|
|Product:||WebKit||Reporter:||Nicholas Shanks <email@example.com>|
|Component:||CSS||Assignee:||Nicholas Shanks <firstname.lastname@example.org>|
|Severity:||Enhancement||CC:||email@example.com, firstname.lastname@example.org, Justin@mecham.me, email@example.com, firstname.lastname@example.org, email@example.com|
|OS:||Mac OS X 10.4|
This patch is a first pass at adding support for text-effect: outline; from the css3-fonts module. It does not support emboss or engrave effects yet. Potentially there will be the ability to choose an outline typeface from the same family, and for this reasonn the outline style bit is accompanied by a synthetic bit, but at the moment all outlines are synthetic, and the synthetic bit is never set, so only the style bit is checked. This first-pass patch also only modifies the CG path, and not ATSUI. I have been considering adding support for three new properties related to this: -webkit-font-outline-weight; -webkit-font-outline-fill-color; and -webkit-font-outline-fill-opacity: which would take the same parameters as weight, color and opacity respectivly. I should probably prefix this property with -webkit- too, but that's not done at present. Test page at URL given.
Created an attachment (id=8870) [details] Patch version 1
(From update of attachment 8870 [details]) (1) It's highly unlikely that this property will keep the same name if/when this spec is ever revived. It's really more of a text-effect than a font-effect. Your property should therefore be -webkit-font-effect and not just font-effect. I think we should suggest that the property be renamed to text-effect though, because to me this is more like text shadow than it is like other font options. (2) This is just a way you draw. It has no effect on metrics. Therefore this should not be part of the actual font information but just held separately in the style. It should not be included as one of the properties that have to be checked first either, since it really isn't a font property. (3) You should go ahead and fully parse the property, even if embossed/engraved don't do anything yet. (4) There's some crazy indentation in the patch (in cssstyleselector.cpp).
I think you should consider sending a message to firstname.lastname@example.org or email@example.com to kick off some kind of discussion, since I think the current name (font-effect) is pretty terrible.
I sent mail to the CSS WG about this property.
1) I agree completely, I just hadn't got around to changing it yet. 2) I spent a day pulling my hair out trying to figure out why I couldn't get the effect to show up, before maciej pointed out I had used "text-effect" in the page I was using to test it! I also just noticed that I have done the same in this bug's summary! HOWEVER: You are wrong to suggest that this effect would not change the metrics. Although not yet coded, preferably this feature would select a font's associated Outline style, and only simulate it when none can be found. A suitable font would have bit 3 of the fsSelection field in it's OS/2 table set; bit 3 of the QuickDraw FOND style field set; or be an AAT font with an Outline feature and possibly a variations axis controlling the outline thickness. Either of the first two would be a different font, and the last would likely change the metrics too. 3) I thought I was. Am I missing something? 4) Yeah, I didn't bother converting tabs as there was no intention to have this patch accepted. And lastly a point of my own I request that you put to w3-css-wg (I tried to subscribe but emails bounced): This property has an "engrave" option, which is not what most fonts mean, nor what I would expect, when they describe themselves as Engraved (see Academy Engraved LET, supplied with AppleWorks, for example). There should be an engrave option which looks like Academy Engraved, and another option (possibly 'embed') to describe the effect currently called 'engrave'. These would be mutually exclusive, so should be on the same property.
Created an attachment (id=8879) [details] Same patch but using -webkit-
(From update of attachment 8879 [details]) I second darin's comments. The patch should be updated to parse fully. Also, there are a lot of "unrelated" changes in this patch, which make it harder to review/land, particularly since trunk and feature-branch have changed so much recently.