Closed Bug 511339 Opened 15 years ago Closed 14 years ago

implement experimental support for -moz-font-feature-settings

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jtd, Assigned: jfkthame)

References

(Depends on 1 open bug)

Details

(Keywords: dev-doc-complete)

Attachments

(5 files, 8 obsolete files)

(deleted), text/html
Details
(deleted), text/html
Details
(deleted), patch
dbaron
: review+
Details | Diff | Splinter Review
(deleted), patch
jtd
: review+
Details | Diff | Splinter Review
(deleted), patch
jtd
: review+
Details | Diff | Splinter Review
Jonathan and I put together a proposal for supporting OpenType font features in CSS: http://lists.w3.org/Archives/Public/www-style/2009Jun/0506.html In a nutshell, font features affect glyph selection and/or positioning without changing the font used. The proposal above is for a set of font-variant-xxx properties that are turned into OpenType/AAT features within text rendering code. As a simple way of experimenting with OpenType features directly, add an experimental CSS property -moz-font-feature-opentype with the syntax below: -moz-font-feature-opentype: auto | <feature-string> ; Here, 'auto' means use font defaults and is the initial value. The <feature-string> would be a sequence of tags and values: -moz-font-feature-opentype: "liga=0, salt=2, xyz1=1"; Each of the 4-character tags are OpenType feature tags, as defined here: http://www.microsoft.com/opentype/otspec/featurelist.htm The order of the tags would not affect rendering since OT features are processed in the order they occur within font data. I'm assuming that the string is opaque to style code and gets passed down within a gfxFontStyle object associated with a run of text. We can then hook this up to harfbuzz (bug 449292) or rig a quick hack on Vista/Win7 to render using the Uniscribe calls that allow OT features to be specified explicitly. ScriptShapeOpenType http://msdn.microsoft.com/en-us/library/dd368565%28VS.85%29.aspx Note: I'm not advocating this as a general CSS property, although some would see utility in it. This is just a convenient way of experimenting with OT features within the browser.
Assignee: nobody → jdaggett
First pass at implementing -moz-font-feature-opentype. Parses the CSS and sticks the value into nsCSSFont, which uses it to set the value in nsFont, which puts it into the gfxFontStyle. From gfxFontStyle, font groups can parse the OpenType feature list and pass the values to harfbuzz/Uniscribe/whatever. Not the ideal way to do this but it's enough for experimentation. I've added logging code in gfxAtsuiFontGroup to dump the opentype feature list when text run logging is enabled: export NSPR_LOG_MODULES=atsuiTextRun:5
Set logging, run FF, open testcase, output below will appear: InitTextRun 1d358b10 fontgroup 1d358aa0 (serif) lang: x-western len 10 opentype: liga=0, salt=2, xyz1=1 TEXTRUN "Great fun!" ENDTEXTRUN
Comment on attachment 395544 [details] [diff] [review] patch, initial implementation of -moz-font-feature-opentype You'll also want to add to layout/style/test/property_database.js and then make sure all the mochitests in layout/style pass. > PRBool > CSSParserImpl::ParseFont() ... >+ // hmmm, think i need to reset font-feature-opentype here, not sure how... Just add an extra AppendValue call in the three obvious-looking places in that function.
WIP/proof-of-concept. This patch enables CSS control of features through the HarfBuzz layout engine, using the patches from bug 449292 (which in turn depend on bug 502906).
Javascript get/set methods work but the syntax is a tad funky, I probably need to tweak something. The setter requires an extra nested quote, I'm guessing this is because the setter is essentially passing the value to the parser and the parser in this case is looking for quotes. var el = document.getElementById("main"); el.style.MozFontFeatureOpentype = "'burp=1'"; This will cause the string "burp=1" to end up in the opentypeFeatures field of gfxFontStyle.
Cool .... yes, I think it was the quotes that were confusing me. Works fine now. I'm attaching a sample file that I'm using to experiment with feature settings.
This based on John's original patch, but updated in several ways: - rename the property to -moz-font-feature-settings (following the current CSS3 Fonts editor's draft) - store features as an array of tag+value pairs rather than just passing the string down to gfx - also implement -moz-font-language-override (see CSS3 Fonts draft) This seems to pass the style mochitests successfully; however, I'm not sure whether something is still needed in CSSParserImpl::ParseFont (see comments there). I tried adding what seemed to me like the "obvious" extra AppendValue calls, but this led to lots of assertion spew.... clearly, I don't really understand this code.
Attachment #395544 - Attachment is obsolete: true
Attachment #395661 - Attachment is obsolete: true
This is dependent on the harfbuzz patches from bug 449292.
Comment on attachment 442941 [details] [diff] [review] part 1 - implement -moz-font-feature-settings and -moz-font-language-override in CSS >diff --git a/dom/interfaces/css/nsIDOMNSCSS2Properties.idl b/dom/interfaces/css/nsIDOMNSCSS2Properties.idl >--- a/dom/interfaces/css/nsIDOMNSCSS2Properties.idl >+++ b/dom/interfaces/css/nsIDOMNSCSS2Properties.idl >@@ -116,16 +116,22 @@ interface nsIDOMNSCSS2Properties : nsIDO You need to rev the IID of nsIDOMNSCSS2Properties. >diff --git a/layout/style/nsCSSParser.cpp b/layout/style/nsCSSParser.cpp >--- a/layout/style/nsCSSParser.cpp >+++ b/layout/style/nsCSSParser.cpp So, in order to help you with your question about the parser, the first thing I looked at was nsStyleStruct.cpp - the constructor of nsStyleFont - to see what the initial values of these new properties are. And as far as I can tell, featureSettings's initial value is null (from the nsTArray constructor called by the nsFont constructor), which seems OK, but languageOverride looks like it's uninitialized in nsStyleFont's constructor (actually 2 constructors; the copy-constructor is ok, though). I think you also need to change the two nsFont constructors that initialize other things to initialize languageSettings as well; I think you probably also want to add parameters to those constructors to catch places that need to pass them through. (You fixed the copy-constructor, and there's one constructor that's intended to leave things uninitialized.) In nsRuleNode.cpp, for both properties, you have handling for a 'none' value that you don't handle in the parsing code; you should just remove that (and handle just eCSSUnit_Initial and eCSSUnit_Normal). Should there be a named constant to represent that 0 means no language override? nsFont.h and gfxFont.h should probably say what values the languageOverride member takes (e.g., have a comment pointing to a header file with the appropriate comments). In any case, now that I'm past the things I noticed trying to figure out the initial values of the property, back to your question: >@@ -7441,28 +7445,34 @@ CSSParserImpl::ParseFont() > AppendValue(eCSSProperty_font_family, family); > AppendValue(eCSSProperty_font_style, family); > AppendValue(eCSSProperty_font_variant, family); > AppendValue(eCSSProperty_font_weight, family); > AppendValue(eCSSProperty_font_size, family); > AppendValue(eCSSProperty_line_height, family); > AppendValue(eCSSProperty_font_stretch, family); > AppendValue(eCSSProperty_font_size_adjust, family); >+ // Do we need to do something here with these? >+ // eCSSProperty_font_feature_settings >+ // eCSSProperty_font_language_override I think doing something here is desirable since I think these new properties probably should be reset by the 'font' shorthand. Here you can just make the exact same AppendValue calls and it shouldn't cause any problems. This is handling the case of 'font:inherit' or 'font:-moz-initial', which set all the subproperties of these properties to 0. > else { > AppendValue(eCSSProperty__x_system_font, family); > nsCSSValue systemFont(eCSSUnit_System_Font); > AppendValue(eCSSProperty_font_family, systemFont); > AppendValue(eCSSProperty_font_style, systemFont); > AppendValue(eCSSProperty_font_variant, systemFont); > AppendValue(eCSSProperty_font_weight, systemFont); > AppendValue(eCSSProperty_font_size, systemFont); > AppendValue(eCSSProperty_line_height, systemFont); > AppendValue(eCSSProperty_font_stretch, systemFont); > AppendValue(eCSSProperty_font_size_adjust, systemFont); >+ // Do we need to do something here with these? >+ // eCSSProperty_font_feature_settings >+ // eCSSProperty_font_language_override Here you have two options. You could either: * append an eCSSUnit_Normal value, or * append an eCSSUnit_System_Font value I think the latter is probably the better choice, but it means that you need to add handling for eCSSUnit_System_Font in nsRuleNode.cpp (which was probably the cause of the assertions). The reason for these values is that system fonts are single values for the font shorthand, such as: font: menu; but it's legal to write: font: menu; font-style: italic; which means that you should use all aspects of the 'menu' system font, except override the font-style to make it italic. As I said, I think making these new properties be reset by the 'font' shorthand does seem like the right thing to me. However, to do it correctly, you also need to: * adjust gFontSubpropTable in nsCSSProps.cpp * adjust the serialization code for case eCSSProperty_font in nsCSSDeclaration::GetValue * adjust the list of subproperties for "font" in property_database.js >@@ -7514,16 +7524,19 @@ CSSParserImpl::ParseFont() > AppendValue(eCSSProperty_font_style, values[0]); > AppendValue(eCSSProperty_font_variant, values[1]); > AppendValue(eCSSProperty_font_weight, values[2]); > AppendValue(eCSSProperty_font_size, size); > AppendValue(eCSSProperty_line_height, lineHeight); > AppendValue(eCSSProperty_font_stretch, > nsCSSValue(NS_FONT_STRETCH_NORMAL, eCSSUnit_Enumerated)); > AppendValue(eCSSProperty_font_size_adjust, nsCSSValue(eCSSUnit_None)); >+ // Do we need to do something here with these? >+ // eCSSProperty_font_feature_settings >+ // eCSSProperty_font_language_override And here, to make these properties be reset by the font shorthand, you'd want to append an eCSSUnit_Normal value for both properties. I haven't looked through the whole patch yet; just the parts relevant to your question about the parser.
The main cause of the assertions was probably the mismatch between gFontSubpropTable and what the parsing code was doing; the lack of eCSSUnit_System_Font support in nsRuleNode might have been a cause of a few additional ones.
Thanks for the comments, dbaron; very helpful. I think this is a lot closer to being correct now.
Attachment #442941 - Attachment is obsolete: true
Attachment #442942 - Attachment is obsolete: true
Attachment #445111 - Flags: review?(dbaron)
Comment on attachment 445111 [details] [diff] [review] part 1, v2 - implement -moz-font-feature-settings and -moz-font-language-override in CSS Is it really safe to assume all language tags in this context are <= 4 characters? Are there relevant rules that forbid using zh-Hans, zh-Hant, etc.? I'm not a big fan of default parameters in cases like the way you use them in the nsFont constructor, since they tend to hide places that need to be changed. But I suppose they're ok in this case (especially since those constructors already use them). gfxFont.h: >+inline PRBool >+operator==(const gfxFontFeature& a, const gfxFontFeature& b) >+{ >+ return (a.mTag == b.mTag) && (a.mValue == b.mValue); >+} I think you don't need to write this since the operator== implicitly generated by the compiler is fine. (If you want to leave a comment that says // implicit operator==(const gfxFontFeature&, const gfxFontFeature&) // is correct that's fine, though. I think gfxFontStyle could perhaps use some comments as to how language and languageOverride relate to each other. Do we really need both? nsCSSDeclaration.cpp: >- // The font-stretch and font-size-adjust >+ // The font-stretch, font-size-adjust >+ // -moz-font-feature-settings and -moz-font-language-override commas after font-size-adjust and -moz-font-feature-settings, please. nsCSSParser.cpp: >+ case eCSSProperty_font_feature_settings: >+ return ParseVariant(aValue, VARIANT_NORMAL | VARIANT_INHERIT | VARIANT_STRING, nsnull); >+ case eCSSProperty_font_language_override: >+ return ParseVariant(aValue, VARIANT_NORMAL | VARIANT_INHERIT | VARIANT_STRING, nsnull); Please drop the first "return" line and just have both cases share the code. Also, please wrap at less than 80 characters (i.e., break after the last | and align VARIANT_STRING with VARIANT_NORMAL). nsComputedDOMStyle.cpp: AppendTagToString isn't quite right in terms of escaping. I think should instead just append the four characters to a string without any escaping, and its callers should then be responsible for passing the result to nsStyleUtil::AppendEscapedCSSString, which does the correct escaping for a CSS string (and also places the quote marks at the start and end). (AppendEscapedCSSString has to be done only once per string, so you can't put it inside AppendTagToString; but see paragraph after next.) Please add tests to property_database.js for things that were broken without this. (I'm also a bit concerned about the feature settings property being a single string rather than a comma-separated list of strings. It just feels odd to have a list all stored *inside* a string.) nsRuleNode.cpp: For both properties, it's probably better to use nsCSSValue::GetStringBufferValue than nsCSSValue::GetStringValue. If you want to use string APIs on the value, wrap it in an nsDependentString rather than copying. For both properties, I think your case for eCSSUnit_SystemFont should be separate from normal and initial: it should assign the featureSettings/languageOverride from systemFont.featureSettings/languageOverride. This would mean that if any of the system font backends start filling them in (which I could certainly see them doing for languageOverride), they'd get used. + if (!nsCRT::IsAscii(ch)) { // valid tags are pure ASCII + return NO_FONT_LANGUAGE_OVERRIDE; + } This doesn't seem very CSS-ish. If a value is invalid, it should be dropped by the parser. (This is part of CSS's forward-compatibility mechanism: it allows new values to fill the space unused by old ones in a way that lets authors use multiple declarations as fallback for older implementations.) The way one would normally accomplish that is by having the parsing code live in the parser (not in nsRuleNode), which turns the string into some useful data structure that's stored in the rule (i.e., an nsCSSValue, nsCSSValuePair, nsCSSValueList, or nsCSSValuePairList). (This also means nsCSSDeclaration needs to know how to serialize it.) Same (but with more complications) for font feature settings. I'm not sure whether I'd insist on this change, though... property_database.js: should probably have examples of invalid values, at least the same as the valid one but unquoted. I presume the nsChildView.mm changes and the nsTArray changes weren't really intended to be part of this patch. This looks good otherwise, though.
Attachment #445111 - Flags: review?(dbaron) → review-
(In reply to comment #13) > (From update of attachment 445111 [details] [diff] [review]) > Is it really safe to assume all language tags in this context are <= 4 > characters? Are there relevant rules that forbid using zh-Hans, > zh-Hant, etc.? For -moz-font-language-override, the tags are not ISO or BCP 47 language tags but OpenType language system tags. These are a separate collection without a simple or unambiguous mapping to standard language tags; they represent something more like "a particular set of orthographic conventions for a writing system". (And hence the need to sometimes override the default that can be inferred from the document or element's language.) The OpenType language tags are defined to be <= 4 characters (they're treated as 4-byte integers within OpenType; they just happen to be integers that, when interpreted as 4 ASCII bytes in big-endian order, look like mnemonic language tags, and so they're normally written that way). > I think gfxFontStyle could perhaps use some comments as to how language > and languageOverride relate to each other. Do we really need both? Unfortunately, we do, because they're different things; I'll add comments to clarify this. > (I'm also a bit concerned about the feature settings property being a > single string rather than a comma-separated list of strings. It just > feels odd to have a list all stored *inside* a string.) This would be a spec issue; the current CSS3 Fonts draft (http://dev.w3.org/csswg/css3-fonts/#font-feature-settings-prop) says it's a string, but perhaps we should consider allowing a list of strings. As far as CSS is concerned it's an opaque value whose exact contents and interpretation is dependent on the type of font being used. John, any thoughts on the benefits or drawbacks of making this property a list of strings at the CSS level? > I presume the nsChildView.mm changes and the nsTArray changes weren't > really intended to be part of this patch. The patch requires us to add nsTArray::Equals (used when testing equality of nsFont objects with arrays in the featureSettings field); I implemented nsTArray::Compare as well as Equals because it seemed logical in terms of the API, but that's not actually required here. IIRC, the nsChildView.mm change (just moving the #include of prlog.h a bit earlier) was required because of a build issue that shows up otherwise, but I'll double-check what that was about. Will work through the other issues mentioned. Thanks!
I'd note that the parsing issues are also a spec issue: it's unusual (and bad for cascading) for a CSS spec to accept values at parse time that don't mean anything. The spec ought to define which values are accepted at parse time, but in the end we ought to match it.
(In reply to comment #14) > > I think gfxFontStyle could perhaps use some comments as to how language > > and languageOverride relate to each other. Do we really need both? > > Unfortunately, we do, because they're different things; I'll add comments to > clarify this. The current editor's draft has an example of how font-language-override is used: http://dev.w3.org/csswg/css3-fonts/#font-language-override-prop > > (I'm also a bit concerned about the feature settings property > > being a single string rather than a comma-separated list of > > strings. It just feels odd to have a list all stored *inside* a > > string.) > > This would be a spec issue; the current CSS3 Fonts draft > (http://dev.w3.org/csswg/css3-fonts/#font-feature-settings-prop) > says it's a string, but perhaps we should consider allowing a list > of strings. As far as CSS is concerned it's an opaque value whose > exact contents and interpretation is dependent on the type of font > being used. > > John, any thoughts on the benefits or drawbacks of making this > property a list of strings at the CSS level? We could use a slightly different syntax, something like: font-feature-settings: <tag>[(<integer>)]? [<tag>[(<integer>)]? ]* where <tag> is a four-character sequence and <integer> is zero or greater. Example: font-feature-settings: salt(5) ss03 liga hlig(0); This would be equivalent to: -moz-font-features-opentype: "salt=5,ss03=1,liga=1,hlig=0"; I'm not sure we can really go beyond that. The whole point of this is to effectively allow a pass-through to an OpenType layout engine, which tag values have meaning is defined by OpenType.
(In reply to comment #16) > We could use a slightly different syntax, something like: > > font-feature-settings: <tag>[(<integer>)]? [<tag>[(<integer>)]? ]* > > where <tag> is a four-character sequence and <integer> is zero or greater. I'd be hesitant to do this, as it ties the CSS syntax too closely to the particulars of OpenType technology, and if/when some other font technology comes along that does not express font features as 4-char tags and integer values, we're in trouble. Alternative: font-feature-settings: <string> [, <string>]* which is simply a list of strings to be passed to the font-rendering system for interpretation. > Example: > > font-feature-settings: salt(5) ss03 liga hlig(0); This then becomes font-feature-settings: "salt=5", "ss03", "liga", "hlig=0"; > I'm not sure we can really go beyond that. The whole point of this is > to effectively allow a pass-through to an OpenType layout engine, > which tag values have meaning is defined by OpenType. I would put this differently: The whole point of this is to allow a pass-through of arbitrary parameters to a font layout engine, without attempting to define or interpret the values at the CSS level. (I.e., it should not be tied to the OpenType feature model, IMO.) Even in today's world, there are at least two existing font technologies (AAT and Graphite) that would require something other than 4-character tags to control arbitrary features. This CSS property should leave room for that.
At the risk of overengineering: should font-feature-settings have a way to specify exactly which font the features apply to? That is, given: font-family: "Foo", "Bar", "Baz"; you may want "salt(5) ss03 liga hlig(0)" to only apply to the "Foo" family, not whatever fallback font the browser chose if the user doesn't have Foo available, because you have no idea what effect those features might have there (and might indeed be broken).
(In reply to comment #18) > At the risk of overengineering: should font-feature-settings have a way to > specify exactly which font the features apply to? That is, given: > > font-family: "Foo", "Bar", "Baz"; > > you may want "salt(5) ss03 liga hlig(0)" to only apply to the "Foo" family, not > whatever fallback font the browser chose if the user doesn't have Foo > available, because you have no idea what effect those features might have there > (and might indeed be broken). Yeah, lots of discussion on www-style related to this topic. The current draft spec allows for font-feature-settings to be specified per-font in @font-face rules. Combining this with src: local() use allows it to be applied to specific platform fonts: http://dev.w3.org/csswg/css3-fonts/#rendering-considerations Scroll down to view Example XXIII. Not the most elegant solution but since feature use is most likely to be done in conjunction with downloadable fonts, it's not entirely crazy. I don't think the "only for the first font" way works very well, it starts to break down when you have font stacks that are designed for multiple platforms rather than "missing glyph" fallback (e.g. font-family: Helvetica, Arial, sans-serif;). In some cases you may want fallback (e.g. hlig(1)) in other cases not (e.g. ss03).
That makes sense -- sorry for the drive-by comment, and thanks for the explanation!
Depends on: 449292
In the light of review comments (thanks, dbaron!) and thinking about how the current CSS3 Fonts draft defines these properties, I think it's more logical to maintain them as simple strings at the CSS parser/rule level. As far as CSS is concerned, that's all they are; it has no concept of "valid" or "invalid" contents of these strings. Any interpretation of the string content is a matter for the font-rendering backend. So I've modified this to keep featureSettings and languageOverride as uninterpreted strings at the CSS level; they only get parsed further (or ignored, if their content isn't meaningful) when they are handed to the thebes font system, which knows about arrays of OpenType features and with OpenType langSys tags. I've also added -moz-font-feature-settings and -moz-font-language-override as descriptors in @font-face rules.
This supports -moz-font-feature-settings and -language-override both as style properties and @font-face descriptors, with the style taking precedence, as in the current CSS draft. Note that adding these descriptors to @font-face required me to modify the gfxFontCache so that the key is not (psname + style); instead, it's now keyed on the (fontEntry + style), as there may be multiple fontEntries with different properties referring to the same postscript name. A side benefit is that this should make the cache marginally faster, and tryserver results suggest this may even show up as a (slight) visible improvement.
Attachment #445112 - Attachment is obsolete: true
Attachment #451899 - Flags: review?(jdaggett)
Attached patch part 2a - add an Equals method to nsTArray (obsolete) (deleted) — Splinter Review
Although the CSS side no longer uses this, it is still needed to support the gfxFontStyle code that handles arrays of feature settings.
Attachment #451900 - Flags: review?(dbaron)
+inline PRBool +operator<(const gfxFontFeature& a, const gfxFontFeature& b) +{ + return (a.mTag < b.mTag) || ((a.mTag == b.mTag) && (a.mValue < b.mValue)); +} Unused, trim. From comment 23: > Note that adding these descriptors to @font-face required me to > modify the gfxFontCache so that the key is not (psname + style); > instead, it's now keyed on the (fontEntry + style), as there may be > multiple fontEntries with different properties referring to the same > postscript name. A side benefit is that this should make the cache > marginally faster, and tryserver results suggest this may even show > up as a (slight) visible improvement. > static PLDHashNumber HashKey(const KeyTypePointer aKey) { > - return HashString(aKey->mString) ^ aKey->mStyle->Hash(); > + return NS_PTR_TO_INT32(aKey->mFontEntry) ^ aKey->mStyle->Hash(); > } Hmmm, I don't think this will work. While font entries for platform fonts have a lifetime of the app, downloadable fonts don't, a font entry could be reused. The only way to distinguish between fonts is via the name unfortunately. I don't quite follow why this was needed. If the key is (name + style) and the feature list is part of style, you still have two unique keys no?
(In reply to comment #25) > +inline PRBool > +operator<(const gfxFontFeature& a, const gfxFontFeature& b) > +{ > + return (a.mTag < b.mTag) || ((a.mTag == b.mTag) && (a.mValue < b.mValue)); > +} > > Unused, trim. Actually, this is needed by gfxFontStyle::ParseFontFeatureSettings because it does InsertElementSorted when adding features to the array. This was done so that (for example) styles with "dlig=1,smcp=1" and "smcp=1,dlig=1" will be seen as identical. However, maybe it's not worth bothering with this. > > From comment 23: > > > Note that adding these descriptors to @font-face required me to > > modify the gfxFontCache so that the key is not (psname + style); > > instead, it's now keyed on the (fontEntry + style), as there may be > > multiple fontEntries with different properties referring to the same > > postscript name. A side benefit is that this should make the cache > > marginally faster, and tryserver results suggest this may even show > > up as a (slight) visible improvement. > > > static PLDHashNumber HashKey(const KeyTypePointer aKey) { > > - return HashString(aKey->mString) ^ aKey->mStyle->Hash(); > > + return NS_PTR_TO_INT32(aKey->mFontEntry) ^ aKey->mStyle->Hash(); > > } > > Hmmm, I don't think this will work. While font entries for platform > fonts have a lifetime of the app, downloadable fonts don't, a font > entry could be reused. The only way to distinguish between fonts is > via the name unfortunately. > > I don't quite follow why this was needed. If the key is (name + > style) and the feature list is part of style, you still have two > unique keys no? As per discussion on irc, with feature/language descriptors in @font-face, we can have multiple font entries with the same name but different features in the entry (which are separate from the features in the style). Using the font entry pointer is safe because it's guaranteed to remain unique as long as any font based on that entry remains in existence; the possibility of re-use only occurs after all instances have expired.
Attachment #451899 - Flags: review?(jdaggett) → review+
Attachment #451900 - Flags: superreview?(benjamin)
Attachment #451900 - Flags: review?(dbaron)
Attachment #451900 - Flags: review+
Comment on attachment 451900 [details] [diff] [review] part 2a - add an Equals method to nsTArray You probably want to save Length() into a local variable so you don't have to probe into mHdr each iteration... and probably the same for Elements() of both arrays. r=dbaron with that
Comment on attachment 451898 [details] [diff] [review] part 1, v3 - maintain features/langOverride as strings in css rules; also support @font-face descriptors r=dbaron; sorry for the delay getting to this I'd note that while this is labeled as patch 1, it depends on the gfxFontStyle members added in patch 2 (and the constructor changes). (I'm not sure if there's a good way to split the patches so the halfway state compiles.)
Attachment #451898 - Flags: review?(dbaron) → review+
Comment on attachment 451900 [details] [diff] [review] part 2a - add an Equals method to nsTArray While refreshing patches, I realized we don't need "part 2a" (adding the nsTArray::Equals method) at all, as there's already an operator== defined that we can use. Hence marking this patch as obsolete.
Attachment #451900 - Attachment is obsolete: true
Attachment #451900 - Flags: superreview?(benjamin)
Assignee: jdaggett → jfkthame
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
bustage fix for Android build (FT2Fonts): http://hg.mozilla.org/mozilla-central/rev/14d43898f62a
Jonathan, could you update your "CSS OpenType Font Feature demo", so we can easily try this out?
Good point - the property names have changed since that was done, so it won't work as-is. I'll refresh it....
Rob: yes, your copies should still work, the syntax hasn't changed (again). Note that the link to the "experimental build" on those pages is now obsolete, however. I've updated my samples at http://people.mozilla.com/~jkew/feature-samples/ to work with current trunk builds.
This adds a small collection of font-feature reftests, using the Linux Libertine font (as it has a wide range of features and language support - we'll also be able to use it to test a number of more specific CSS properties once we implement those).
Attachment #457599 - Flags: review?(jdaggett)
Keywords: dev-doc-needed
(In reply to comment #36) > This adds a small collection of font-feature reftests, using the Linux > Libertine font (as it has a wide range of features and language support - we'll > also be able to use it to test a number of more specific CSS properties once we > implement those). Can you check in the fonts for these so that I can run the tests using them?
As mentioned in IRC, the ref case (font-features-ref.html) needs to set the lang attribute to 'en' or some other language, otherwise these tests will fail when run under Turkish locales.
Added explicit lang=en to all the tests except where a different lang is specifically being tested, to give consistent behavior regardless of system locale.
Attachment #457599 - Attachment is obsolete: true
Attachment #458412 - Flags: review?(jdaggett)
Attachment #457599 - Flags: review?(jdaggett)
Attachment #458412 - Flags: review?(jdaggett) → review+
Blocks: 585718
Is the syntax in comment #0 still accurate, or is there someplace else I can go to look at syntax details? Or do I just need to pore over the examples and piece it together to write documentation?
(In reply to comment #41) > Is the syntax in comment #0 still accurate, Almost. The property is now -moz-font-feature-settings (following changes to the Editor's Draft of CSS3 Fonts). Note that there may well be further changes in the future as that draft evolves. In addition to -moz-font-feature-settings, we have also implemented -moz-font-language-override. These correspond to the draft font-feature-settings and font-language-override properties described in http://dev.w3.org/csswg/css3-fonts/.
Depends on: 589573
Summary: implement experimental support for -moz-font-feature-opentype → implement experimental support for -moz-font-feature-settings
This got documented yesterday or day before.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: