On Fri, 2014-03-21 at 00:59 -0400, Martin Owens wrote:

Hi Devs,

I need some help with the style bug #167937

CSS has very specific rules for how cascading works. Values set on properties using the "style" attribute have precedence over values set using 'class' or 'element' values in style sheets. Technically, a style sheet that uses an 'id' selector could override a value from the "style" attribute but this is unlikely and code to handle this would be complicated.

The rules for precedence are defined in CSS2:

http://www.w3.org/TR/2008/REC-CSS2-20080411/cascade.html#cascade

Inkscape currently (in sp_style_read)

1a. Reads the style from an object's "style" sheet (i.e. 'style' attribute). 1b. If value not set, reads from internal or external style sheet (using libcroco). 2. If not set, reads from presentation attributes (which have lowest precedence). 3. Cascading values from parent.

This is the correct order and should not be changed.

I've created this patch which I've put into a branch which fixes the direct issue as it does not corrupt element styles with css values any more:

http://bazaar.launchpad.net/~doctormo/inkscape/css-style/revision/13177

It's a fairly simple patch which redirects the stylesheet setter so it sets the computed value of each of the properties instead of the actual value.

But it has bugs. Elements appear black until they have been nudged, then the style is applied. I can imagine the parent/inherited styles probably suffer the same bug but it's been under reported.

I'd also be interested in hearing if this is the right approach to fix the bug. Would it be better to change it so it updated the styles value but kept the .set set to false to indicate no real value should be saved?

The correct way to fix the bug is to keep track of where each property is set. Until editing of style sheets is properly handled by Inkscape, one could use a flag to indicate whether or not a property was set in a "style" attribute. Inkscape would then only write out the value in the "style" attribute if it was originally set in a "style" attribute.

Note, that I am currently in the middle of rewriting the code in style.cpp to use classes. I have found it a real PITA to add new properties with the existing code. This new code should make it much easier as well as make it easy to add the above mentioned flag.

Tav