Created attachment 306830 [details] [review]
patch

(I forgot to remove the changes to gtkrender.c)

Hmm, I must say that I'm not looking forward to adding css text property support that goes 1-property-at-a-time and 1-widget-at-a-time.

Created attachment 306841 [details] [review]
patch

(In reply to Matthias Clasen from comment #2)
> Hmm, I must say that I'm not looking forward to adding css text property
> support that goes 1-property-at-a-time and 1-widget-at-a-time.

I understand, however it is not different from how other things are handled... For instance in gtklabel itself the text-shadow is handled in a similar ad-hoc way.

What about this updated patch: we add a private gtk_style_context_get_pango_attributes which can be used to return other css properties that map to pango attributes and can be used in all widgets that render a pango layout.

Created attachment 306842 [details] [review]
entry patch

This follow up patch uses the context method to also add support for letter-spacing to GtkEntry.

(In reply to Matthias Clasen from comment #2)
> Hmm, I must say that I'm not looking forward to adding css text property
> support that goes 1-property-at-a-time and 1-widget-at-a-time.

We wouldn't need to do that if Pango had a pango_font_description_set_letter_spacing().
Because that's generally the way we apply CSS: We modify the font description (that would happen automatically in gtkwidget.c), too.

Review of attachment 306841 [details] [review]:

Some comments on all those magic flags when declaring CSS properties:

::: gtk/gtkcssstylepropertyimpl.c
@@ +617,3 @@
+  return _gtk_css_number_value_parse (parser,
+                                      GTK_CSS_PARSE_LENGTH
+                                      | GTK_CSS_PARSE_PERCENT

The spec doesn't allow for percentage values.

@@ +618,3 @@
+                                      GTK_CSS_PARSE_LENGTH
+                                      | GTK_CSS_PARSE_PERCENT
+                                      | GTK_CSS_POSITIVE_ONLY

The spec says negative values are allowed. And I think Pango allows negative values, too.

@@ +619,3 @@
+                                      | GTK_CSS_PARSE_PERCENT
+                                      | GTK_CSS_POSITIVE_ONLY
+                                      | GTK_CSS_NUMBER_AS_PIXELS);

That flag is a backwards compatibility flag for 3.0, so we shouldn't use it for newly added properties.
(It interprets "3.0" as "3.0px" and issues a deprecation warning to stderr/emits the error signal.)

@@ +1081,3 @@
+                                          G_TYPE_NONE,
+                                          GTK_STYLE_PROPERTY_INHERIT | GTK_STYLE_PROPERTY_ANIMATED,
+                                          GTK_CSS_AFFECTS_TEXT | GTK_CSS_AFFECTS_CLIP,

This should be GTK_CSS_AFFECTS_FONT | GTK_CSS_AFFECTS_TEXT (same as font-variant or font-style).

@@ +1084,3 @@
+                                          parse_letter_spacing,
+                                          query_length_as_int,
+                                          assign_length_from_int,

Please use NULL for both the query and assign function. I don't want to make new properties available via gtk_style_context_get/set().

I am willing to (try to) make a patch to add letter spacing to PangoFontDescription if that's the way forward.

That said it seems to me that letter-spacing is not really a property of the font, but of how the layout is rendered and adding it to the font description seems a bit of a slippery slope... what if next we want to support word-spacing or text-decoration? I do not think we should shove all of that in the font description.

For instance text-decoration=underline would today map to the underline PangoAttr so if we have an entry point to add attrs to the context it should be relatively easy to support (and for instance we could remove hardcoded underline for links and do it from the css of the theme)

Created attachment 306860 [details] [review]
patch

updated according to Benjamin's comments

Created attachment 306862 [details] [review]
text-decoration-line patch

As outlined in my comment above, this implements support for text-decoration-line=(none|underline|line-through) which works automatically for label and entry.

Note 1: this does not yet add support for text-decoration-color, text-decoration-style and for the text-decoration shorthand

Note 2: this only supports underline and line-through since overline is not supported by pango

Created attachment 306864 [details] [review]
linkbutton patch

Use the css property added in the previous patch to drop custom code in linkbutton

Created attachment 306870 [details] [review]
patch for text-decoration-colo

I pushed a wip/pbor/css-text-attributes with a rebased and improved patchset

- it fixes a couple of things pointed out by Benjamin I missed before
- it introduces an AFFECTS_TEXT_ATTRIBUTES flag as discussed on irc 
- it supports strikethrough color
- it adds the shorthand text-decoration property

<Company> pbor: with those nitpicks, feel free to push to master


I fixed up a what Benjamin pointed out on irc and pushed the branch to master

See bug 686179 for adding overline support to pango

According to the CSS spec, we also need wavy, double, dotted and dashed overline (and strikethrough and dotted/dashed underline).

Not that I think anybody wanted to use it right now, but it's what's missing still.

pango supports some of this. Main omissions are dashed and dotted variants, and the gap stuff.

I pushed a patch to support single/double/wavy.