It seems Servo has implemented most parts (if not all) of CSSStyleDeclaration, and I think Stylo should share the code of that. However, the majority of that code currently lives in script component (script/dom/cssstyledeclaration.rs), which is not usable for stylo. I hope we can move the majority of the spec logic into the style component (presumably style/properties/properties.mako.rs: impl PropertyDeclarationBlock), so that they can be shared between Servo and Gecko. I imagine that, after moving that code, impl of methods in CSSStyleDeclarationMethods could almost be as simple as: 1. get declaration block 2. call the corresponding method on the declaration block And the glue function for stylo as well. Ideally the methods of PropertyDeclarationBlock would accept a fmt::Write like ToCss, so that it is possible to do any string conversion on-the-fly.

Simon, could you take this bug?

I think we can move code of the following methods: * Length * Item * GetPropertyValue * GetPropertyPriority * SetProperty * SetPropertyPriority * RemoveProperty CssFloat, SetCssFloat, IndexGetter, CssText and SetCssText look simple enough. And after this moving, I guess we may be able to unpub "important" and "normal" fields in PropertyDeclarationBlock, as it is now the implementation details which may be changed in the future. (Actually we would eventually need to change it, as there is requirement on the property order)

For string input, I guess using &str is enough for now. We probably want to use Atom for property name in the future.

Regarding the important and normal fields: I think they need to be merged into a single Vec (in order to preserve relative ordering). I’ll look into that first, and moving code as you propose next.

I suppose that we would no longer need the Vec to be Arc'ed after the merge and code movement? I'm going to make GeckoDeclarationBlock refcounted, and it's unfortunate to see nested Arc like this. I guess Servo code can refcount PropertyDeclarationBlock directly in its dom code, which would be independent to the refcounted GeckoDeclarationBlock. Does that sound fine to you?

Yes, I’m gonna remove that Arc so that PropertyDeclarationBlock contains just Vec<_> instead of Arc<Vec<_>>, and instead have other things hold an Arc<PropertyDeclarationBlock>. Then the Gecko reflector can hold another reference to it.

For importance, I think all comparison and generating of "important" string can be done in the Gecko side. Servo side should accept and return a bool value about whether a property is important.

FWIW, the current implementation of Servo's SetPropertyValue is actually wrong. According to the spec, calling that wouldn't change the important flag of the property declaration, however, the current implementation changes. I guess we may want a method in PropertyDeclarationBlock which takes two Options for value and priority, so that it covers SetProperty, SetPropertyValue, and SetPropertyPriority without much code duplication.

I think that issue of whether !important is overridden is a longstanding disagreement dbaron has with the spec.

Oh, hmmm, it seems actually no mainstream browser implements SetPropertyValue at all. Other than that, I don't see other difference between the spec and implementation. Setting property value via setters well overrides the important flag, in both impls and the spec.

Simon, is there any progress on this? What is currently blocking you from having this done? I suppose some dilemma on locking? Could you inform me about blockers of this via adding them to the "Depends on:" field, so that I can follow what's going on?

Sorry for the delays with this. There is no technical blocker that I know of; I’ve been busy with travel, conferences, and working group meetings. I’m traveling home tomorrow and don’t have more travel planned until December. For locking, I think we have a solution in Bug 1305141 and https://github.com/servo/servo/pull/13459.

That's fine. I just want to follow what's going on there. Thanks for the update!

https://github.com/servo/servo/pull/13640 is up. There is still a fair amount of code in components/script/dom/cssstyledeclaration.rs, but most of it a Servo-specific (like marking things dirty) or do early returns. Let me know if you’d like more to be moved. I’m also planing more refactoring to use atoms more for property names, removing some string allocations and comparisons.

Looks like this has been fixed.